qr code component

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 .container into a <main>.

  • 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).

  • "Improve your" is a heading, not a paragraph. "Scan the QR code" on the other hand, is a paragraph, and must be a <p>, not a <span>.

  • .attribution should be a <footer>, and its text must be wrapped in a <p>.

  CSS:

  • You don't need to put anything on html, only the body. I would move all the properties on .container over on the body, and change height to min-height - this way, the content will not get cut off if it grows beneath the viewport. Also, add flex-direction: column and gap: 2rem.

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

  • The card should not have a width, only a max-width of around 20rem, to prevent it from getting too wide on larger screens.

  • On the image, add display: block and change width to max-width: 100% - the max-width prevents it from overflowing its container.

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

  • As the design doesn't change, there is no need for any media queries. When you do need them, they should be in rem, not px. Also, it is common practice to do mobile styles first and use media queries for larger screens.

  Marked as helpful

  1

  ---

  

  Abdelrahman Anwar•10

  @abdelrahmenPosted 11 months ago

  @Islandstone89 Wow, thank you so much for your precious observations, i will update the solution as per your points and surely use them in my future code.

  1

  

  Abdelrahman Anwar•10

  @abdelrahmenPosted 11 months ago

  @Islandstone89 hey, I did all of your points, plus, I changed height: 100% in the body to min-height:100vh, when it was 100% it didn't take the full height and i do not understand why, if you know please tell me, also i have another question, when the width of the screen falls under 150 px, the card overflows to the left, how can I fix that?

  0

  

  P

  Øystein Håberg•10,710

  @Islandstone89Posted 11 months ago

  @abdelrahmen no screens are less than 150px wide, so that's not a problem.

  0
• 

  Matthew Sperry•190

  @sperrowPosted 11 months ago

  Looks great on desktop. To design for a mobile layout, you can use Chrome/Safari/Firefox's device emulator to display your page how it would look on different screen sizes.

  1

  ---

  

  Abdelrahman Anwar•10

  @abdelrahmenPosted 11 months ago

  @sperrow thank you, I didn't test it on a Real mobile phone, I just toke the dimensions specified in the design guide and went for it.

  1

  

  Matthew Sperry•190

  @sperrowPosted 11 months ago

  @abdelrahmen makes sense, just fyi if you know how to use Chrome's dev tools/inspector it's really easy to switch the screen size so you can see what it'd look like without having to use an actual phone: https://developer.chrome.com/docs/devtools/device-mode

  0

  

  Abdelrahman Anwar•10

  @abdelrahmenPosted 11 months ago

  @sperrow that's what i did to make it work on a width of 375px as specified in the design guideline, but surely it needs more editing to work on other screen sizes, as most phones nowadays are wider than this and won't trigger the current media query.

  1

  

  Matthew Sperry•190

  @sperrowPosted 11 months ago

  @abdelrahmen ahh I see haha, sorry I misunderstood. Perhaps you could just change your media query to a wider breakpoint (e.g. 768 instead of 375) so that it will still look good at 375px but also good on wider phones/tablets.

  0