Order-Summary-Component-Main

Community feedback

• 

  Melvin Aguilar 🧑🏻‍💻•61,140

  @MelvinAguilarPosted over 1 year ago

  Hello there 👋. Good job on completing the challenge !

  I have some suggestions about your code that might interest you.

  HTML 🏷️:

  • The <picture> tag can be useful when you need to provide different images for different viewports, but in this specific challenge, it's not necessary. You can directly use the <img> tag to include the image in your solution.

  • The <article> tag is used to mark up self-contained, independent content that makes sense on its own and can be syndicated or shared on its own. This allows the content to be understood by users even when it is viewed outside of the context of the original website.

    In summary, in your solution there is no need to add any <article> tag.

  CSS 🎨:

  • Using both min-width and max-width with the same value doesn't make sense, there is no value between those values, so it is the same as using width. The only one that should exist is max-width and not both.

  • Use min-height: 100vh instead of height. Setting the height to 100vh may result in the component being cut off on smaller screens, such as a mobile phone in landscape orientation.

  CSS Reset 🔄:

  • You should use a CSS reset. A CSS reset is a set of CSS rules that are applied to a webpage in order to remove the default styling of different browsers.

    CSS resets that are widely used:

    • Reset CSS
    • "My Custom CSS Reset" by JoshWComeau

  I hope you find it useful! 😄 Above all, the solution you submitted is great!

  Happy coding!

  Marked as helpful

  1

  ---

  

  MortalCoder•180

  @mortalcodePosted over 1 year ago

  @MelvinAguilar Thankyou so much for taking time, going through my code and suggesting these amazing points. really helpful!

  0
• 

  Hassia Issah•50,650

  @HassiaiPosted over 1 year ago

  Change the value of the background-size to contain instead of cover.

  To center .main-body on the page using grid, replace the height in the body with min-height:100vh and align-items with align-content. for more on thisClick here

  For a responsive content, there is no need for min-width in .main-body.

  In the media query you only have to change the background-image of the body.

  Hope am helpful.

  Well done for completing this challenge. HAPPY CODING

  Marked as helpful

  1

  ---

  

  MortalCoder•180

  @mortalcodePosted over 1 year ago

  @Hassiai I'm grateful for you time and effort, really helpful! Thankyou

  1