Order-Summary-Component

Community feedback

• 

  Lucas 👾•104,300

  @correlucasPosted over 2 years ago

  👾Hello Aman, congratulations for your new solution!

  Your solution is really good and there's nothing to say about the code, nice that you've used BEM. The value you’ve used for the shadow make it too much dark and strong, to create a smooth shadow you need to give it less opacity and more blur instead try value for example and see the difference between the previous shadow and this one box-shadow: 5px 5px 12px 5px rgb(0 0 0 / 5%);

  If you’re not familiar to box-shadow you can use this site to create the shadow design and then just drop the code into the CSS: https://html-css-js.com/css/generator/box-shadow/

  👋 I hope this helps you and happy coding!

  Marked as helpful

  1

  ---

  

  Aman•630

  @AmanGupta1703Posted over 2 years ago

  @correlucas Thank you! 😊

  1

  

  Lucas 👾•104,300

  @correlucasPosted over 2 years ago

  @AmanGupta1703 you're welcome Aman, feel free to ask me further questions 🙂

  Marked as helpful

  1
• 

  Eray•1,410

  @ErayBarslanPosted over 2 years ago

  Hey there, nice work with your solution! Some suggestions:

  • Regarding html structure, you have 3 nested <div> containers which you can achieve the same result by just using one. You can simply remove .summary-box without effecting the design. Removing .container will require little refactoring but you can remove anything related to that.
  • Using background on container breaks it's design on screen change. Using on body would be better option. After removing these elements from html you can use:

  body {
    ...
    background: url("/images/pattern-background-desktop.svg");
    background-repeat: repeat-x;
  }
  
  .summary-item {
    ...
    width: 90%;
    max-width: 414px;
  }
  

  As you can see, just adding max-width is enough to achieve the same thing. You can also lower the need of media-query by adjusting % values for the mobile version, then give it a max-width. This approach is also preventing content overflowing between 375-550px due to width: 48%.

  • Lastly, now we have just .summary-item as container, you can use <main> on it instead div to be semantically right. Aside these good work again and happy coding :)

  Marked as helpful

  1

  ---

  

  Aman•630

  @AmanGupta1703Posted over 2 years ago

  @ErayBarslan Thank you! 😊

  0