HTML and CSS

Community feedback

• 

  T

  Grace•31,230

  @grace-snowPosted almost 3 years ago

  This needs quite a few changes I’m afraid. All quite easy fixes and important to learn

  • you need to use landmark elements (eg main and footer)
  • the attribution (challenge by…) should be within the footer, not in the card
  • it’s invalid to have paragraphs inside heading elements
  • there should only be one heading in this. Even if there were more headings, they would have to go in order
  • the alt text on the wrong code img is extremely important. The image is of a QR code, not ERROR.
  • font size must never be in px. Use rem so it scales properly
  • you shouldn’t need a media query on this. It’s actually making the component look broken on my mobile. If you want to stop component hitting screen edges on small screens make sure there is either a little margin on the card, or padding on the wrapper (eg body in this case)
  • similar to not relying for viewport units for sizing, it’s best not to use % for things like padding. You need to have control of the layout, but don’t know what % will always equate to. Better to use rem (or px if you’re not comfortable with using rem yet)

  I hope this helps

  Marked as helpful

  0

  ---

  

  Nerdytoo•30

  @olalemiPosted almost 3 years ago

  @grace-snow Thanks for your feedback, really helpful, will make sure I affect the changes

  0
• 

  LiBee•370

  @Li-BeePosted almost 3 years ago

  Looks good - couple of comments.

  • You need to add border radius to the card element
  • I would take out the <attribute> and put outside your card element
  • for you accessiblity issues report if you add <main> to the body that should help with the landmark issue.

  Marked as helpful

  0

  ---

  

  Nerdytoo•30

  @olalemiPosted almost 3 years ago

  @Li-Bee Thanks for your feedback, much appreciated

  0