Order Summary

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted about 3 years ago

  Hey, awesome work on this one. Layout in general looks larger than the design, toning it down would be really great.

  You already got some great feedback, just going to add some as well:

  • Avoid using height property with a fixed vh unit. This makes the element's height limited to the remaining screen/viewport's height. On the main tag, you don't need to use the height property, you could replace it with min-height: 100vh, this takes full height and will expand if it needs to. Also, you don't need to use the width: 50% on it, let the content inside it create the sizes.
  • The .card selector, instead of using width: 500px on it, use width: 100% and max-width: 500px, this way, you are limiting the size of the card plus you are making it respond to screen-size changes.
  • Also, when using alt attribute, avoid using words that relates to "graphic" such as "illustration, image, icon..", it is already an image so no need to describe it as one.
  • Music-icon should have used alt="" and aria-hidden="true".
  • annual-plan could have used a heading tag.

  Apart from those, great work again on this one.

  Marked as helpful

  0
• 

  Muhammad Shajjar•1,100

  @muhammadshajjarPosted about 3 years ago

  Hey! congrats on completing your first FEM challenge, below their some suggestions to improve your code.

  • By Giving card a width cause horizontal scroll bar, avoid using width and height try instead max-width and max-height (if needed), using hard code cause responsiveness error
  • proceed to payment and cancel implies navigation so should probably be a anchor tags.
  • Your images should have alt attribute but for decorative images, you should leave alt as blank alt="" and use role=" presentation" or aria-hidden=" true" to make sure all screen readers ignore those images. In your case, the music icon and your hero image are decorative.
  • Need to work on naming conventions

  Marked as helpful

  0