Submitted over 1 year ago

Responsive-QR-Code-Card

Nadun Dhananjaya• 210

@nadun-dhananjaya

Preview site(opens in new window)View code(opens in new window)

This is a solution for...

QR code component

HTML
CSS

1newbie

View challenge

Design comparison

SolutionDesign

Solution retrospective

Hello, everyone! Just completed qr-code-component solution challenge.😊 🤩

Any feedback's and suggestions will be appreciated. Thanks : ).

Community feedback

Melvin Aguilar 🧑🏻‍💻• 61,220

@MelvinAguilar

Posted over 1 year ago

Hello there 👋. Good job on completing the challenge !

I have other suggestions about your code that might interest you.

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.
You don't need to add font-family: "Outfit" to each <p>, <h*> separately. Instead, add it to body.

Alt text 📷:

The alt attribute should explain the purpose of the image. Uppon scanning the QR code, the user will be redirected to the frontendmentor.io website, so a better alt attribute would be QR code to frontendmentor.io

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

Nadun Dhananjaya• 210

@nadun-dhananjaya

Posted over 1 year ago

@MelvinAguilar Hello! Thank you for your kind words and for taking the time to give me feedback on my code. I really appreciate it!

You make a great point about the <br> tag. I agree that it's often overused and doesn't add any semantic meaning to the code.

Thank you for your suggestion about the font-family. Adding it to the body element is a cleaner and more efficient way to apply the font to all the elements on the page. I will definitely update my code to reflect this.I also appreciate your feedback on the alt text for the QR code image.

Thank you again for taking the time to review my work!

Hassia Issah• 50,670

@Hassiai

Posted over 1 year ago

Replace <div class="page-container"> with the main tag and <h2> with <h1> to fix the accessibility issues. click here for more on web-accessibility and semantic html

Give .testimonial 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.

To center .qr-container on the page using flexbox or grid instead of position: absolute, 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;
}

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

Nadun Dhananjaya• 210

@nadun-dhananjaya

Posted over 1 year ago

@Hassiai Thank you for sharing these helpful tips!

I appreciate your suggestion to use relative units like rem or em instead of absolute units like px for padding, margin, and font-size values. Your advice on centering the .qr-container using flexbox or grid is also very useful.

Overall, I found your comment to be informative and well-explained. Thank you for sharing your expertise!!!

Atif Iqbal• 3,200

@atif-dev

Posted over 1 year ago

Hi Nadun, congrats🎉 on completing the challenge. Better take care about following points.

Always check Frontendmentor Report Generator issues after submitting the project for removing errors and warnings. Use alt (alternate text) attribute as it is mentioned in your Accessibility Report. alt attribute is used for Screen readers technology. Why alt attribute matters? Read here. To avoid accessibility issue "All page content should be contained by landmarks" use code as :

<body>
    <main>
        ---your code here----
    </main>
  <footer>
  </footer>
</body>

(why main matters? Read here)

Following is simple method to center a card(container) vertically and horizontally:

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

Or you can learn centering from here.

Hope you will find this Feedback Helpful.

Marked as helpful

Nadun Dhananjaya• 210

@nadun-dhananjaya

Posted over 1 year ago

@atif-dev Hello! Thank you so much for your congratulations and for sharing these helpful tips for improving my code.

You're absolutely right that it's important to use the Frontend Mentor Report Generator to check for errors and warnings. I will make sure to use it in the future to ensure that my projects are as error-free as possible.

Thank you for your suggestion about using the alt attribute for images. I understand the importance of providing alternative text for users who rely on screen readers, and I will make sure to use it in all my future projects.

I appreciate your advice on using landmarks to organize the page structure and improve accessibility. I will definitely try to incorporate them into my future projects.Your tip on centering a card/container using CSS grid is also very helpful.

Thank you again for sharing your expertise with me. I really appreciate it!

Please log in to post a comment

Join our Discord community

Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!

Join our Discord