QR-Code-component using semantic HTML and Basic CSS

Community feedback

• 

  AlejandroIMP•190

  @AlejandroIMPPosted 2 months ago

  Great job!

  I have some suggestion for you:

  Incorrect Heading Usage:

  • The <h1> tag is being used incorrectly. It should not wrap a <p> tag. Instead, the text inside <p id="one"> should be directly inside the <h1> tag.
  • Example:

    <h1>Improve your front-end <br/> skills by building projects</h1>
    

  Unnecessary <br/> Tags:

  • Using <br/> for line breaks is not ideal for responsive design. Consider using CSS (e.g., margin or padding) to control spacing.
  • Example:

    <p id="one">Improve your front-end skills by building projects</p>
    <p id="two">Scan the QR code to visit Frontend Mentor and take your coding skills to the next level</p>
    

  Overall, the code is well-structured and functional, but with a few tweaks, it can be more semantically correct and accessible.

  Hope you find this helpful

  Marked as helpful

  1
• 

  P

  Kamran Kiani•2,740

  @kaamiikPosted about 2 months ago

  Some notes:

  1. alt message: Read this article and It really helps: How to Write Good Alt Text

  ---

  2. About headings: You should have one h1 in your page but this is not a page and It is a card. So remember in a real scenario you are not using h1 here. Also read this: Heading Order Guide

  ---

  3. Line break: Avoid using <br> to wrap text; instead, use max-width and let the browser handle text wrapping.

  ---

  4. Modern CSS reset: It's recommended to use a proper CSS reset at the beginning of your styles. Both Andy Bell and Josh Comeau provide excellent CSS resets, which you can easily find online.

  ---

  5. min-height: Use min-height: 100vh; instead of height: 100vh; to prevent overflow issues.

  ---

  6. Do not limit text container: Avoid setting fixed width and height for elements containing text. Instead, use max-width for better adaptability. In your code, width: 290px; and height: fit-content; are wrong and should replace with max-width.

  ---

  7. No need for fixed positioning: Use flexbox to stick your footer to the bottom. Fixed positioning here is a bad practice.

  Marked as helpful

  0

  ---

  

  Donnish233•40

  @Donnish233Posted about 2 months ago

  @kaamiik Thank you for your feedback. I'll make sure to work on it.

  However, as regards to the seventh point you made, which is "No need for fixed positioning", could you better explain the steps I should follow to achieve the desired result?, because I'm relatively new to all this coding and stuff.

  0

  

  P

  Kamran Kiani•2,740

  @kaamiikPosted about 2 months ago

  @Donnish233 As I see you added the fixed positioning to stick your footer at the bottom of the page. This is not a good approach here to use fixed positioning. Try to learn this with flexbox or grid. After your body you have main and footer as a direct child. One of them needs to grow and one not. Try to learn how to work with flex: 1;

  0