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

Product-Preview Card Component Challenge

Sunil Duttβ€’ 20

@superuser2345

Desktop design screenshot for the Product preview card component coding challenge

This is a solution for...

  • HTML
  • CSS
1newbie
View challenge

Design comparison


SolutionDesign

Solution retrospective


Please check my solution and let me know your input. BTW I have used FlexBox to build this card. I have used it the first time and really want to know, what could i have done instead of something.

Thanks Sunil

Community feedback

@MelvinAguilar

Posted

Hi @superuser2345 πŸ‘‹, good job completing this challenge! πŸŽ‰

I have some suggestions you might consider to improve your code:

  • Another alternative to using a flex layout for two columns with the same size is to use a grid layout:
section > div {
    display: grid;
    grid-template-columns: repeat(2, 1fr);
}
  • If the solution does not have content in the <header>, <nav> or <footer> elements, it is not necessary to add those elements.
  • In this challenge, you should not use the background property to set the image because this image has semantic meaning. Use the CSS background property if the image is not part of the content.

Above all, the project is done wellπŸ‘. I hope those tips will help you! πŸ‘

Good job, and happy coding! 😁

Marked as helpful

1

Sunil Duttβ€’ 20

@superuser2345

Posted

@MelvinAguilar Thanks Melvin, it's so kind of you that you have taken the time to share your valuable input.

  1. I thought grid is kind of old now and flexbox is what needs urgency to get hands-on right now, so that's why I have used that. I'm a bit confused between the two.
  2. Yes, that's I thought too in my first challenge, but Frontend Team has reverted with the remarks to keep it in practice.
  3. I have used background property to get rid of using two different image tags and for them two more div tags. If it's ok or not. And really thanks for helping with those suggestions, I will read further and implement them too. Thanks Sunil Happy Coding U 2! :)
1

@MelvinAguilar

Posted

@superuser2345 Hello again ! Thanks for sharing your comments.

  • The platform warns you to use semantic elements, but you are not required to use all of them and even more if they have no content. If you are suspicious, you can see my solution to this challenge, where I only use a single semantic element (<main>), and my report works perfectly.

In addition, I noticed that your solution has no styles on screens larger than 1440px due to the media query

  • When an image is set as a background, screen readers ignore those elements, making people with vision problems unaware of its presence

You can use a <picture> tag when you need to change an image in different viewports. Using this tag will prevent the browser from loading both images, saving bandwidth and preventing you to have two image

<picture>
   <source media="(max-width: 375px)" srcset="./images/image-product-mobile.jpg">
   <img src="./images/image-product-desktop.jpg" alt="your_alt_text">
</picture>

Marked as helpful

1

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