Order Summary Component - HTML and CSS implementation

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted about 3 years ago

  Hey, awesome work on this one. Layout in general looks great but the layout is not responsive in terms of scaling right now.

  Sebastian Breit already gave great idea, just going to add some suggestions on this as well:

  • Don't style the html element, remove those and place them inside the body tag. You don't usually style the html element except for font-size and box-sizing.
  • Also, it would be great to have these base stylings:

  html {
    box-sizing: border-box;
    font-size: 100%;
  }
  
  *,
  *::before,
  *::after {
    box-sizing: inherit;
  }
  

  It would be long for me to explain this but trust me on this one, this would make handling the element easier than not setting the styling. You can search about it on this one.

  • Always have a main element to wrap the main content of your site. On this one, your #container should be using main instead of div.
  • Do not use margin to center the component, what you could do is that, remove the margin on the #container and on the body tag add:

      align-items: center;
      display: flex;
      flex-direction: column;
      justify-content: center;
      margin: 0;
      min-height: 100vh;
  

  This will make the layout always be centered dynamically. Also, always to set that margin: 0 on the body tag since by default there is an extra margin on the body.

  • When using img always include the alt attribute on it. If you don't add this, screen-reader would read the source path of the image which you don't want, either the alt is empty or not, include it always.
  • Do not use too many or just avoid in general using id attribute especially when using it as a styling selector, use class instead of id when selecting selector.
  • Same for music icon, alt is missing.
  • annual plan could use a heading tag like h2, since it gives overview on what the section would contain.
  • Do not just use span to wrap a text-content, always wrap with meaningful element like p tag, or wrap the span with a p tag.

  Aside from those, great work again on this one.

  Marked as helpful

  1

  ---

  

  Alisha•20

  @AlishaSoPosted about 3 years ago

  @pikamart This was really useful, thank you so much!! I'm working on the FAQ accordion project now, and will try to implement this on that 😊

  1
• 

  Sebastian Breit•70

  @SebastianBr11Posted about 3 years ago

  Instead of adding so much padding to the button, you could make the parent display flex and flex-direction column, so that the button automatically goes full length. You would then probably also have to add padding to the container and instead of adding the margin to each component separately you could add a flex gap.

  Marked as helpful

  1

  ---

  

  Alisha•20

  @AlishaSoPosted about 3 years ago

  @SebastianBr11 Helpful advice, thank you!!

  1