QR code component

Community feedback

• 

  PhoenixDev22•16,950

  @PhoenixDev22Posted over 2 years ago

  Hello Rajesh Janyani,

  Congratulation on completing another frontend mentor challenge.You did great work. Regarding your solution , I have some suggestions:

  HTML

  • Page should contain a level-one heading. In this challenge (as it's not a whole page ), you can<h1> visually hidden and use <h2> instead <h3> . Bear in mind , always use the headers on their chronological order.

  • Alternate text should be human readable(Not hyphenated).

  CSS

  • To center the component on the page , use flex/grid properties to the body and add min-height: 100vh to let the the body grows taller if the content outgrows the visible page (remove the large margin off the main.)

  • In order to reduce the specificity , the better way to style the elements is a single class.

  • width: 300px; an explicit width is not a good way . consider using max-width in rem instead and a little margin to the card .That will let it shrink a little when it needs to.

  • It rarely a good practice to set the height of the component. let the content dictate the height.

  It's not recommended to use px for font size. If the user tries to resize the text through his browser, any element with an absolute font size will not change, although any other element will. This prevents users from making your page's text larger if they need to see it.

  • Bear in mind whenever you use padding Percentage that is Based on the Parent Element’s Width.

  Overall , your solution looks good.

  I hope these are helpful learning points.

  Marked as helpful

  0

  ---

  

  Rajesh Janyani•330

  @Rajesh7rjPosted over 2 years ago

  @PhoenixDev22 thanks 🙏 for your kind words and valuable feedback. 👍

  1

  

  PhoenixDev22•16,950

  @PhoenixDev22Posted over 2 years ago

  @Rajesh7rj Glad it was helpful .

  0