Order Summary Component

Community feedback

• 

  darryncodes•6,390

  @darryncodesPosted about 3 years ago

  Hi Hoangnam,

  Well done on completing your first project, it looks like a perfect match!

  A few thoughts from me

  • try updating this: background-size: 100% auto; and removing background-position-y: 5rem; the background image currently jumps down on small device screen widths
  • have you considered using media queries with rem units to help with responsive design?
  • css unit best practice
  • a brief guide on responsive web design
  • here is an example of what I mean:

  html {
      font-size: 62.5%; //1rem = 10px
  
      @media only screen and (max-width: 56.25em) { //900px
          font-size: 56.25%; //1rem = 9px
      }
  
      @media only screen and (max-width: 37.5em) { //600px
          font-size: 50%; //1rem = 8px
      }
  }
  

  Marked as helpful

  0

  ---

  

  hoangnam-nguyen•115

  @hoangnam-nguyenPosted about 3 years ago

  @darryncodes Many thanks for your comments! Your suggestions and additional links are very helpful! Much appreciated.

  0
• 

  Juveria Dalvi•1,375

  @Juveria-DalviPosted about 3 years ago

  hey.. the accessibility issue says that you should have wrapped all divs in <main> which is the semantic tag ,except for the div with class attribution that should be wrapper in <footer> tag also headings should be wrapped in <h1> <h1>Order Summary</h1>

  Marked as helpful

  0

  ---

  

  hoangnam-nguyen•115

  @hoangnam-nguyenPosted about 3 years ago

  I was not aware of this report. I have fixed the issues, please let me know if you have any further comments. Thanks a lot @JuveriaD !

  1