Order Summary Challenge with Responsive Grid and Flexbox

Community feedback

• 

  Vanza Setia•27,775

  @vanzasetiaPosted over 3 years ago

  👋Hi Ahmad Syafiq Aqil Wafi!

  I have some feedback on this solution:

  • On mobile view (360px * 640px), the music icon is so big. Try to keep the original size (from the svg file).
  • For any decorative images, you should leave the alt="" empty and add aria-hidden="true" or role="presentation". It will make sure all screen reader ignore those images. In this case, all images are decorative only.
  • It seems like you are using BEM class naming convention or method. You can't nest an element inside an element. I recommend to read this BEM by example article

  <div class="modal-box__plan-item__details">
              <h2 class="modal-box__plan-item__details__item">Annual Plan</h2>
              <p class="modal-box__plan-item__details__item">$59.99/year</p>
  </div>
  

  • This one is also should be a modifier.

  <button class="btn btn-primary">
          Proceed to Payment
        </button>
  

  • You should not limit the height of the body element. You should use min-height instead. Right now, the card gets cut off on mobile view (360px * 640px).
  • To make the modal-box perfectly center on the page, I recommend to use this methods. It's much easier than using absolute positioning.

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

  • You should wrap all your page content with landmark, like main and footer tag. Wrap all your page content, except the attribution with main tag. For the attribution, you can swap the div with footer tag.
  • Annual Plan should not be a heading. Heading tags only for titling a section. I recommend to use strong tag instead.
  • Never use px. Use rem or sometimes em instead.

  Tips for writing a better HTML:

  • Plan your HTML markup first, without taking care about the styling.
  • Decide the HTML markup, like you're writing a document.

  That's it! Hopefully this is helpful!

  Marked as helpful

  1

  ---

  

  Ahmad Syafiq Aqil Wafi•170

  @SyafiqjosPosted over 3 years ago

  @vanzasetia

  Wow, thank you for your detailed feedback Vanza Setia! I learnt more than I can imagine!

  I just knew about role attribute, I will read more about it.

  That's right, I try to use BEM naming convention. Thank you for providing BEM resources! The examples are clearing my confusion.

  That tips for centering modal-box is really useful! Although the centering div meme is really popular haha. I rarely use min- or max- styling. I think I should try to use them from now on.

  That's never use px units tips is probably the reason why my work looks different on firefox and chrome. The pixel / screen on each browser probably different. I'll use rem or em from now on and will use px units when I have a reason to do so.

  Also thank you for the last second tips. You are right, I was actually switch between html and css file and do it simultaneously. Planning is really important.

  I would like to say thank you again for this helpfull feedback! I wish other person who has the same problem as mine could read your feedback, so they will got better next time.

  1

  

  Vanza Setia•27,775

  @vanzasetiaPosted over 3 years ago

  @Syafiqjos 🤗 You're welcome! I am glad that it's really helpful for you!

  0