@vanzasetia
Posted
👋Hi Sloth!
I have some feedback on this solution:
p
tags should never be a children ofh1
instead I recommend to use this markup.
<h1>Reliable, efficient delivery
<span>Powered by Technology</span>
</h1>
- I recommend to use
header
tag since the class name isheader
so it becomes more semantic rather thandiv
- Your BEM is for this class
card card__supervisor
, I recommend to change it as a modifier like thiscard card--supervisor
. - I recommend to leave the default
html
font size which is100%
. The problem with setting it to62.5%
, it will make the user that has bad vision have to zoom a lot since it will always 62.5% of the current size. Yes, it makes developer easier to calculate 1rem but the purpose is not about what convenient for the developer, it's about what convenient to the user.
That's it! Hopefully this is helpful!
Marked as helpful
@Sloth247
Posted
@vanzasetia
Thanks, I'll fix it based on your suggestion.
Just a question for the font size on html, I looked at your sass file for this project and you did not seem to convert the font-size to the default 15px. I am seeing that you write font-size: rem(15)
in body
, does this means 15px? I have never seen and learnt this style of unit, so I'm just curious.
@vanzasetia
Posted
@Sloth247 It's a custom Sass function (not CSS) that I created to convert px
to rem
. So if I write rem(16)
, it will convert it to 1rem
.
The body
font size will make p
tag or any elements that inherit body
font size to be 0.9375rem
( is equal to 15px) as default font size.
So, you can change the body font size using rem
unit however you like, but don't change the html default font size, which is 100%
.