Compact QR Code Challenge

Community feedback

• 

  Stephen Yu•150

  @StephenYu2018

  Posted about 2 years ago

  I liked the sizing of the component, image and texts. I'd also like to give a few suggestions for what you can do in the future:

  • alt attribute of <img> represents alternative text that is displayed in place of the image when the browser cannot display that image for some reason/error. A better and more descriptive alt text for the <img> would be alt="QR code to visit Frontend Mentor"
  • Prefer using rem measurements instead of px measurements for sizing. rem calculates its measurement off the font size of the root element (which is html). The font size of the root element is dictated by the browser and can be changed by the user. While the default is 16px on most browsers, users can choose to make it larger, possibly because most text would be too small to read. By sticking with px measurements, you remove that capability from the user, as the text won't scale larger when the user chooses larger font sizes. Instead the text will remain the same size.
    • Also, do not style html font size using px to change how the root font size appears. That also removes that feature from the user as well. If you want the root font size of your page to appear at a different size:
      • Use percentage units %
      • Divide your apparent root font size with the actual default root font size
      • Example: If I wanted my root font size to appear at 13px instead of 16px, I would do the following calculation: 13px / 16px = 0.8125 = 81.25%
      • The final CSS property would be html { font-size: 81.25%; }

  23

  Marked as helpful
• 

  visualdennis•8,375

  @visualdenniss

  Posted about 2 years ago

  Hello there,

  good work overall. Congrats.

  • height: auto; is kinda redundant as it is the default value. E.g. you gave height: auto; for the img.

  • Also avoid fixed heights like height: calc(100vh - 40px); better would be to use min-height approach.

  Hope you find this feedback helpful!

  10

  Marked as helpful
• 

  Ecem Gokdogan•9,380

  @ecemgo

  Posted about 2 years ago

  Some recommendations regarding your code that could be of interest to you.

  In order to fix the accessibility issues:

  • You need to replace <div class="container"> with the <main> tag and <div class="attribution"> with the <footer> tag. You'd better use Semantic HTML, and you can also reach more information about it from Using Semantic HTML Tags Correctly.
  • Each main content needs to start with an h1 element. Your accessibility report states page should contain a level-one heading. So, you need to use a <h1> element in the <main> tag instead of using <h3>. You can replace your <h3>Improve your front-end skills by building projects</h3> element with the <h1>Improve your front-end skills by building projects</h1> element.

  Hope I am helpful. :)

  10

  Marked as helpful