Qr code component using HTML and CSS

Community feedback

• 

  Travolgi 🍕•31,400

  @denieldenPosted about 2 years ago

  Hello Fico, You have done a good work! 😁

  Some little tips to improve your code:

  • use main tag to wrap the card and improve the Accessibility but not as a container of that one element
  • also you can use article tag instead of a simple div to the container card for improve the Accessibility
  • use h1 for title of card and not a simple p
  • remove all margin from main tag
  • use flexbox to the body to center the card. Read here -> best flex guide
  • after, add min-height: 100vh to body because Flexbox aligns child items to the size of the parent container
  • instead of using px use relative units of measurement like rem -> read here

  Keep learning how to code with your amazing solutions to challenges.

  Hope this help 😉 and Happy coding!

  Marked as helpful

  0
• 

  Lucas 👾•104,340

  @correlucasPosted about 2 years ago

  👾Hi @fico444, congratulations on your solution!

  Great solution and a great start! From what I saw you’re on the right track. I’ve few suggestions for you that you can consider adding to your code:

  To maintain the card responsive use max-width instead of width this way you allow the content to be flexible. The difference between max-width and width is that width is fixed and max-width has a maximum size but can shrink to fit the content.

  Replace the <p> containing the main title with <h1> note that this title is the main heading for this page and every page needs one h1 to show which is the most important heading. Use the sequence h1 h2 h3 h4 h5 to show the hierarchy of your titles in the level of importance, never jump a level.

  Clean your code by removing some unnecessary divs, most of the content can stand alone without a div. Use div only for blocks that need a special alignment or the content needs a special positioning.

  Add a margin of around margin: 20px to avoid the card touching the screen edges while it scales down.

  Use relative units as rem or em instead of px to improve your performance by resizing fonts between different screens and devices. These units are better to make your website more accessible. REM does not just apply to font size, but to all sizes as well.

  Here's my solution for this challenge if you wants to see how I build it: https://www.frontendmentor.io/solutions/qr-code-component-vanilla-cs-js-darklight-mode-nS2aOYYsJR

  ✌️ I hope this helps you and happy coding!

  Marked as helpful

  0
• 

  Vanza Setia•27,775

  @vanzasetiaPosted about 2 years ago

  Hi there! 👋

  Congratulations on completing your first Frontend Mentor challenge! 🎉 Good job on wrapping all the page content with the correct landmark elements! 👏`

  Here are some suggestions for improvements.

  • I recommend making the first paragraph a h1 instead. For your information, each page needs one h1. You can think of the h1 as the title on the document. So, it is important to have it.
  • I recommend adding rel="noopener" to any anchor tags that have target="_blank". It helps protect users of legacy browsers. I suggest reading "Links to cross-origin destinations are unsafe" article to learn more about this.
  • To place the card in the center of the page, I recommend using flexbox or grid. You can use the body element as the container of the card element. This method will make sure the card is centered regardless of the user's screen size.
  • Use rem or sometimes em unit instead of px. Using px will not allow the users to control the size of the page based on their needs. I recommend reading this article about rem and em unit. This article explains both units in a simple way.

  That's it! I hope this helps! Happy coding! 🙌

  Marked as helpful

  0