Submitted almost 2 years ago

Responsive QR Code Component Main

Sandy Marrone• 145

@SamCastle16

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!

I hope you will tell me what you think about my development of this mini-project and what things could be changed and improved. It would be greatly appreciated!

Community feedback

バレンタイン 😈• 64,190

@VCarames

Posted almost 2 years ago

Hey @SamCastle16, some suggestions to improve you code:

For this challenge you want to use the Image Element not the Background Image Property. By using the Background Image Property screen readers users have no idea where the image is to scan.
Once you fix the image implementation, you'll want to include an Alt text tag with them. Inside that Alt Tag Its needs to tell screen reader users what it is and where it will take them to when they scan it.
Change the width to max-width in your card container to make it responsive.

Happy Coding! 👻🎃

Marked as helpful

Sandy Marrone• 145

@SamCastle16

Posted almost 2 years ago

@vcarames Hello! Thank you very much for your answer. I have already updated the project, improving the code with all your suggestions. I hope you review it again, if you find another detail or recommendation, I would appreciate it!

I implemented the QR code image inside the img tag.

I simplified the code by removing unnecessary divs and just using the basic html tags (main and footer). I also simplified the selectors in css, removing the classes to make the code cleaner.

I put the attribution outside the main element and inside the footer as you told me.

And I adjusted the responsive and it looks correct.

Thank you very much for the recommendations, they will make me grow a little more as a developer!

Lucas 👾• 104,440

@correlucas

Posted almost 2 years ago

👾Hi @SamCastle16, congratulations on your solution!👋 Welcome to the Frontend Mentor Coding Community!

Great solution and a great start! From what I saw you’re on the right track. I’ve few suggestions for you that you can consider adding to your code:

1.Use relative units like rem or em instead of px to have a better performance when your page content resizes on different screens and devices. REM and EM does not just apply to font size, but all sizes as well. To save your time you can code your whole page using px and then in the end use a VsCode plugin called px to rem to do the automatic conversion or use this website https://pixelsconverter.com/px-to-rem

2.Your solution is great and the code is working, but the HTML structure can be reduced by removing unnecessary divs, all you need is a single <main> or <div> to keep all the content inside, and nothing more. The ideal structure is the div and only the image, heading, and paragraph.

Here’s one example to show can be cleaner this HTML structure:

<body>
<main>
<img src="./images/image-qr-code.png" alt="QR Code Frontend Mentor" >
 <h1>Improve your front-end skills by building projects</h1>
<p>Scan the QR code to visit Frontend Mentor and take your coding skills to the next level</p>
</main>
</body>

To reduce the CSS you can use the direct selector for each element instead of using class this way you have a code even cleaner, for example, you can select everything using the direct selector for (img, h1, and p, main).

Here's my solution for this challenge if you wants to see how I build it: https://www.frontendmentor.io/solutions/qr-code-component-vanilla-cs-js-darklight-mode-nS2aOYYsJR

✌️ I hope this helps you and happy coding!

Marked as helpful

Sandy Marrone• 145

@SamCastle16

Posted almost 2 years ago

@correlucas Hello! Thank you very much for your answer. I have already updated the project, improving the code with all your suggestions. I hope you review it again, if you find another detail or recommendation, I would appreciate it!

I simplified the code by removing unnecessary divs and just using the basic html tags (main and footer). I also simplified the selectors in css, removing the classes to make the code cleaner.

Thanks for recommending the VSCODE "px to rem" extension. I will use it a lot!!!

Vanza Setia• 27,795

@vanzasetia

Posted almost 2 years ago

Hi, Sandy! 👋

Congratulations on completing this challenge! 🎉

In addition to what @vcarames has said about the QR code image, only make the decorative images as background images. In this case, the QR code is the main content of the page. So, it should be on the HTML.

Then, put the attribution outside the main element. It should be live inside the footer.

After that, I recommend using the body element as the container of the card. Also, use min-height instead of height. You can see the issue by looking at the site on a mobile landscape view.

Lastly, I suggest adding rel="noopener" to any anchor tags that have target="_blank". It helps protect users of legacy browsers. I suggest reading "Links to cross-origin destinations are unsafe" article to learn more about this.

I hope this helps! Happy coding!

Marked as helpful

Sandy Marrone• 145

@SamCastle16

Posted almost 2 years ago

@vanzasetia Hello! Thank you very much for your answer. I have already updated the project, improving the code with all your suggestions. I hope you review it again, if you find another detail or recommendation, I would appreciate it!

I implemented the QR code image inside the img tag.

I simplified the code by removing unnecessary divs and just using the basic html tags (main and footer). I also simplified the selectors in css, removing the classes to make the code cleaner.

I put the attribution outside the main element and inside the footer as you told me.

And I adjusted the responsive and it looks correct.

Thank you very much for the recommendations, they will make me grow a little more as a developer!

Vanza Setia• 27,795

@vanzasetia

Posted almost 2 years ago

@SamCastle16 You're welcome! 👋

There are two things that need to be done.

First, add some padding on the body element to prevent the page content from touching the edges of the browser.
Second, add rel="noopener" to all links that have target="_blank".

Other than those two things, everything else is looking great to me. Good job! 👍

Marked as helpful

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