Community feedback

• 

  Melvin Aguilar 🧑🏻‍💻•61,140

  @MelvinAguilarPosted almost 2 years ago

  Hello there 👋. Good job on completing the challenge !

  I have some suggestions about your code that might interest you.

  Tips:

  • Here is something strange, you shouldn't repeat all the HTML for the mobile view, because you are repeating a lot of code that can be avoided, to make a responsive website, the main protagonist are the CSS styles.

  • You can use the <picture> tag when you have different versions of the same image 🖼. Using the <picture> tag will help you to load the correct image for the user's device saving bandwidth and improving performance. You can read more about this here 📘.

    Example:

    <picture>
        <source media="(max-width: 460px)" srcset="./images/image-product-mobile.jpg">
        <img src="./images/image-product-desktop.jpg" alt="{your alt text goes here}">
    </picture>
    

  HTML 📄:

  • Use the <main> tag to wrap all the main content of the page instead of the <div> tag. With this semantic element you can improve the accessibility of your page.

  • Use the <footer> tag to wrap the footer of the page instead of the <div class="attribution">. The <footer> element contains information about the author of the page, the copyright, and other legal information.

  • The <h1> is the most important heading on the page, In this challenge the perfumer's name can be considered like the title of the page, so it should be the <h1>

  CSS 🎨:

  • You should use the box-sizing: border-box property to make the width and height properties include the padding and border of the element. This will make it easier to calculate the size of an element. You can read more about this here 📘.

    Most likely, if you don't use box-sizing, padding won't work on your component. Try to remove all margins, add the padding and use * { box-sizing }.

  • Centering an element with position: absolute would make your element behave strangely on some screen sizes, "there's a chance the content will grow to overflow the parent". You can use Flexbox or Grid to center your element. You can read more about centering in CSS here 📘.

  body {
      min-height: 100vh;
      display: flex;
      justify-content: center;
      flex-direction: column;
      align-items: center;
  }
  
  .card {
      /* position: absolute; */
      /* top: 50%; */
      /* left: 50%; */
      /* transform: translate(-50%, -50%); */
  

  I hope you find it useful! 😄

  Happy coding!

  Marked as helpful

  0