QR code component

Community feedback

• 

  Mohammed Fakih•1,590

  @javascriptor1Posted 11 months ago

  Hello,

  Congratulations for completing this challenge. Here is my feedback on your solution which I hope it will help improve your code:

  HTML

  1- Use semantic tags always to reflect element's content meaning (like <main> <footer>).
  Avoid non-semantic elements like <div> <span>. Use these two elements for grouping/styling purposes.

  Apply this in your code by replacing the parent div with main tag. Same for footer.

  2- Start with h1 tag instead of h2 as its level 1 heading to make your age accessible

  3- Enhance the alt text for the QR code. Instead of alt="QRcode", you can write a more descriptive text like : QR code for frontend mentor website.

  4- wrap QR abbr in the paragraph with <abbr> tag like this:

  <p>Scan the <abbr title="Quick Response">QR</abbr> code to visit Frontend Mentor and take your coding skills to the
  next level</p>
  

  CSS

  5- The required font is Outfit but you have included three more fonts by mistake I guess:

  <link href="https://fonts.googleapis.com/css2?family=Fraunces:opsz,[email protected],400;9..144,500;9..144,600;9..144,700&family=Montserrat:wght@200;300;400;500&family=Outfit:wght@400;700&family=Overpass:wght@400;700&display=swap" rel="stylesheet">

  This usually occurs when you add a font in google website and don't remove it after you finish. When you visit google font website next time , your old selection will stay there and will be added to the fonts you add recently.

  6- As per style guide, font-size should be 15px which was not applied. Changing to 15px will make the page look like design in terms of Paragraph.

  7- You can remove width:300px from h2 tag - it has no effect.

  8- Instead of adding a style which can be inherited for each child element,apply the style to the parent instead:

  For example , you want to align h2 and p text to center. Instead of applying styling to each tag , do it once for Child div which is parent:

  text-align: center;

  text-align is a property that gets inherited to child elements.

  9- Avoid capitalizing class names -> parent instead of Parent.

  10- Finally, increase the padding in child container div.

  Keep coding and best of luck.

  MKF

  2

  ---

  

  Che Fernandez•170

  @CheFernandez99Posted 11 months ago

  @javascriptor1 hey thanks for the feedback, the width on paragraph did make it shorter it fit in the box that way, but yeah a div around that would probably have done the same thing.

  0
• 

  Daniel 🛸•44,250

  @danielmrz-devPosted 11 months ago

  Hello @CheFernandez99!

  Your project looks great!

  I have just one minor suggestion for you to improve it:

  • Consider replacing your div.parent with main.parent and your h2 with h1. This won't change anything visually, but it makes your HTML code more semantic and improves SEO optimizantion as well as the accessibility of your project.

  I hope it helps!

  Other than that, you did an excelent job!

  2