Order Summary Component

Community feedback

• 

  Vanza Setia•27,775

  @vanzasetiaPosted about 2 years ago

  Hi, Arnav! 👋

  Congratulations on completing your first Frontend Mentor challenge! 🎉

  Here are some suggestions for improvements.

  • There should not be text in span and div alone whenever possible. Instead, wrap the text with a meaningful element like a paragraph element.
  • Always specify the type of the button. It will prevent the button from behaving unexpectedly.
  • To place the card in the center of the page, I recommend using flexbox or grid. These modern techniques are more robust than absolute positioning and require less code to implement.
  • I recommend setting a max-width to the card (.box). This way, the card is allowed to shrink if ever needed while still preventing it from filling the entire page. As a result, you can make the card responsive without media queries.

  I have two recommended videos. The first one is about how hard HTML is (HTML is not easy). The second one is talking about HTML and modern CSS.

  • Manuel Matuzović - Lost in Translation - YouTube
  • Andy Bell – Be the browser’s mentor, not its micromanager - YouTube

  I hope this helps! Happy coding!

  Marked as helpful

  1

  ---

  

  Arnav Luhadiya•20

  @arnav-luhadiyaPosted about 2 years ago

  @vanzasetia Thank you for taking your time out to check my code and give these suggestions. This helps a lot.

  0

  

  Vanza Setia•27,775

  @vanzasetiaPosted about 2 years ago

  @arnav-luhadiya No problem! 👍

  0
• 

  Lucas 👾•104,400

  @correlucasPosted about 2 years ago

  👾Hello @arnav-luhadiya, Congratulations on completing this challenge!

  Great code and great solution! I’ve few suggestions for you that you can consider adding to your code:

  The background wave image is missing and here’s the step-by-step to add it.First of all add the image as a background inside the body this is the code for that: background-image: url(../images/pattern-background-desktop.svg);

  Then you add background-repeat: no-repeat to avoid the background repeating and background-size: contain to make it fit full width and center with the card this is the best choice, but an alternative to resize it is to use background-size: 125%, Here’s the code with the modification and the image applied as background:

  body {
      background-size: contain;
      background-image: url(./images/pattern-background-desktop.svg);
      background-repeat: no-repeat;
  }
  

  ✌️ I hope this helps you and happy coding!

  Marked as helpful

  1

  ---

  

  Arnav Luhadiya•20

  @arnav-luhadiyaPosted about 2 years ago

  @correlucas Thank you Lucas. I did not realize that the background was provided to us. I thought I had to use some kind of linear gradient to make it work but could not, hence dropped the idea. Thank you for pointing it out to me.

  0
• 

  Mauger•210

  @mauger1998Posted about 2 years ago

  Hey great job, if this is your first challenge then your doing so well by the looks of things! I would like to give you a bit of feedback after looking at your code. First thing I would say is when importing fonts from google, dont use @import in your css instead use the <link and put it in your html, importing things in css can slow things down. Apart from that youve done an amazing job well done. If you want go check out other peoples code and how they did it.

  Marked as helpful

  1

  ---

  

  Arnav Luhadiya•20

  @arnav-luhadiyaPosted about 2 years ago

  @mauger1998 Thank you for the suggestion. Did not know that using @import slows down the page. Thank you for telling me this :)

  0