QR-code component using HTML and CSS

Community feedback

• 

  P

  Øystein Håberg•10,710

  @Islandstone89Posted 12 months ago

  Hi, here is some feedback:

  HTML:

  • You need to have a <main>, this is important for accessibility. Change .container from a div to a main.

  • Alt text also needs to say where it leads(frontendmentor.io).

  • .attribution should be a <footer>.

  • Footer text needs to be wrapped in a <p>.

  CSS:

  • It's good practice to include a CSS Reset at the top.

  • On body, remove width: 100% - as body is a block element, it takes up 100% of the width by default.

  • Remove position: absolute, top, left, and transform - you don't need any of these in this challenge. And you're already centering the card using Grid.

  • To center the card vertically, you do need to add min-height: 100vh to the body. As mentioned, it is 100% wide by default, but it does not fill the viewport height by default - instead, it gets its height from its content. So, it doesn't have any available space to center the card in, unless we give it a min-height of 100vh.

  • Remove the fixed width and height on the card. You rarely want to set fixed dimensions! Remember, we want our pages to be responsive, meaning they can adapt to different screen sizes. Setting fixed dimensions prevents this, and causes issues like overflow.

  • Remove the width and height of the image. You only need to set display: block and max-width: 100%, to prevent the image from overflowing its container.

  • You can also remove the margin - you're using padding on the card to get the space between the image and the edge of the card.

  • Font-size must never be in px. Use rem instead.

  • All the text is centered, so put text-align: center on the body, and remove it elsewhere. The result will be the same, as the children will inherit the value. But you are declaring text-align only once, which is more efficient.

  Hope this is helpful - good luck!

  Marked as helpful

  1

  ---

  

  Chyka•60

  @CrownedTechiePosted 12 months ago

  Thank you so much for this🧎🏾‍♀️🧎🏾‍♀️🧎🏾‍♀️.... you're highly appreciated🥺❤️. I didn't know it was necessary in HTML to have landmark elements About the responsiveness, what do you think? I didn't use a media query, is that bad? @Islandstone89

  1

  

  P

  Øystein Håberg•10,710

  @Islandstone89Posted 12 months ago

  @CrownedTechie happy to help :)

  Yes, landmarks are important for screen reader navigation.

  This card should have the same design on all screens, so you don't need a media query on this challenge.

  Marked as helpful

  1