Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found

Submitted

Responsive Product Review Card

Modesire Soneyeβ€’ 270

@Desireye

Desktop design screenshot for the Product preview card component coding challenge

This is a solution for...

  • HTML
  • CSS
1newbie
View challenge

Design comparison


SolutionDesign

Community feedback

@MelvinAguilar

Posted

Hello there πŸ‘‹. Good job on completing the challenge !

I have some suggestions about your code that might interest you.

HTML 🏷️:

  • Wrap the page's whole main content in the <main> tag.
  • You can use the <picture> tag when you have different versions of the same image πŸ–Ό. Using the <picture> tag will help you to load the correct image for the user's device saving bandwidth and improving performance. You can read more about this here πŸ“˜.

    Example:

    <picture>
        <source media="(max-width: 460px)" srcset="./images/image-product-mobile.jpg">
        <img src="./images/image-product-desktop.jpg" alt="{your alt text goes here}">
    </picture>
    
  • Avoid using uppercase text in your HTML because screen readers will read it letter by letter. You can use the text-transform property to transform the text to uppercase in CSS.

    The word "perfume" is written as separate letters, which does not convey the meaning that this text is a single cohesive unit of content. This can be confusing for users and for screen readers, as it can be difficult to understand the meaning of the text.

    Example:

    <p>Perfume</p>
    
    p {
        text-transform: uppercase;
        letter-spacing: 0.3em;
    }
    

CSS 🎨:

  • Instead of using pixels in font-size, use relative units like em or rem. The font-size in absolute units like pixels does not scale with the user's browser settings. Resource πŸ“˜.

I hope you find it useful! πŸ˜„

Happy coding!

Marked as helpful

3

Modesire Soneyeβ€’ 270

@Desireye

Posted

@MelvinAguilar Thank you for taking the time to look through my code and suggest solutions :D

I've taken your advice in most of your suggestions, but for reason I couldn't get the <source> tag to work properly. Also I have a question, is it advisable to use %, em and rem in a code, or should I stick to one parameter per code?

Anyway, thank you once again! I've learnt alot from your comment and will try to implement them going forward :D

0
Hassia Issahβ€’ 50,670

@Hassiai

Posted

Replace<div class="container">with the main tag and wrap <h5>Coded by <a href="https://github.com/Desireye">Desireye</a></h5> within the footer tag to fix the accessibility issues. click here for more on web-accessibility and semantic html

To center .container on the page using flexbox or grid instead of margin, add min-height:100vh; display: flex; align-items: center: justify-content: center; or min-height:100vh; display: grid place-items: center to the body.

USING FLEXBOX:
body{
min-height: 100vh;
display: flex;
align-items: center;
justify-content: center;
}
USING GRID:
body{
min-height: 100vh;
display: grid;
place-items: center;
}

Hope am helpful.

Well done for completing this challenge. HAPPY CODING

Marked as helpful

1

Modesire Soneyeβ€’ 270

@Desireye

Posted

@Hassiai Thank you for your comment! I've taken your advice and can really see the difference :D

0

Please log in to post a comment

Log in with GitHub
Discord logo

Join our Discord community

Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!

Join our Discord