Global Styles: Filter out color and typography variations #60220
+80
−18
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
scruffian
added
the
Global Styles
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Size Change: +114 B (+0.01%) Total Size: 1.74 MB
ℹ️ View Unchanged
|
|
Works as expected. What do you think about making an exception for I've found that the typography changes to a button almost always result in a necessary change to the button size via padding.
|
|
One odd bit I can't nail down. I'm testing with this typography-only variation, but it's showing up under colors (I removed the button padding for now). Any ideas? |
draganescu
reviewed
Apr 1, 2024
packages/edit-site/src/components/global-styles/style-variations-container.js
Outdated
Show resolved
Hide resolved
...ges/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js
Show resolved
Hide resolved
packages/edit-site/src/components/global-styles/style-variations-container.js
Outdated
Show resolved
Hide resolved
I don't love targeting a specific exception—what about allowing typography and spacing values for typesets? I think that'd work, as when changing font size you may want to change the size of elements (particularly padding), and maintaining support for all typography values is important to maintaining creative expression. |
scruffian
force-pushed
the
update/global-styles-filter-color-and-typography-variations
branch
from
3a3c133
to
7ad5f93
Compare
|
There are several things missing from this PR:
|
|
I have updated this so that:
|
This is happening because we filter out typography variations where the heading and body font are the same. I'm not sure we should keep doing that, what do you think? |
Well, they shouldn't render as colors though. |
Correct, that was a different bug that should be fixed in this PR |
|
Ah gotcha, what do you think about allowing |
Let's follow up with adding spacing as part of the typography presets. |
This was referenced Apr 29, 2024
scruffian
force-pushed
the
update/global-styles-filter-color-and-typography-variations
branch
from
6680eb1
to
4687ad8
Compare
|
Flaky tests detected in 4687ad8. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/8922802060
|
…ns-container.js Co-authored-by: Andrei Draganescu <me@andreidraganescu.info>
…ns-container.js Co-authored-by: Andrei Draganescu <me@andreidraganescu.info>
scruffian
force-pushed
the
update/global-styles-filter-color-and-typography-variations
branch
from
4687ad8
to
57c7862
Compare
scruffian
deleted the
update/global-styles-filter-color-and-typography-variations
branch
|
I just noticed the only TT4 dark theme variation disappeared from the theme styles list, and it appears to be a result of this PR. From reading the description above it seems to be deliberate, but isn't this a back compat break? Shouldn't we still show all style variations that theme authors created as such? I'd also note that, while I eventually found what I wanted (a color scheme with a dark background) in the colors section below, I had to click through every color palette in order to find it. It's not obvious from those palettes which color goes where, unlike with the style variations previews. Couldn't we have a preview for the colors too? |
I wouldn't think so. It's a color-only variation that would be active under the same conditions that that style variation is also active with. |
Do you mean like a BlockPreview for the colors? That's an interesting idea; worth trying for sure. |
Something like that! From looking at the below, it's really not clear which are the background/text colors: 
|
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.
What?
Now that Global Styles displays color and typography settings separately we should find a way to allow theme authors to give access to these without needing to create full variations.
This filters color and typography variations out of the list of style variations so that theme authors can add variations that target only these properties without having to create full variations.
Related #56604
Why?
This gives theme authors the ability to create multiple color and typography settings.
How?
Iterate over the style variations. For each one we extract the color settings/styles and typography settings/styles. If either one is the same as the variation itself then this variation must be a color/typography only setting, so we should remove it from the main list.
Testing Instructions
Screenshots or screencast