@ApplePieGiraffe
Posted
Hi, tediko! š
This looks awesome! š¤© I love the extra-page and animations that you added and everything else looks really, really good! š
Your code looks nice, as well! š I sometimes feel like my styles are all-over-the-place when I finally finish a project, too, but I think you've used styled-components rather well! š
A few tiny suggestions I have are,
- Setting the root font-size of the document to a percentage (rather than a fixed value) so that users can scale the design of your app up/down by changing the default font-size of their browser (which is good for accessibility).
- Probably using
rem
rather thanpx
in your media queries (so that the breakpoints in your styles will scale with any changes made to the root font-size of the document and consequently, with the rest of the design). - Using unit-less values for line-height, such as
1.5
or1.75
, etc. (which is often considered a good practice because then the line-height will be proportional to the given font-size). - Perhaps skipping
ItemController.js
for the header navigation and just putting that code inNavDesktop.js
instead. They are just both pretty small components so it might be a little easier for them to be together. But just a thought! š
But those are like teeny-tiny things because I can't find other stuff to suggest! š
Kontynuuj kodowanie (i szczÄÅliwego kodowania)! š
(Mam nadziejÄ, że tÅumaczenie Google nie przetÅumaczyÅo tego wyrażenia niepoprawnie!) š
Marked as helpful
@ApplePieGiraffe
Posted
One more thingāthe solution report is complaining about bgcolor
not being a valid HTML attribute on the navigation links. I just recently found out that you can prefix props like those (that are only meant to be used by styled-components) with a $
sign, so that they are used by styled-components but do not get passed down to the underlying HTML element (so $bgColor
instead of just bgColor
). š
@ApplePieGiraffe Hello, APG! Thank you very much for exhaustive feedback, as always! :)
To begin with, I would like to mention the first two points. I once read Zellwk article about which unit we should use for a queries. After that I was about to switch from using pixels for root element aswell as using pixels units for media-queries to using percentages and rems/ems for queries. But I wanted to read other perspective on that topic and I came across Adam Wathan article where he challenged Zellwk's approach and tests and showed his perspective. He said that "Pixels are the only unit that behave consistently across all commonly used browsers" and he stressed that we should "set an explicit root font size in pixels. It will override the user's custom default font size which is mildly annoying for the user, but it will force them to zoom instead which always behaves the way you want.". Maybe you have more recent articles on that?
I'll start using unit-less values for line-height. Now I read that this is the preferred way to set line-height and avoid unexpected results due to inheritance. You've right also on keeping my two small components together. Both ItemController
and NavDesktop
are so small so they can fit in one component. Except that thanks for pointing me that I can prefix props for styled-components!
You did great with Polish! Keep coding too! :)
@ApplePieGiraffe
Posted
@tediko
Those are actually very interesting articlesāthanks for sharing them! š I should probably be careful about what I say relating to font-sizes š, but I think I should probably also look into this some more on my own. š So I guess that's definitely another approach! š
Looking forward to your next challenge! š