Mobile-first solution using CSS Flexbox

Community feedback

• 

  Navendu•270

  @AshxaryaPosted almost 2 years ago

  Hi! 😊

  I have some feedback to help you out.

  Make sure you look at the accessibility report you're provided with below your solution.

  HTML 📄:

  As you can see in your accessibility report, you are recommended to use a level-one heading as in h1 for the first text. H tags improve user experience in the sense that they're part of a web page's hierarchical structure. Think of them less as a way to size font and more of a way to show the order of headings, since the size can be manipulated in CSS regardless.

  Use the <footer> tag to wrap the footer of the page instead of the <div class="attribution">. The <footer> element contains information about the author of the page, the copyright, and other legal information.

  CSS 🎨:

  For ease of access in future projects, you can create variables of different colors at the beginning of your sheet. You can read more here to learn about this here.

  You may want to also use rem instead of pixels for font-sizes! You can learn more about this topic here , similarly, you can search for why you might want to use em values instead of px for padding, margins etc in some cases as well.

  Have a great day/night ^^

  Marked as helpful

  1

  ---

  

  Maciej Stolarski•20

  @AlearsonPosted almost 2 years ago

  @Ashxarya Thank you for taking the time to write me this review! <3

  Honestly I knew everything you wrote, but I had a few months break from studying Frontend and many things in my head are still in hibernation, but fortunately you reminded me of these things and now I won't make these mistakes again. Thanks again for this! <3

  0
• 

  Hassia Issah•50,650

  @HassiaiPosted almost 2 years ago

  Replace the <p> in the header tag<header> with <h1> to fix the accessibility issue.

  Use the colors that were given in the styleguide.md found in the zip folder you downloaded.

  There is no need to restyle the body in the media query.

  Use relative units like rem or em as unit for the padding, margin, width values and preferably rem for the font-size values, instead of using px which is an absolute unit. For more on CSS units Click here

  There is no need to give .perfume a max-height or min-height value the padding value in .description can replace that and its a responsive replacement which will prevent the content from overflowing on smaller screen seizes.

  in the media query replace the min-width of .perfume with max-width.

  Hope am helpful.

  Well done for completing this challenge. HAPPY CODING

  Marked as helpful

  1

  ---

  

  Maciej Stolarski•20

  @AlearsonPosted almost 2 years ago

  @Hassiai Thank you so much! I'm struggling with that min / max sizes but i will work on that. Hope to see you on my next challenge! <3

  1