Qr code component

Community feedback

• 

  Masseh•260

  @Masseh2025

  Posted 12 months ago

  By the way I noticed that you used alot of ids in your code, I think you would be fine just using class names for the most part unless you really need them and remove the unnecessary "child" classes and try to make the names a little more descriptive. For exmaple instead of naming your img "img" I would name it qr-code-img.

  01

  Marked as helpful
• 

  

  Grace•32,130

  @grace-snow

  Posted 11 months ago

  I hope this feedback is helpful

  1. Don't set all those styles on the HTML element. You will hardly ever have to touch the html selector in your styles. The html doesn't need to be a flex element or styled at all, and the height: 100%; is causing breakage -- a really nasty overflow bug for people who need to zoom text or use a larger text size. Remove all those styles from html and leave them on the body.
  2. The body needs a min height in viewport units. Default the body is only as tall as its content which means in this case because the content is only a small card the height of the body is very small. That's why you need to give it a min height. Tell it to be at least as tall as the viewport e.g. min-height: 100svh.
  3. All content should be contained within landmarks. This needs a main wrapping the card and a footer for the attribution. Every page should at least have a main Element and there should only ever be one of them.
  4. Avoid using keywords like Min or max content. These are advanced topics and they won't work how you expect them to work. This card does not need it. The card needs a single max width in rem.
  5. To stop the Page contents from hitting the screen edges you either need to give that content a small margin on all sides or you need to give a wrapping element some small padding on all sides. For example in this case you could add a little padding to the body or to the landmarks.
  6. Make sure you understand the difference bttween padding and margin. The image doesn't need padding, only the card does.
  7. This is invalid html: width="250px" height="250px". Those attributes must not have units inside them. The browser would read them as pixels anyway. But really you need to understand what they're therefore if you want to include them. The purpose is for the browser to be able to calculate aspect ratio of that image and save the space for the image, thus improving performance slightly. You could put width 600 x 600 if you wanted and it would do the exact same thing -- tell the browser this image is square (i.e. it has an aspect ratio of 1). If you choose to include a width and height attribute you will need to set this image to be 100% wide in the CSS. Whether or not you include width and height this image should be styled to display block and max width 100% both of which are usually included in a standard CSS reset.
  8. Get into the habit of always including a full modern CSS reset at the start of your styles in every project. Andy Bell or Josh Comeau both have good examples you can look up and use.
  9. Image elements must always have an alt attribute. This image in particular is extremely important content in the component so it must have a proper description within that alt attribute. In this case, the alt must say what the image is (QR code) and where it goes to (to frontendMentor.io).
  10. Don't wrap everything in a div!! You only need a div for block element styling or span for in-line element styling. Only add them when necessary. You don't need to wrap every single element in one of these. Keep the HTML as simple as possible.
  11. Instead you must use meaningful HTML elements. I can't stress how important this is. It is extremely important, more important than anything else. Like in architecture the HTML is the foundations of the building you build on -- if you get that wrong everything is wrong. You need to look at a design, think about the content and carefully translate that content into the most appropriate meaningful HTML elements. It it is an essential skill. This card is made up of an image with meaningful description. a heading and a paragraph. Other challenges you do unlikely to have more HTML elements such as Lists, headings which need to be in the right order, buttons for actions, links for navigation, tables for tabular data etc.
  12. Never style on IDs. That's not what they're for. Use single class selectors wherever possible. That will keep your CSS specificity as low as possible, which is a very good thing, making styles much easier to manage.
  13. Remember in CSS order really matters. You mustn't ever have reset styles or wildcard selectors at the end of your CSS. This will override earlier styles and lead to horrible bugs. Styles lower in the style sheet will override what has been written above. This is called the Cascade and it is core to how CSS works -- Cascading Style Sheet. This will become even more important in later larger challenges.

  01
• 

  Masseh•260

  @Masseh2025

  Posted 12 months ago

  Overall its pretty good, but one thing I would probbaly to make the solution a little bit bigger.

  04