Submitted 9 months ago

QR code component HTML & CSS beginner

S02K• 50

@S02K

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

This was my first ever (small) coding project, my question is simple and i hope you all can tell me what i could have done better so i can become a better web developer.

Are there any best practices i should have used?
Did i do it the easiest way or are there better / faster ways to tackle this project?

Community feedback

Dreamleaf• 230

@Dreamleaf

Posted 9 months ago

Pretty clean code for a beginner, well done!

Two things I can spot in your code that would improve things that may not be obvious.

For the QR image, be specific about what the image is doing... eg. "QR Code to frontendmentor.io" instead of just "QR Code". This will be more informative for users of screen readers and tell them exactly what will happen.

Secondly, you have used 100vh on the body class. This isn't wrong, but you may sometimes get unintended results on mobile viewports. Do a search and read about the alternative values of SVH and DVH - basically, just using VH includes UI elements such as the address bar - the alternative can ignore these and just focus on the DOM part of the view.

Great work though, I look forward to seeing you tackle more challenges.

Marked as helpful

S02K• 50

@S02K

Posted 9 months ago

@Dreamleaf

Thank you for the helpful feedback i have now changed the ALT text i will also read up on SVH and DVH so i don't run into mobile issues later on!

Øystein Håberg• 9,450

@Islandstone89

Posted 9 months ago

Hi there. I agree with the comment above, your code is quite good for a beginner. You have a <main>, which most beginners don't include, and you're using Custom Properties. The card is nicely centered using Flexbox, and you are using padding to create the space between the image and the edge of the card - many tend to use margin on the image, instead.

Here are my tips on how to make your solution even better. Hope it helps :)

HTML:

.attribution should be a <footer>.
Footer text needs to be wrapped in a <p>.

CSS:

Performance-wise, it's better to link fonts in the <head> of the HTML then using @import.
It's good practice to include a CSS Reset at the top.
The body should have a min-height instead of height, this allows the content to grow.
Font-size must never be in px. Use rem instead.
Remove the fixed width. You rarely want to set fixed dimensions, as this easily creates issues with responsiveness.
Add a max-width of around 20rem on the card, to prevent it from getting too wide on larger screens.
Since all text is centered, I would set text-align: center on the body, and remove it elsewhere.
The image should have display: block, and instead ofwidth use max-width: 100%.

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