Responsive QR code component

Community feedback

• 

  P

  Øystein Håberg•10,710

  @Islandstone89Posted 29 days 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 a page's "main" section. Wrap the card in a <main>.

  • Do not include words like "image" in the alt text. Screen readers start announcing images with "image", so an alt text of "QR code image" would be read like this: "image, QR code image". Also, the alt text must say where it leads(the frontendmentor website). A good alt text would be "QR code leading to the Frontend Mentor website."

  • .attribution should be a <footer>, and you should use <p> for the text inside. Make sure the <footer> is outside of the main - they should both be direct children of the body.

  CSS:

  • Including a CSS Reset at the top is good practice.

  • I recommend adding a bit of padding, for example 16px, on the body, to ensure the card doesn't touch the edges on small screens.

  • Remove max-width and min-width on body, as none are needed.

  • To center the card horizontally and vertically, with space between <main> and <footer>, I would use Flexbox on the body:

  display: flex;
  flex-direction: column;
  justify-content: center;
  align-items: center;
  min-height: 100svh;
  gap: 2rem;
  

  • Remove the margin on the card - it is already centered using Flexbox.

  • Remove the width and height in px on the card. We rarely want to give a component a fixed size, as we want it to grow and shrink according to the screen size.

  • We do want to limit the width of the card, so it doesn't get too wide on larger screens. To solve this issue, give the card a max-width of around 20rem.

  • font-size must never be in px. This is a big accessibility issue, as it prevents the font size from scaling with the user's default setting in the browser. Use rem instead.

  • Setting the font-size to 10px makes the text hard to read, so adjust it to a higher number.

  • Paragraphs have a default value of font-weight: 400, so there is no need to declare it.

  • Remove text-align: center on .attribution- since you have set text to be centered on the body, all of its descendants will inherit the value.

  • On the image, remove the fixed width in px. Instead, change display: inline-block to display: block and add max-width: 100% - the max-width prevents it from overflowing its container. Without this, an image would overflow if its intrinsic size is wider than the container. max-width: 100% makes the image shrink to fit inside its container. Remove margin-top and justify-self - the latter property needs a display: flex to work, which is not needed here. I would also increase the border-radius a bit.

  • To create the space between the image and the edge of the card, set padding on all 4 sides of the card:padding: 16px;.

  Marked as helpful

  1

  ---

  

  Ahmed Nafrawy•150

  @Ahmed-NafrawyPosted 29 days ago

  I didn't expect this response! Ouh than you so much for your insights and effort, i will do as you said and I will reupload it .

  You really don't know how thankful I am! And I wish you do this in any project I post, as I am learning right now and it would be much of a help if I listened to some professional like you

  Thank you again 💓 @Islandstone89

  1

  

  P

  Øystein Håberg•10,710

  @Islandstone89Posted 29 days ago

  @Ahmed-Nafrawy Glad to help :) You don't have to re-upload the solution, you only need to update your repo.

  0

  

  Ahmed Nafrawy•150

  @Ahmed-NafrawyPosted 29 days ago

  Yes I understand 🙏@Islandstone89

  1