Product review card component

Community feedback

• 

  Zubair Adham•400

  @atmahanaPosted about 1 year ago

  Hello there. Good job on finishing the challenge! 🥳

  You said you had a problem with media queries and it is understandable when you start learning to create responsive web layout.

  My suggestion is to use mobile-first approach. It's a good practice to start developing the layout and stuffs for mobile devices first.

  For example from your code, instead of writing the CSS like this:

  .container {
    width: 500px;
    height: 380px;
    border-radius: 10px;
    display: flex;
    background-color: white;
    overflow: hidden;
  }
  
  .div_1 {
    width: 50%;
    float: left;
    display: flex;
    background-image: url("/images/image-product-desktop.jpg");
    background-size: cover;
  }
  
  @media only screen and (max-width: 600px) {
    .container {
      width: 100%;
      height: 700px;
      display: flex;
      flex-direction: column;
      margin: 50px 12px;
    }
    .container .div_1 {
      background-image: url("/images/image-product-mobile.jpg");
      background-size: cover;
      background-repeat: no-repeat;
      background-position: center;
      width: 100%;
      height: 300px;
    }
  }
  

  You can write for mobile devices first:

  .container {
    width: 100%;
    height: 700px;
    display: flex;
    flex-direction: column;
    margin: 50px 12px;
    border-radius: 10px;
    background-color: white;
    overflow: hidden;
  }
  .div_1 {
    background-image: url("/images/image-product-mobile.jpg");
    background-size: cover;
    background-repeat: no-repeat;
    background-position: center;
    width: 100%;
    height: 300px;
  }
  
  @media (min-width: 640px) {
    .container {
      width: 500px;
      height: 380px;
      flex-direction: row;
    }
  
    .div_1 {
      width: 50%;
      float: left;
      background-image: url("/images/image-product-desktop.jpg");
    }
  }
  

  And now the code look less cluttered on the media queries.

  My other suggestions:

  • Use semantic class names. Instead of using div_1, create more meaningful class name like image-container to describe what is the element for. Read more here.
  • Use <img> element for the product image with alt attribute. Why? because it can improve the accessibility. Read more here.

  Overall you are doing great! Keep up the amazing work! 👏

  0
• 

  Neitodesu•180

  @NeitodesuPosted about 1 year ago

  Hello! As Induwara said, Kevin powell is an amazing resource for anything css! I'm no expert but I can say your css isnt looking bad. One thing to keep in mind is before we add any css, the page is already responsive. May not be pretty but it works for the most part. For simple projects as these "Newbie" ones, start with a "Mobile-first" approach. Once you block it out in mobile view it will be easier to add media queries where you need them. You would want to use ->

  @media screen and (min-width: 740px){
    content here
  }
  

  Now whenever the screen is 740px or above, the content will change.

  0
• 

  Ifeanyi Aladi•250

  @ShyneADLPosted about 1 year ago

  If you struggle with media-queries, might I suggest you pickup tailwind css, it's a lot easier to create responsive designs, it's also much faster.

  Great work!

  0
• Account deleted

  Hi, I just did this project and submitted it and I'm not a professional either 😀

  In your project the way you have done the media query is correct but I don't think you had to specify every font-size because in the style guides we were given that the p tag has 14 px. Yeah, I too nested divs like you and created it. That's about it 😅😊. If you actually have doubt about it there's the freecodecamp responsive design course and Kevin Powell's course.

  Hope it helped 😀😅

  0