Order-Summary-Component

Community feedback

• 

  DJ Drakos•150

  @dj-drakosPosted almost 2 years ago

  Hey Carlos, great work on this. I have a couple of notes for you to take this to the next level.

  • Most important: Check out the accessibility report and fix the issues in your code that are causing warnings.
  • In the mockup, the background covers the entire viewport. Try working with the background-size property.
  • Pay close attention to the color and direction of the shadows on the card and the button.
  • The Annual Plan option should have a background.
  • Your solution looks pretty small compared to the design mockup. Consider making the desktop version larger.

  Overall, great work! Nice work using pseudoclasses and nested element selectors.

  Marked as helpful

  1
• 

  Adriano•38,390

  @AdrianoEscarabotePosted almost 2 years ago

  Hi Carlos, how are you? I really liked the result of your project, but I have some tips that I think you will enjoy:

  To improve the structure of the html document, prefer to wrap all the content with the main tag, since this challenge is just a component, it needs a main tag to know what the main content of the page is!

  To prevent the background image from breaking at higher resolutions, we can prevent this in two different ways:

  1. Add a background-repeat: repeat-x;, the image will repeat on the horizontal axis, preventing it from breaking.

  2. Add a background-size: 100% 50vmin;, the 50vmin will set its height as the page target, and 100% will make it stretch on the horizontal axis.

  Feel free to choose one of the two!

  You have used <br> , using <br> is not only bad practice, it is problematic for people who navigate with the aid of screen reading technology. Screen readers may announce the presence of the element. This can be a confusing and frustrating experience for the person using the screen reader. You can read more in MDN.

  The rest is great!

  I hope it helps... 👍

  Marked as helpful

  1