Skip to content
Submitted 8 months ago

cart product preview

Edward Shanahan•50
@EdwardShanahan09
Preview site (opens in new tab)View code (opens in new tab)
This is a solution for...Product preview card component
  • HTML
  • CSS
1newbie

Design comparison

SolutionDesign

Solution retrospective

What are you most proud of, and what would you do differently next time?
What challenges did you encounter, and how did you overcome them?
What specific areas of your project would you like help with?

Community feedback

  • Emidio Valeretto•140
    @emidiovalerettoPosted 8 months ago

    It looks like you used flex to create this division between image and content. In this case, the best option would have been to use a two-column display grid. This would (by default) leave both parts in the same proportions. However, you've done a great job!

    Marked as helpful
    1
    Edward Shanahan•50
    @EdwardShanahan09Posted 8 months ago

    @emidiovaleretto thank you for your feedback 😊

    0
  • Flóra•180
    @Glorit74Posted 8 months ago

    I'm a newbie so not sure in my comments

    You set the font-size on the <html> element in percent. I would set on the <body> in px (this was given by the task). Why 62.5%?

    You dont use the desktop image (in line 24) , just the mobile version. You change the source at 800px but the mobile-desktop view is changed at 900px. I would put it in one place.

    You defines image height in line 41. If you have width (max-width would be better) and object-fit you dont need height at all.

    The price has its own font-family (Fraunces) but you didn't use it

    1
    Edward Shanahan•50
    @EdwardShanahan09Posted 8 months ago

    @Glorit74 Thank you for your feedback I have to go back in and change a few things. So i use 62.5% because its easier to calculate font sizes. 62.5% equals 10px. So if i want a font size to be 16px I will use 1.6rem.

    1
    Flóra•180
    @Glorit74Posted 8 months ago

    @EdwardShanahan09

    Thanks. I had to read it twice, but I understood it and it makes perfect sense. Have you tried how it affects the sizes if the user changes the default font size on her own browser?

    0
  • Daniel 🛸•44,310
    @danielmrz-devPosted 8 months ago

    Hello @EdwardShanahan09!

    Your solution looks excelent!

    I have just one suggestion:

    📌 To improve semantic clarity, try to maintain the titles hierarchy with <h1>, <h2> <h3>, and so on..

    It's more than just text size — it's about structuring your content effectively:

    • <h1> to <h6> are used to define HTML headings, with <h1> being the most significant.

    While these adjustments might not alter the visual appearance much, they significantly enhance semantic clarity, SEO optimization, and accessibility.

    Hope this suggestion proves helpful! Keep up the great work!

    1
    Edward Shanahan•50
    @EdwardShanahan09Posted 8 months ago

    @danielmrz-dev Thank you Daniel I forgot about the hierarchy. I will change it thank you again ☺️

    0

Please log in to post a comment

Log in with GitHub

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