Perfume product page using flexbox

Community feedback

• 

  P

  Huy Phan•3,420

  @huyphan2210Posted 3 months ago

  Hi, @Tabong-Ang I checked out your solution and I have some thoughts:

  Refactoring the <picture> element:

  Your current <picture> element has some redundancy in the second <source> tag. Here’s a simplified version:

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

  This approach is cleaner and avoids overlapping media queries.

  Improving semantic HTML:

  Consider replacing some of your div tags with more meaningful HTML elements:

  • .main-content could be a <main>.
  • .notes could be an <article>.
  • The "Perfume" heading could be a <label> instead of <h6> since it complements the main <h1>.
  • The price doesn’t feel like a heading. Instead of an <h1>, you could use a <span> for the price and style it to match the design.

  Ensuring full viewport coverage:

  Your body should fill the entire viewport. To achieve this, you can apply:

  body {
    min-height: 100vh;
    display: flex;
    justify-content: center;
    align-items: center;
  }
  

  min-height: 100vh ensures the body always spans the full height of the viewport, even when the content is smaller.

  Flexbox centers your card both vertically and horizontally within the body.

  Hope this helps!

  Marked as helpful

  0

  ---

  

  Tabong Philip•270

  @Tabong-AngPosted 3 months ago

  @huyphan2210 Thank you so very much for taking out your time to give me these corrections. I'm grateful 🥰

  1
• 

  GaZtuki•200

  @GaZtukiPosted 3 months ago

  Great Job

  Marked as helpful

  0

  ---

  

  Tabong Philip•270

  @Tabong-AngPosted 3 months ago

  @GaZtuki Thank you

  0