QR-code-component

Community feedback

• 

  Melvin Aguilar 🧑🏻‍💻•61,140

  @MelvinAguilarPosted over 1 year ago

  Hello there 👋. Good job on completing the challenge !

  The solution looks much better now, I just have a few small suggestions.

  HTML 🏷️:

  • The <br> tag is often used to create line breaks, but it doesn't convey any semantic meaning. When a screen-reader reads the text, it will break the flow of reading at the line break tag, which can be confusing for users.

  Alt text 📷:

  • To make the alt attribute as useful and effective as possible, avoid using words such as "image", "photo", or "picture" as they are redundant because the image tag already conveys that information. Instead, try to make the description as human-readable and understandable as possible.

    The alt attribute should explain the purpose of the image, for example, in the case of a QR code, a description like "qr code to frontendmentor.io" would be more appropriate.

    If you want to learn more about the alt attribute, you can read this article. 📘.

  I hope you find it useful! 😄 Above all, the solution you submitted is great!

  Happy coding!

  Marked as helpful

  1

  ---

  

  'Tiana Coker•40

  @Tiana-CokerPosted over 1 year ago

  @MelvinAguilar Oops! I forgot to eliminate the br tag in the paragraph element. I'll do so right away.

  Regarding the alt text, your explanation makes it more clearer now. I will make it descriptive as suggested.

  Thank you so much.

  0
• 

  Hassia Issah•50,650

  @HassiaiPosted over 1 year ago

  Replace <section class="container"> with the main tag and wrap Improve your front-end <br>skills by building projects in <h1> to fix the accessibility issues. click here for more on web-accessibility and semantic html

  There is no need for the <br> in the html.

  Give the alt attribute in the img a value. The value of the alt attribute is the description of the image. For decorative images like icons, there is no need to give it an alt value, for more on alt attribute Click here.

  Give the body a background-color of light-gray. There is no need to give the body a padding value.

  There is no need to style .container. To center .main-container on the page using flexbox or grid instead of margin, add min-height:100vh; display: flex; align-items: center: justify-content: center; or min-height:100vh; display: grid place-items: center to the body.

  USING FLEXBOX:
  body{
  min-height: 100vh;
  display: flex;
  align-items: center;
  justify-content: center;
  }
  

  USING GRID:
  body{
  min-height: 100vh;
  display: grid;
  place-items: center;
  }
  

  For a responsive content which require a media query for this challenge, give .main-container a fixed max-width value and a padding value for all the sides. max-width: 320px padding: 16px . Give the img a max-width of 100%.

  Give content a margin value for all the sides, text-align: center and a font-size of 15px which is 0.9375rem, this will be the font-size of both p and h1. Give p a margin-top or h1 a margin-bottom value for the space between the text.

  Use relative units like rem or em as unit for the padding, margin, width values and preferably rem for the font-size values, instead of using px which is an absolute unit. For more on CSS units Click here

  Hope am helpful.

  Well done for completing this challenge. HAPPY CODING

  Marked as helpful

  1

  ---

  

  'Tiana Coker•40

  @Tiana-CokerPosted over 1 year ago

  @Hassiai I truly value your feedback and the time you took to evaluate my code. I will apply your suggestions. Thanks for stopping by.

  1
• 

  safdar•130

  @sfdrkPosted over 1 year ago

  Hey 👋 Nice work ... Great design .... Here is my suggestions

  You can remove the padding of body completely .... then you can add min-height:100vh for the body .... You can center the main container in different ways .... what i suggest is flex and grid . Here are the styles .

  body{min-height:100vh;display:grid;place-items:center}

  or

  body{min-height:100vh;display:grid;align-items:center;justify-content:center}

  Then you can add max-width for your main-container because then it will not go out of flow it will stay relatively to your screen sizes ... that's good for better responsiveness.

  main-container{max-width:350px;width:100%}

  Your design is pretty good 🎉

  Marked as helpful

  1

  ---

  

  'Tiana Coker•40

  @Tiana-CokerPosted over 1 year ago

  @sfdrk Duly noted, I will try your suggestions right away. Thanks for your valuable feedback.

  1