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 Using Css grids and Flexbox

@AliSherTR

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

P

@jguleserian

Posted

Greetings, Ali

Thank you for letting me take a look at your solution for the Product Preview Card Component. I like your use of grid to switch the orientation of the two columns and your CSS to switch the hero image.

Another method, which you probably thought of for the former, would be to use a flex box for the switching of the columns. Likewise, another element to be aware of is the <picture> elements that dynamically changes the image based on media queries as delineated in the HTML. You can take a look here in case you don't already know about this latter technique. Regardless of what you choose, your solutions worked very well.

I might also point out a couple of things I noticed that would be an easy fix. First, I noticed that in your <img> elements, you have the rel property listed, but without a description of the image. This could lead to issues for screen readers or for other users if the link should happen to break. Something like rel = "A bottle of Gabrielle Essence Eau De Parfum" would be enough to suffice.

I also noticed that the scaling was off on the entire component. I'm thinking that you did not have access to the Figma file which would have made getting the dimensions of your <div class="product"> easier (it was 60rem, not the 50rem that you have). In fact, you might take a look at the max/min-width settings and whether you need them if you are not making the container responsive and actually take care of the sizing in the media query. Since you are starting with the larger size, in your case 50rem, you could just put that as the width and let the media query do the rest.

Anyway, the container sizing threw off the other sizes and proportions as well. I encourage you to compare you project with the model they give you in the starting files.

Other things you might consider:

  1. Filling out the README-template.md, and then renaming it so it is the README.md file. This will help reviews understand your processes and struggles. It will also help you process the work better in your own mind.
  2. Try more semantic HTML, i.e., naming your containers that will have a better logical order. If a screen reader is trying to make sense of your site, I will have structures such as <header> <footer> <section> <article> etc. to help it navigate. It's also easier to others to get a handle on your structure a little more.

In the end, your solution looks great, and your project ended up being an attractive solution. I just encourage you to make more comparisons to the original, since the exercise is to copy it exactly.

Thank you again for letting me take a look at your work. If I can be of any help in any way, please let me know.

Happy coding!

Marked as helpful

0

@AliSherTR

Posted

@jguleserian thank you for the feedback. I will make sure I dont make these mistakes in future

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