-
-
Notifications
You must be signed in to change notification settings - Fork 78.8k
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
Colors rewrite #30622
Colors rewrite #30622
Conversation
I really prefer the percentage values you're suggesting to remove |
Is this safe to bring back and set to ready for review for Alpha 3? |
2f1ae1a
to
27dba2e
Compare
Yeah, fine by me. Worked a bit on this today, but it seemed to be a bit more complex than I initially thought. I'll keep you posted. Edit: It's done ;) |
6e8a4bb
to
e7cf9a6
Compare
e7cf9a6
to
1892c8c
Compare
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.
LGTM, really helpful changes!
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.
Hell yeah, huge refactor and update, @MartijnCuppens!
Any reason to implement this change using black and white (essentially working in HWB) rather than using Sass's |
A little more guidance in the migration documentation would be helpful when 5.0 is released. Through some experimentation I found that |
Purpose of this PR is to use a uniform approach on using colors:
tint-color()
to replacelighten()
shade-color()
to replacedarken()
scale-color()
to replacecolor-level()
(since we don't work with levels anymore)$theme-color-interval
, the8%
levels are hard to calculate with.vs
(color contrast on the light variants is still an issue, but can be tackled later on to prevent this PR from being too overwhelming)