Skip to content
OverviewSolutions
22
Comments
151
Favour
@Nadine-Green

All comments

  • Favour•2,160
    @Nadine-GreenPosted almost 2 years ago

    HEY YISHAK!

    I noticed that the screenshot generated does not match the website, you will need to generate a new one, just wanted to inform you.

    HOPE I COULD BE OF HELP :)

    HAPPY CODING!

    1
  • Franci Melink•400
    @francimelinkSubmitted almost 2 years ago

    Hi,

    Another solution from my side. I tried to solve it as easily as possible. For selectors, I mostly used tags themselves, on the contrary, I named classes with and without BEM methodology. I used relative sizes/distances where I knew/could.

    I would appreciate any comments.

    Order summary card flex, BEM, CSS reset

    2
    Favour•2,160
    @Nadine-GreenPosted almost 2 years ago

    HEY FRANCI!

    I really like your design however I noticed that none of the active states were included, just thought I would inform you.

    HOPE I COULD BE OF HELP :)

    HAPPY CODING!

    Marked as helpful
    0
  • Favour•2,160
    @Nadine-GreenPosted almost 2 years ago

    HEY BOHDAN!

    I noticed that a mobile design was not included in your solution, to ensure a good user experience, it is good practice to have a mobile design as well as a desktop design, you will need to address this.

    Instead of using a div for the <div class="attribution">, you should opt for a more semantic element like footer which will remove the error on your accesibilty report.

    HOPE I COULD BE OF HELP :)

    HAPPY CODING!

    1
  • Favour•2,160
    @Nadine-GreenPosted almost 2 years ago

    HEY THERE!

    You will need to set a max-width for the desktop design of your elements because, if you notice, they keep expanding, adding this property will prevent them from doing that

    HOPE I COULD BE OF HELP :)

    HAPPY CODING!

    0
  • Favour•2,160
    @Nadine-GreenPosted almost 2 years ago

    HEY ALEX!

    I believe for the change, the underline is supposed to be removed when the cursor hovers on it.

    HOPE I COULD BE OF HELP :)

    HAPPY CODING!

    Marked as helpful
    1
  • Favour•2,160
    @Nadine-GreenPosted almost 2 years ago

    HEY NIKITA!

    Congratulations on completing your first challenge

    I really like your design

    However, I think you forgot to include an alt for your image, which is why you have a warning for it

    HOPE I COULD BE OF HELP :)

    HAPPY CODING!

    1
  • Favour•2,160
    @Nadine-GreenPosted almost 2 years ago

    HEY MURAT!

    I love the outcome of your solution, however, I would like to give some pointers on how it could be improved.

    • You seem to have forgotten to capitalize the word 'Perfume', you will need to use a text-trasform for this.

    • You also need to give the button a hover effect, you will need to use a darker green.

    • As for the accessibility warnings, you will need to use a main instead of a div for the <div class="container">, this should remove the warning.

    HOPE I COULD BE OF HELP :)

    HAPPY CODING!

    Marked as helpful
    0
  • Favour•2,160
    @Nadine-GreenPosted almost 2 years ago

    HEY MUHAMMAD!

    To get rid of your accessibility warning, you will need to replace the div in <div class="container"> with a more semantic element like main.

    HOPE I COULD BE OF HELP :)

    HAPPY CODING!

    1
  • Favour•2,160
    @Nadine-GreenPosted almost 2 years ago

    HEY SAGUN!

    I noticed that in the mobile design, you have a problem with centering, a quick fix for this would be the give the body a height of 100vh height:100vh then a display of grid display: grid and then use the code place-items: center.

    Instead of using a div for <div class="footer">, you should use a more semantic element like footer instead.

    HOPE I COULD BE OF HELP :)

    HAPPY CODING!

    1
  • Favour•2,160
    @Nadine-GreenPosted almost 2 years ago

    HEY OLUWASINA!

    Congratulation on completing your first challenge

    I noticed a few things I would like to point out:

    • You will need to reduce the size of your parent container as it is too large.

    • You have some accessibility issues, I can help you with some of them:

      • Instead of using a div for <div class="container">, you should instead opt for a more semantic element like main

      • The same goes for <div class="attribution"> instead of a div a footer would be much better.

      • you can learn more about sematic element here

    HOPE I COULD BE OF HELP :)

    HAPPY CODING!

    Marked as helpful
    0
  • Favour•2,160
    @Nadine-GreenPosted almost 2 years ago

    HEY SHIVANGI!

    Congratulation on completing your first challenge

    Your solution is so close to being perfect however I noticed a few things I would like to point out:

    • You either forgot to include your image or the link is broken because it is not showing up, you should check up on this.

    • You have some accessibility issues, I can help you with some of them:

      • You will need to give your img an alt text.

      • Instead of using a div for <div class="container">, you should instead opt for a more semantic element like main

    HOPE I COULD BE OF HELP :)

    HAPPY CODING!

    Marked as helpful
    0
  • Favour•2,160
    @Nadine-GreenPosted almost 2 years ago

    HEY YOUXIE!

    Congratulations on completing your first challenge

    Concerning your question about centering, One of the best ways for centering would be to give the body a height of 100vh height:100vh then a display of grid display: grid and then using the code place-items: center

    There is no need to place a p inside an h2 or h3, this will get rid of the error on your HTML report.

    You will also need to increase the width of your parent container and set a max-width for as well so that it does not constantly increase in size.

    HOPE I COULD BE OF HELP :)

    HAPPY CODING!

    Marked as helpful
    1