Order-Summary Challenge

Community feedback

• 

  P

  Fluffy Kas•7,715

  @FluffyKasPosted about 3 years ago

  Hey, your solution looks good! If you wanted to fix the few issues you have, here's how you could do it:

  1. As mentioned above, using a max-width will make it responsive.

  2. To fix the background, add background-size: contain.

  3. To center the card, use these on the body: min-height:100vh, display: grid and place-items: center. With this you can get rid of that margin-top as well.

  In general, using max/min-width instead of width and min/max-height instead of height is more useful. I'd also just avoid setting height unless it's really necessary (like it is on the body but probably unnecessary anywhere else).

  Marked as helpful

  1

  ---

  

  Steven Clark•10

  @phtevenclarkosPosted about 3 years ago

  @FluffyKas thank you very much for the feedback this was very helpful, as stated above I'm making the needed adjustments to my project as per your advice. Cheers!

  0
• 

  Syazwan Fitri•400

  @syzwnftrPosted about 3 years ago

  Hi Steven, as a first project I think this is good. But I noticed that it isn't responsive so maybe you can set max-width instead of width for main-box. Make sure to avoid using px because it is an absolute unit. For the accessibility issue, you can get rid of it by changing h2 to h1. I hope this will help.

  Marked as helpful

  1

  ---

  

  Steven Clark•10

  @phtevenclarkosPosted about 3 years ago

  @syzwnftr thank you for the feedback! I'm now amending as per your suggestion! Cheers!

  0