Design comparison
Solution retrospective
Hello everyone! π
I just want to create a simple project to boost my motivation a bit. Also, I keep it as simple as possible since it's a simple challenge so I don't want to think too much about it. However, I have written a README where I mention some approaches that I could take to finish this challenge. I hope that it will be helpful.
Anyway, feel free to give me any suggestions or feedback. Thanks!
Community feedback
- @MohmedElshaarawyPosted almost 2 years ago
Hello there π. Good job on completing the challenge !
I have some suggestions about your code that might interest you.
HTML π:
Use the <main> tag to wrap all the main content of the page instead of the <div> tag. With this semantic element you can improve the accessibility of your page. Use the <footer> tag to wrap the footer of the page instead of the <div class="attribution">. The <footer> element contains information about the author of the page, the copyright, and other legal information. Since this component involves scanning the QR code, the image is not a decoration, so it must have an alt attribute. The alt attribute should explain its purpose. e.g. QR code to frontendmentor.io CSS π¨:
Instead of using pixels in font-size, use relative units like em or rem. The font-size in absolute units like pixels does not scale with the user's browser settings. This can cause accessibility issues for users who have set their browser to use a larger font size. You can read more about this here π. Use min-height: 100vh instead of height: 100vh. The height property will not work if the content of the page grows beyond the height of the viewport. I hope you find it useful! π
Happy coding!
Marked as helpful0@vanzasetiaPosted almost 2 years ago@MohmedElshaarawy
Hi, Mohmed!
Thank you for your suggestion about the alternative text. I have made the improvement.
For the other suggestions, I have implemented all of them since the beginning like:
- Using
<main>
element for the card, - Using
<footer>
for the attribution, - No
px
unit in my CSS, - Use
rem
for allfont-sizes
, and - Set
min-height: 100vh
to the<body>
element (never setheight
)
You said "You can read more about this here π", it looks like you want to give me a link (?).
0 - Using
- @DarrickFauvelPosted over 2 years ago
Hi @vanzasetia, π
Great job! π
The only things I can see are slight font size and line height issues. I don't know if you've ever tried the
:root { font-size: 62.5% }
trick? I use it all the time (until someone tells me a better way). It sets up all of yourrem
font sizing to be1rem = 10px
. So, a 25px font from Figma would be equal to 2.5rem.As for line height, I have been using the
calc()
function to do the math for me. I get the pixel line height from Figma, say 28px for the heading, divide it by the heading pixel font size of 22px, and the result will be a number.So, in this challenge try this:
:root { font-size: 62.5%; } .card__title { font-size: 2.2rem; line-height: calc(28 / 22); } .card__description { font-size: 1.5rem; line-height: calc(19 / 15); }
πNote: It will throw off your other sizes based on the root font size. So, you would adjust accordingly.
I hope this helps! π
1@vanzasetiaPosted over 2 years ago@DarrickFauvel Hi Darrick! π
Thank you for taking some time to review my code! I really appreciate it!
So, about root font size, I would not change it. Changing the
html
or root font size can cause huge accessibility implications for those of the users with different font size or zoom requirements. You can read what an accessibility expert (Grace Snow) has said about it.For the
line-height
, I never try usingcalc
to measure it. Thank you for telling me that trick. πAs far as I know,
line-height
will automatically adjust based on thefont-size
, and in this case, thefont-size
is the same for both mobile and desktop design, so I don't think I need to adjust theline-height
.I just want to make sure we're all in the same page so, if I set
font-size: 1rem
and then I set theline-height: 1
, now theline-height
would be16px
. So, if I increase thefont-size
to2rem
, that means theline-height
would be32px
.2@DarrickFauvelPosted over 2 years ago@vanzasetia Thank you so much for correcting me, Vanza.
Like I said, "until someone tells me a better way" for font sizing. You just did. π Your reference to @grace-snow is greatly appreciated.
I also noticed @grace-snow is using a Sass function to do px to rem calculations, like so:
@function rem($size) { @return $size / 16px * 1rem; }
I will be trying this out.
As for the line height, here are my calculations from the design file:
card__title
: 28px line height / 22px font size =1.2727
card__description
: 19px line height / 15px font size =1.2666
- apparently, these are a result of Figma's default
auto
line height setting
It's my understanding that a default CSS line height is roughly
1.2
(which is a multiplier). So, the results of both pieces of text are a little bit larger than the default multiplier. It may be hair-splitting at these small font sizes, but I know this is important when I've worked with designers (maybe the wrong ones π€ͺ), and so I try to be accurate where I can as I learn more.1@vanzasetiaPosted over 2 years ago@DarrickFauvel The Sass function that she is using is working fine if you are going to compile the Sass code with the Live Sass Compiler. But, if you are using the Dart Sass, which is the newest and the recommended Sass compiler, you need to change some things a bit.
For the line height, I'm not going to calculate that in detail. π It's because if I do that, it makes me write
line-height
property for each different element, which might not be a best practice to do, especially on bigger projects.Also, I don't think that the designer is going to be angry because of the
0.0666
difference between the site and the design. π2@DarrickFauvelPosted over 2 years ago@vanzasetia That's good to know about the difference with the Sass compiling. Thank you.
Yes, you are probably right about the line height and I am just over thinking it. π€¦ββοΈ Thanks for your feedback on it. Much appreciated. π
1@vanzasetiaPosted over 2 years ago@DarrickFauvel You're welcome! It's great to have a discussion with you! π
2
Please log in to post a comment
Log in with GitHubJoin 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