Product Review Card Solution - using Grid and Flex

Community feedback

• 

  Lucas 👾•104,340

  @correlucasPosted over 2 years ago

  Hello Terry, welcome to the Frontend Mentor community and congratulations for your first challenge solution!

  I think you start with a really good solution, everything seems fine and accurate in comparison to the design files. Note that your card isn't flexible because you've used width instead of max-width if you use max-width this allows your container stretch in smaller screens, width keeps the container with a fixed value so this means that the container cannot grow or stretch.

  .product-container { max-width: 600px; }

  I did the challenge using the same approach you did, desktop = grid / mobile = flex. But you can do this only with grid, all you need to do is set a single column for the mobile version, grid-template-columns: 1fr;

  If you're interested, here's my solution: https://www.frontendmentor.io/solutions/product-preview-card-vanilla-css-and-custom-hover-state-on-hero-85A1JsueD1]

  I hope it helps you bro, keep coding!

  Marked as helpful

  1

  ---

  

  Terry•10

  @tbabyukPosted over 2 years ago

  @correlucas

  Hey Lucas, thanks for your valuable feedback!

  Regarding the max-width, this is a good tip but I have the media query set to display the card vertically when the screen is less than 600px, so does it matter here if I use max-width? Or perhaps I am misunderstanding something?

  As for "template-columns: 1fr", thank you for reminding me! I had wanted to use grid for both displays but tried to do "grid-template-rows" instead, which did not work out. Can't believe I didn't think of setting the columns to 1 col!

  1

  

  Lucas 👾•104,340

  @correlucasPosted over 2 years ago

  @tbabyuk Hello Terry, about the max-width I said to consider using it because it make your container flexible before the media-query. Do you know when you open an website and start to stretch and shrink a page and the content flows? Probably these contents has max-width/min-width. The media query is really useful for sure because they change more properties once they're activated, for example a flex-direction, the max-width doesn't do the same but allow you to have a container flexible with 350px or less. I hope this time my explanation was better Terry.

  1

  

  Terry•10

  @tbabyukPosted over 2 years ago

  @correlucas

  Thanks again Lucas, yes it was. By the way, is it common to re-submit a solution after receiving feedback from community members? Or do people usually submit just one solution?

  0

  

  Lucas 👾•104,340

  @correlucasPosted over 2 years ago

  @tbabyuk No you can just update the solution, go to your solution > visit challenge hub > update solution there you can update the links and say if you've another questions this way you keep the same solution but updated.

  Marked as helpful

  0

  

  Terry•10

  @tbabyukPosted over 2 years ago

  @correlucas

  Got it, thanks Lucas!

  0
• 

  Elaine•11,400

  @elaineleungPosted over 2 years ago

  Hi Terry, great discussion here :) For my challenge I actually tried something I don't usually do: I used flexbox for both views (or all views I guess), and specifically I opted to use flex-wrap in my card container and flex: 1 1 {width of child} in the two children divs, along with clamp() for the width property on the card div to handle responsiveness. This meant that when I wrote my desktop media query, I didn't need to make any changes to the display property; I only changed the width.

  Here's my solution if you want to check it out: https://www.frontendmentor.io/solutions/card-component-using-cube-css-tNaBY0y_Nx

  1
• 

  jerry•300

  @realsalePosted over 2 years ago

  @Yo Terry,

  Great job, it's pretty close to the design!

  Regarding to layout approach, I use grid just to center my component, and in desktop version I use flex property to simply layout my component content side-by-side(flex-direction: row; default).

  1
• 

  David Miller•180

  @HumerousPosted over 2 years ago

  Hi Terry,

  Overall great effort.

  I good way to send out your overall different font sizes as well on all screens sizes is to set the

  html { This is the maths: /* font-size: 10px; / / 10px / 16px = 0.625 = 62.5% / / Percentage of user's browser font-size setting */ font-size: 62.5%; }

  by doing this you are converting the 16px standard font size, to 10 rem or em and this in my opinion is the right approach.

  Also, always run your code through HTML/CSS validators.

  Please like the comment if you found it helpful, Happy coding :)

  0