Components: Update default accent color #50193
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mirka
added
[Package] Components
|
Size Change: -378 B (0%) Total Size: 1.37 MB
ℹ️ View Unchanged
|
jasmussen
reviewed
May 1, 2023
jasmussen
reviewed
May 1, 2023
jasmussen
reviewed
May 1, 2023
jasmussen
approved these changes
May 1, 2023
mirka
commented
May 1, 2023
Comment on lines
+166
to
+171
| // 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) |
There was a problem hiding this comment.
These styles were added after our initial theming effort. Adding a TODO comment now.
Comment on lines
+106
to
+112
| // Editor and component styles should be excluded from the default CSS vars output. | ||
| .concat( | ||
| file.includes( 'common.scss' ) || ! file.includes( 'block-library' ) | ||
| file.includes( 'common.scss' ) || | ||
| ! ( | ||
| file.includes( 'block-library' ) || | ||
| file.includes( 'components' ) | ||
| ) |
There was a problem hiding this comment.
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.
mirka
commented
May 2, 2023
| @@ -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.
Changing this to a color that does trigger a contrast error with the new accent color.
There was a problem hiding this comment.
Can you expand on this? We still want to use the existing grayscale for components:
// WordPress grays.
$black: #000; // Use only when you truly need pure black. For UI, use $gray-900.
$gray-900: #1e1e1e;
$gray-800: #2f2f2f;
$gray-700: #757575; // Meets 4.6:1 text contrast against white.
$gray-600: #949494; // Meets 3:1 UI or large text contrast against white.
$gray-400: #ccc;
$gray-300: #ddd; // Used for most borders.
$gray-200: #e0e0e0; // Used sparingly for light borders.
$gray-100: #f0f0f0; // Used for light gray backgrounds.
$white: #fff;
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.
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.
Ah, sounds great. Thanks again!
This reverts commit a8582f8.
mirka
commented
May 2, 2023
storybook/style.scss
Outdated
There was a problem hiding this comment.
These base styles are/should be provided by the wp packages themselves.
mirka
removed
the
Needs Dev Note
|
Removing the "Needs Dev Note" tag because I realized that this change does not affect anyone consuming the package via a WP release (only affects npm consumers). Instead, I added back compat instructions in the PR description and pointed to it in the changelog entry. |
Do you think we should consider adding those instructions in some docs (like in the README) ? |
Not really. This affects a very small segment of consumers, and if I were such a consumer that noticed this change in my app I would first look in the changelog. Wdyt? |
|
Agree, it's not a big deal. If anything it's a bugfix :) |
ciampo
approved these changes
May 5, 2023
storybook/style.scss
Outdated
There was a problem hiding this comment.
Why is this file not necessary anymore? Was its actual only purpose to load --wp-admin--* variables?
There was a problem hiding this comment.
In fact I don't know if this file was ever needed. The packages we showcase in Storybook already have the --wp-admin-* variables loaded in their main stylesheets, and any Sass dependency imports should already have been dealt with at package build time before anything hits Storybook.
|
Thanks for this one! |
This was referenced May 22, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #44123
What?
Updates the default accent color scheme of the package from
#007cbato#3858e9.This will only affect consumers outside of a WordPress context.
Why?
As requested by @jasmussen in #43994 (comment), we want to start embracing the new color scheme in the components package.
How?
See inline comments.
Testing Instructions
Should update wp-component defaults
npm run storybook:devShould not affect wp-admin usage
npm run dev/wp-admin/profile.phpand set the admin color scheme to something that is not "Modern".🛠️ How to restore previous color scheme
This only affects those using the
@wordpress/componentspackage in a non-WordPress context.The default color scheme of the
@wordpress/componentshas been updated to use the new WP Blueberry (#3858e9).To restore the previous color scheme in your app, create a stylesheet that imports
default-custom-propertiesfrom the@wordpress/base-stylespackage:Then, load this stylesheet later than the
@wordpress/componentsstylesheet (@wordpress/components/build-style/style.css).