FrontendMentor-QR code component-1

Community feedback

• 

  P

  Øystein Håberg•10,710

  @Islandstone89Posted 12 months ago

  Hi, here is some feedback.

  HTML:

  • .container should not be a <div>, but a <main>.

  • I would simplify your structure, by removing the divs around the <h1> and the <img>. In this simple project, you don't really need them.

  • For decorative images (like icons, illustrations, etc), the alt text should be empty. However, in meaningful images, like this one, it must not be empty. It should describe the content of the image, and in this example, it also needs to tell where it leads (frontendmentor.io).

  • Remove the <br>. Spacing between elements should be done in CSS.

  • .attribution should be a <footer>.

  • The text in the footer must be wrapped in a <p>.

  CSS:

  • CSS should be written in a separate file, often called "style.css", and not be written in the HTML.

  • It's good practice to start the stylesheet with a CSS Reset. Andy Bell has a good one to use.

  • The card needs to be centered horizontally and vertically. An easy way is to use Flexbox on the body:

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

  • Remove the fixed width on the card. Setting fixed dimensions can easily create all kinds of trouble with responsiveness. You should, however, set a max-width in rem, to prevent the card from getting too big at larger screens.

  • Remove also height, margin-left and margin-right.

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

  • text-align: center should only be on the body. The result will be the same but with fewer lines of code.

  • Change width on the image to max-width: 100%, and also add display: block.

  Marked as helpful

  0