@vanzasetia
Posted
👋Hi Said Alejandro!
It looks great 🙌 on both portrait and landscape on my mobile device.
Now for the feedback on this solution:
- I would recommend to use
ulandliinstead ofpandspanfor the list. - Also keep the default HTML font size
100%. If you cut it off to62.5%it will make the users who already set bigger font size will still see small font size while for the opposite, they will see very small text (like me). - For the body font size, you need to use
remunit too. - I noticed that you used
margin-block,padding-block, and other logical CSS properties. In my opinion, it would be much better if you use themarginandpaddingproperty or more "common" CSS property, since they 100% get supported by all browsers. But, if you may want to use them, you might need to use@supportto support older browser.
That's it! Hopefully this is helpful!
Marked as helpful
@said-alrove
Posted
@vanzasetia ✌ Hi!, first of all, thanks for your feedback, and I'd like to reply to everything:
✅.- Your first recommendation is already done 👏, I had already done this challenge a couple of months ago when I wasn't too much confident with CSS and HTML, therefore I guess using spans instead of a list made more sense for me, thus when recreating it I was trying to focus on recreating a cleaner code (with the 7-1 patter) more than checking if the original code was correct, so ... my bad 😥.
✅.- Using 16px in the body to reset the root size to its original was one of those things I learned when I started with CSS, therefore I had never asked myself if that was totally correct, I just assumed it, but now that you mention that, using 1.6rem makes more sense (plus I looked for information after reading your comment for a fact check and yep, you're right), so ... thank you!.
✅.- Yeah, I know, but I use them basically because these are simple challenges that maybe nobody else out of the people from here is gonna see (I'd like to use these challenges in my portfolio in the future, but because logical properties will be good enough supported when I finish of creating it, or at least I hope so, nevermind either), therefore I'd like to get used to using them to be prepare when those properties would be something as normal as the common ones.
Yep, your comment was very helpful 😸, thank you so much for taking a bit of your time to write it!.
Have a nice day :D!.
@vanzasetia
Posted
@said-alrove 👍Great job on improving your code!
Your second point about root font size, I was talking about the html font size.
What I meant that, by changing the root font size, which meant the html font size.
html {
font-size: 62.5%;
}
It will make the users who already set bigger font size will still see small font size while for the opposite, they will see very small text (like me).
So, for example if user set their browser font size to 20px, they will see 12.5px and for the one who set it to 14px, they will see 8.75px (too small).
The math:
user font size * 67.5% = what they will see.
I recommend to keep the root font size 100% (the default).
😀 Hopefully this clarified what I said before.
@said-alrove
Posted
@vanzasetia Yep, now I get the point, I took it for granted 😥, I'll continue looking for more information for things like these, and I'll apply them in future projects 😸.
