Responsive QR-code-component

Community feedback

• 

  P

  Øystein Håberg•10,350

  @Islandstone89Posted 12 months ago

  Hi, you have done a decent job on this challenge. Here is my feedback:

  HTML:

  • Change .container from a <div> to a <main>. Main elements are important for accessibility.

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

  • Remove the .attribution div, and apply .attribution to the <footer>.

  • Footer text must be wrapped in a <p>.

  CSS:

  • It's good practice to include a CSS Reset at the top. The first thing it does is to remove the margin and padding, and add box-sizing: border-box;, just like you have. Note, it should be 0, not 0%.

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

  • max-width on .container should also be in rem.

  • `Change width: 100% to max-width: 100% on img. It should also have a display: block.

  • Remove align-items: center on <h1>. It doesn't do anything without display: flex. And you wouldn't put display: flex on a <h1> anyway, because it doesn't have any children.

  • Paragraphs are full-width by default, so you don't need the width: 100%.

  • It's more efficient to set text-align: center on the body. The result will be the same, it's just a bit more elegant.

  Good luck :)

  0