-
-
Notifications
You must be signed in to change notification settings - Fork 834
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
Declare & Use CSS Custom Properties #3146
Conversation
Modal-loading was removed a long time ago in d44ffd1#diff-c1efc7e5a8c26481ecc00f9d17681fd0971ade5e097c4754a85c61de0be9189bL59
# Conflicts: # less/admin/ExtensionPage.less # less/common/Modal.less
This is huge! Really excited to see us taking this big step towards themability and modern architecture. |
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.
Looks good to me! Just one minor suggestion.
Co-authored-by: David Wheatley <[email protected]>
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.
Excellent!
Fixes #2600
Changes proposed in this pull request:
Reviewers should focus on:
Most of the changes are simple replacements of the common variables. What you should pay attention to is the following:
root.css
declaration of custom properties..Button--color()
,.light-contents()
)For example the
Button--color
mixin used to be directly called as such:Now the mixin is called like this:
This is done so that the color variations created by the mixin are declared as CSS custom properties first, as it would make it possible for us to switch to a darkmode with a simple class name change.
Necessity
Confirmed
composer test
).Required changes: