Qr component using padding and border radius with a bit of font css

Community feedback

• 

  T

  Grace•31,090

  @grace-snowPosted 5 months ago

  I'm afraid this is missing the foundations of necessary html for the content.

  1. Every page must have a main landmark. This is only a single component challenge but you should still have a main container on the page, and your card component would then go inside it.
  2. Keep the html as simple as possible. The image does not need wrapping in an extra div.
  3. The image is the most important content on the page. It must have a proper alt description of what the image is (QR code) and where this code goes (to FrontendMentor.io). There are good posts about how and when to write alt text on images in the resources channel on discord.
  4. Use meaningful elements! This is so important I can't stress it enough as it's the biggest single thing your solutions are missing at the moment. This card should have a heading element and a paragraph, not divs wrapping text. Text should never be in divs or spans alone as they are meaningless elements for layout only.
  5. Get into the habit of including a full modern css reset at the start of the styles in every project. Andy Bell or Josh Comeau both have good ones you can look up and use.
  6. The card must not have a width or height. It should only have a max width in rem. That limits its width but allows it to shrink narrower when it needs to (like on smaller screens or when zoomed). The value should be in rem so the layout scales correctly for all users including those who have a different text size setting. You never want to limit the height of elements that contain text as you can never guarantee what height is needed —that is the browsers job to decide the height based of the content.
  7. The image shouldn't be wrapped in a div with an explicit size and the image must not have a height. It should be max-width 100% (not height) but this would be part of any standard css reset anyway. So if the css reset was included you would only need to give this img a border radius. For a performance boost you could give the img an aspect ratio of 1 as well, as it's a square image.
  8. The padding should be in the card component, not on a div wrapping the img.
  9. Font size must never be in px. Use rem. The font size in the style guide should be converted to rem then used on the body. That gives you the correct paragraph size.
  10. Make sure you understand the difference between padding and margin. I would expect to see margin to create vertical space between elements within the card not padding.

  Marked as helpful

  2

  ---

  

  Ryan•30

  @RyoliveiraPosted 5 months ago

  @grace-snow Thanks for the reply. I went back and implemented a lot the advice you've given me. Learned a bit more about css resets why they're used. Im sure I can still improve, but its a lot better than my first solution submission.

  1

  

  T

  Grace•31,090

  @grace-snowPosted 5 months ago

  @Ryoliveira one outstanding issue is that the component is able to touch the screen edges on all sides. Give a wrapping element (in this challenge thats probably the body) a little padding so that can't happen.

  Marked as helpful

  1