QR code component

Community feedback

• 

  P

  Øystein Håberg•10,390

  @Islandstone89Posted about 2 months ago

  Hi, you've done a great job!

  Some of the things I like about your solution:

  • Using the <main> semantic element
  • Using Custom Properties
  • Font sizes in rem instead of px, which is vital for accessibility
  • No fixed widths or heights in px.
  • max-width in rem on the card

  A few suggestions:

  HTML:

  • <main> holds all of the main content on a page. As a card would likely not be the only component on a page, I would wrap the card content in a <div class="card"> inside of <main>.

  • The alt text must also say where it leads(the frontendmentor website). A good alt text would be "QR code leading to the Frontend Mentor website."

  CSS:

  • Including a CSS Reset at the top is good practice.

  • I would recommend adding 1rem of padding on the body, to ensure the card doesn't touch the edges on small screens.

  • On the body, change height to min-height - this way, the content will not get cut off if it grows beneath the viewport.

  • An alternative way to center the card, is to use Flexbox on the body:

  display: flex;
  flex-direction: column;
  justify-content: center;
  align-items: center;
  min-height: 100svh;
  

  • Instead of main p and main h2, I would give the elements a class and use only that as the selector. Descendant selectors like main p have higher specificity than a single class selector, meaning they are harder to override.

  • Move the styles on main to the <div> you created that holds the card content.

  • letter-spacing must not be in px. You can use em, where 1em equals the element's font size.

  • It's common to add display: block on images - the default display: inline can cause unwanted space under an image - if an image is wrapped in a <div>, it sometimes does not fill the entire height of that <div>.

  Well done, and keep up the good work :)

  Marked as helpful

  1
• 

  P

  Steven Stroud•11,870

  @StroudyPosted about 2 months ago

  Hey, Your CSS is great, Impressed at this stage! Great job with this solution you should be proud, A few things I noticed,

  • Missing a <meta> description tag for SEO purposes,
  • Setting a height and width attribute to your <img> will increase performance to reduce layout shifts and improve CLS, It reserves the space on the page for the image,
  • Having better alt="" descriptions for accessibility is a must check this out Write helpful Alt Text to describe images,
  • You should apply a full modern reset to make things easier as you build, check out this site for a Full modern reset

  I hope you found some of this information helpful, You should give the articles a good read and I look forward to seeing some more from you, Happy coding! 💻

  Marked as helpful

  1