QR Code Component

Community feedback

• 

  P

  Øystein Håberg•10,710

  @Islandstone89Posted 11 months ago

  Hi there, good job! Well done for using appropriate landmark elements like <main> and <footer>, as well as using Custom CSS Properties.

  Here are my suggestions for improvement.

  HTML:

  • I like that you have a very simple structure, without any unnecessary divs. Since you are using <main> as the card, I would probably give it a class, just for clarity.

  • The image must have alt text. This is essential for screen readers to understand the image. The alt text should be descriptive, and in this example, it also needs to say where it leads (frontendmentor.io).

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

  CSS:

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

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

  • You don't need display: block on main as this is its default value.

  • Remove all of the fixed widths. As we want our components to be responsive, it's not a good idea to set fixed dimensions.

  • To prevent the card from getting too big on larger screens, give it a max-width of around 20rem.

  • Instead of using margin on the card itself, you can center it horizontally and vertically by using Flexbox. To do this, you select the card's parent element, which is the body:

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

  • In this challenge, all of the text should be centered. Hence, instead of setting text-align: center on each of the individual parts, you can set it only once, on the body. You get the same result but with less code - the children of the body inherit the values from its parent (look up inheritance in CSS for more info).

  • The image should have a display: block and a max-width: 100% instead of width.

  • Finally, you can remove the media query, as there is no need for it on this challenge. Keep in mind, that when you do need media queries, it's considered best practice to do the mobile styling as default, and use media queries for larger screens. They must also be in rem instead of px.

  Good luck :)

  Marked as helpful

  0

  ---

  

  Fernanda Tomita Manchon•120

  @fernandatmPosted 11 months ago

  Hello @Islandstone89 !

  Thank you for the great report, you gave me a lot to study!

  I applied this changes in this project, now I'm going to study more and practice on the other projects I've already started.

  Thanks a lot :)

  1

  

  P

  Øystein Håberg•10,710

  @Islandstone89Posted 11 months ago

  @fernandatm :)

  0