Skip to content
Submitted about 1 year ago

product preview card component

Ahmad Hassan•310
@Ahmadhassan0
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

Can someone look at my responsive design code, It's working fine but I think I'm not writing good responsive code

Community feedback

  • Rebecca Padgett•1,860
    @bccpadgePosted about 1 year ago

    Hello @Ahmadhassan0. Congratulations on completing this challenge!!!🎉

    I have few suggestions you might be interested in to improve your solution.

    • HTML structure needs to be updated:
      • You can wrap the attribution using the footer tag.
    <body>
  <main></main>
  <footer></footer>
</body>

    HEADING TAGS are improperly used in this project

    • I see you using a h2 for the title and needs to be a h1

    More info📚:

    When building this project, picture tag easily changes the image from mobile to desktop and vice versa.

    IMAGE ALT TAGS ARE VERY IMPORTANT FOR ACCESSIBILITY.

    • If a image is decorative, you can I add aria-hidden: true and image will be ignored by screen readers.

    Here is an example of the picture tag in my solution: Product preview card component

    <picture>
  <source media="(min-width: 37.5em)" srcset="./assets/images/image-product-desktop.jpg" />
  <img src="./assets/images/image-product-mobile.jpg" alt="Gabrielle Essence Perfume bottle laying down with a green leaf above and below it" />
</picture>

    More info📚:

    I suggest you change your media query to max-width: 48em which is 768px. 768px is tablet view and your component would change grid-template-column: 1fr at that breakpoint. Use em units for media queries because works all cross browsers.

    More info📚:

    For the two prices I would add visually-hidden CSS class to indicated the current price and original price for screen readers. You can check out my solution the link is above.

    I use this visually-hidden class from this article: visually-hidden

    Hope this helps you and have any questions I am glad to answer them just let me know✌🏼

    Marked as helpful
    1
    Ahmad Hassan•310
    @Ahmadhassan0Posted about 1 year ago

    Thanks @bccpadge, I highly appreciate your feedback

    I'll apply all your suggestions, and I looked at your code there are many things that I don't know but seem very helpful I'll also learn all those new things🙌

    0
  • Yves Manzi•70
    @ymanziPosted about 1 year ago

    Looks good for me.

    I saw in your html for your "card-title", the space between the letters you can also use the css property letter-spacing

    Marked as helpful
    1
    Ahmed albably•20
    @AhmedAlbablyPosted about 1 year ago

    Thank you. I'll do my best next time.

    0
    Ahmad Hassan•310
    @Ahmadhassan0Posted about 1 year ago

    Thanks @ymanzi

    From now I'll try to give space using CSS 😊

    0
  • Ahmad Hassan•310
    @Ahmadhassan0Posted about 1 year ago

    Thanks eescan

    0
    Ahmed albably•20
    @AhmedAlbablyPosted about 1 year ago

    thank you

    0
  • eescan•230
    @iskandar13abdurakhmonovPosted about 1 year ago

    That looks pretty good

    0
    Ahmed albably•20
    @AhmedAlbablyPosted about 1 year ago

    I'am so happy to see your comment thank you

    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