QR code

Community feedback

• 

  P

  Carlos Castillo•360

  @CharlieCastleWebPosted about 2 years ago

  Hi Nenad! Good work!

  Just a few tips:

  • Instead of using <div id="main-div"> you can use just the <main> tag.

  • Instead of using the h2 tag, I think you should use an h1 tag.

  • According to the design, the card should be centered horizontally and vertically. To do that I use flexbox. The following code should center everything inside the body:

  `html, body { height: 100%; }

  body { display: flex; align-items: center; justify-content: center; }`

  Let me know if you have any doubts or I can help you with other questions!

  Marked as helpful

  1
• 

  Lucas 👾•104,400

  @correlucasPosted about 2 years ago

  👾Hi @nenadmne, congratulations on your solution!👋 Welcome to the Frontend Mentor Coding Community!

  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:

  • Don’t use id to give the style of your elements, it's not a good idea because id is a too specific selector used for forms and Javascript code. Instead, use class for styling and let the id for much specific stuff. It's also not advisable to use IDs as CSS selectors because if another element in the page uses the same/similar style, you would have to write the same CSS again. Even if you don't have more than one element with that style right now, it might come later.
  • Use <main> instead of a simple <div> this way you improve the semantics and accessibility showing which is the main block of content on this page. Remember that every page should have a <main> block and that <div> doesn't have any semantic meaning.
  • Replace the <h2> 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.
  • 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

  1