-
-
Notifications
You must be signed in to change notification settings - Fork 229
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
enhance: add more background gray colors #3855
Conversation
Quick links (staging server):
Login:
SVG tester:Number of differences (default views): 0 ✅ Edited: 2024-08-06 18:25:16 UTC |
Site screenshots are super helpful here! |
@@ -1,9 +1,6 @@ | |||
$dark-text: #5b5b5b; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can revert these "convention changes" I made by swapping out the top level definitions for inline usage of the global variables if you'd prefer, Marcel.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I've done more of these now :)
Overall, I find it nice sometimes that colors are given a more meaningful name with local SCSS variables inside a component. I don't mind that, but I do mind using hex codes when color names are available and make more sense.
I've continued your work, Ike, and have replaced references to colors with their names in almost all places I could find. As a sanity check, I also diffed the generated CSS (owid.css) before and after: The only change is using the correct |
Soo, when I just went to add more gray background colors, I noticed that we've been using the wrong
gray-10
color all this time (marwa-raging) 🤯It's a slight difference, but it's a difference.