QR-Code

Community feedback

• 

  P

  Øystein Håberg•10,710

  @Islandstone89Posted 11 months ago

  HTML:

  • Every webpage needs a <main> that wraps all of the content, except for <header> and footer>. This is vital for accessibility, as it helps screen readers identify the "main" section of a page. Change the container div into a <main>, and give it a class instead of an ID.

  • I would remove the rest of the divs, as they are not needed.

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

  • .attribution should be a <footer>.

  • The footer text and the"Scan your" need to be wrapped in a <p>.

  CSS:

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

  • On body, change height to min-height - this way, the content will not get cut off if it grows beneath the viewport. Add flex-direction-column, and a gap of a couple of rem, to create some space between the <main> and the <footer>.

  • Remove all widths and heights, both in px and %. Except for icons, you rarely want to set fixed dimensions(especially heights), as this easily creates issues with responsiveness.

  • You don't need margin: auto on the card since it is centered using Flexbox.

  • Remove margin from the image - to create the space between the image and the edge of the card, set padding on all 4 sides of the card.

  • Add display: block and max-width: 100% on the image - the max-width prevents it from overflowing its container.

  • Remove positioning: relative and its corresponding properties.

  Marked as helpful

  0

  ---

  

  Ahmed Mostafa•140

  @Ahmed-Mustafa132Posted 11 months ago

  Hi @Islandstone89 Thank you for your feedback I modified my solution and You can take a look to see if this solution is correct or not

  1