-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Components: Update default accent color #50193
Changes from all commits
835a15d
3f3e24f
acebc97
a601a3a
96f8924
33c33a5
a8582f8
b7e975b
09354d8
cc107d5
084c789
4f12c0d
99ef87a
f64b518
3a57e2b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -163,10 +163,12 @@ | |
padding: $grid-unit-15 * 0.5; // This reduces the horizontal padding on tertiary/text buttons, so as to space them optically. | ||
|
||
&:hover:not(:disabled) { | ||
// TODO: Prepare for theming (https://github.com/WordPress/gutenberg/pull/45466/files#r1030872724) | ||
background: rgba(var(--wp-admin-theme-color--rgb), 0.04); | ||
} | ||
|
||
&:active:not(:disabled) { | ||
// TODO: Prepare for theming (https://github.com/WordPress/gutenberg/pull/45466/files#r1030872724) | ||
Comment on lines
+166
to
+171
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These styles were added after our initial theming effort. Adding a TODO comment now. |
||
background: rgba(var(--wp-admin-theme-color--rgb), 0.08); | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -53,9 +53,9 @@ describe( 'Theme color algorithms', () => { | |
'wp.components.Theme: The background color ("#000") does not have sufficient contrast against the accent color ("#111").' | ||
); | ||
|
||
generateThemeVariables( { background: '#eee' } ); | ||
generateThemeVariables( { background: '#1a1a1a' } ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changing this to a color that does trigger a contrast error with the new accent color. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you expand on this? We still want to use the existing grayscale for components:
While we can update them, we should be careful in doing so as it affects so much componentry. So I would do that separately from this PR. Which warning does it trigger? And it's good that it does, it's another reason to merge this soon so we can address those. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, no worries, this is just a unit test that verifies that our contrast-checking algorithm is working as expected 😄 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, sounds great. Thanks again! |
||
expect( console ).toHaveWarnedWith( | ||
'wp.components.Theme: The background color ("#eee") does not have sufficient contrast against the accent color ("#007cba").' | ||
'wp.components.Theme: The background color ("#1a1a1a") does not have sufficient contrast against the accent color ("#3858e9").' | ||
); | ||
} ); | ||
|
||
|
This file was deleted.
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.
This is the part in the build process where all the
--wp-admin-theme-color*
variables are prepended to the main stylesheet of each package.By excluding the
components
package from this step, the built stylesheet of wp-components will no longer include the default admin scheme vars.Instead, we will manually include the admin scheme variables for the Blueberry color.
Eventually, we should stop providing
--wp-admin-theme-*
variables in the wp-components package.