Order summary card

Community feedback

• 

  P

  Fluffy Kas•7,715

  @FluffyKasPosted over 2 years ago

  Hey there,

  It's a good start, looks pretty close to the original, it's just missing responsiveness really. There's a few other things you could improve on as well:

  1. Instead of putting the background pattern in your html, you should use the background property on the body in CSS, as it's really just a background. You can add multiple backgrounds at once, comma separated (so you could add both an image and a color to the same element).

  2. Alt texts are supposed to give a description of images for people who can't see them. So instead of "illustration hero" (which isn't super helpful) you could go with "Girl listening to music" or anything along those lines. In case, you can't come up with anything meaningful, feel free to leave the alt text just blank, that's a valid solution as well (for decorative only images, for example).

  3. You're using position: absolute many times throughout this challenge but there's really not much need for it. Instead, just use the natural flow of elements. If you'd like to center things, use grid or flexbox.

  4. You're also giving elements an unnecessary height. There's no need to define height for your elements, most of them have their own height by default, which you can increase by adding padding to them.

  5. To center your component, you can use grid/flexbox by adding these to the body: min-height: 100vh, display: grid, place-items: center.

  6. For better responsiveness, use max-width instead of width.

  I hope this helps a bit ^^

  Marked as helpful

  2

  ---

  

  David Alejandro Cuastumal Bucheli•100

  @DcuastumalPosted over 2 years ago

  @FluffyKas Thanks for commenting and helping me improve the code!

  0
• 

  Ivan•2,630

  @isprutfromuaPosted over 2 years ago

  Hi there. You did a good job 😎

  keep improving your programming skills🛠️

  your solution looks great, however, if you want to improve it, you can follow these steps:

  ✅ there is no sense in such a division of styles. you make 3 requests to the server, it only reduces performance

    <link rel = "stylesheet" href = "assets / css / main.css">
    <link rel = "stylesheet" href = "assets / css / body.css">
    <link rel = "stylesheet" href = "assets / css / media.css">
  

  here it is better to use the picture tag

   <img class = "background" src = "assets / images / pattern-background-desktop.svg" alt = "pattern background">
    <img class = "background-mobile" src = "assets / images / pattern-background-mobile.svg" alt = "pattern background mobile">
  

  ✅ Do not use links with empty href attributes. It is better to use the button instead

  <a href="" class="btn-payment"> Proceed to Payment </a>
         <a href="" class="btn-cancel-order"> Cancel Order </a>
  

  ✅ You need to choose a name style and follow it. There is no single structure throughout the document

  Good luck and fun coding 🤝⌨️

  Marked as helpful

  1

  ---

  

  David Alejandro Cuastumal Bucheli•100

  @DcuastumalPosted over 2 years ago

  @isprutfromua Thanks for your help, I will implement the recommendations

  0

  

  Ivan•2,630

  @isprutfromuaPosted over 2 years ago

  @Dcuastumal your welcome

  I happy that I was useful for you =>

  Cheers

  0
• 

  Shashree Samuel•9,260

  @shashreesamuelPosted over 2 years ago

  Hey good job completing this challenge

  Keep up the good work

  Your solution looks great however I think that the card can be a little bit wider just to match or become seemingly close to the design.

  In terms of accessibility issues simply wrap all your content between main tags

  I hope this helps

  Cheers Happy coding 👍

  Marked as helpful

  0

  ---

  

  David Alejandro Cuastumal Bucheli•100

  @DcuastumalPosted over 2 years ago

  @TheCoderGuru Thanks for helping me improve the code!

  0