QR code component

Community feedback

• 

  Dominik Gartz•140

  @domieeePosted almost 2 years ago

  👋🏼 Hi Eduardo,

  I have looked at your code and I have noticed some things

  • For the web accessibility it is important that the h1 element is always at the beginning of the page. A web crawler always looks for the h1 element, and uses its content to describe what the page is about. Also you should nest your code into a <main>-element Take a look here

  • Don't use fixed heights or widths. You can give the container for the image a max-width: 320px; and a width: 100%. The img also gets width: 100%. So the container adapts to the browser size, and the image to the container. so everything stays responsive.

  • For the font-size property you should not specify px. Instead you can use rem. px does not adjust when the brower size is changed. Read more about rem here

  Otherwise it looks great and very similar to the design!

  Have a nice day and keep coding!

  Marked as helpful

  1
• 

  T

  Grace•31,270

  @grace-snowPosted almost 2 years ago

  Well done on your first challenge

  Here are some improvements

  1. Use landmarks. Container-boss should be a main element and attribution should be a footer element
  2. This is not how you write alt descriptions on img elements. This img needs a proper description of what it is and what url it goes to when users scan it
  3. This component must not have a height. Let the browser decide how tall it needs to be
  4. This component does not need a width. Instead give it a max width so it can shrink if it needs to
  5. Give the component a little margin so it can't hit screen edges
  6. The card should have padding. The image should not have margin top
  7. Always include a modern css reset at the start of the stylesheet. Amongst other things this would set img elements to display block and max-width 100%. That's all you need on this img, it should not have an explicit width
  8. Font size must never be in px. This is extremely important. Use rem
  9. Heading and paragraph shouldn't really have padding. Padding is for internal spacing, margin is for external. There's no concept of "internal" on low level elements like these (they're not boxes), so they only have margin usually. To limit the width of the paragraph usually you would do that with a max-width in ch..
  10. It's better for performance to link font families in the html document head instead of importing in css

  Marked as helpful

  1