QR code component

Community feedback

• 

  PhoenixDev22•16,950

  @PhoenixDev22Posted over 2 years ago

  Greeting @abuk111,

  I have few suggestions regarding your solution:

  • There should be two landmark components as children of the body element - a main (which will be the component ) and a footer (which will be the attribution).<Footer> should not be in the <main >. HTML5 landmark elements are used to improve navigation .

  • Page should contain a level-one heading . As this is not a real webpage , you can use <h1> with class="sr-only" (Hidden visually, but present for assistive tech).

  • The alternative text is needed on that image for assistive tech and SEO search engine optimization . So it needs to contain a description like Qr code for frontend mentor .

  • Usingmin-height: 100vhinstead ofheight: 100vh; allows the body to to grow taller if the content outgrows the visible page.

  • height: 500px; , or height: 180px;It's rarely ever a good practice to set heights on elements, you almost never want to set it . let the content define the height .

  • width: 320px;an explicit width is not a good way . consider using max-width to card instead and a little margin to the card .

  • You should use em and rem units .Both em and rem are flexible, Using px won't allow the user to control the font size based on their needs.

  • It's recommended not to use px for font-size. you can use rem instead .

  Overall , your solution is good .Hopefully this feedback helps.

  Marked as helpful

  1

  ---

  

  Shashree Samuel•9,260

  @shashreesamuelPosted over 2 years ago

  @PhoenixDev22 I agree with your suggestions however I think that for the last point it needs to be mentioned additionally that rem means responsive em hence if a user has certain preferences in terms of the font size then rem just better adapts to that preference.

  1