-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Duotone: Add Global Styles controls for blocks that support duotone #48255
Conversation
Size Change: +3.24 kB (0%) Total Size: 1.33 MB
ℹ️ View Unchanged
|
Flaky tests detected in 7b843ec. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4258338558
|
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 looking pretty good! I was surprised that unsetting the duotone filter actually worked without any any special code.
It seems like it is accidentally working because the special 'unset'
value that is used in place of the duotone array is a valid CSS value.
When unset is selected I see the following CSS generated when I update the image block.
.wp-block-image img, .wp-block-image .components-placeholder{filter: unset;}
And it looks like this user theme.json is being saved to the database.
{"styles":{"blocks":{"core\/image":{"filter":{"duotone":"unset"}}}},"isGlobalStylesUserThemeJSON":true,"version":2}
When the 'unset'
value is saved in the block supports, the block actually gets filter: none;
set in the CSS.
Ideally, I think it would be better if we didn't generate any CSS for that block if the global styles changed to unset
.
If the global styles system doesn't have a solution for preventing CSS from being generated, I think it would be better if the generated CSS used filter: none;
instead of filter: unset;
for consistency with block supports.
In my testing I caught this probable bug:
This seems wrong considering that setting a color for text of heading affects both site editor blocks and post editor blocks, because it's a global change. |
I had a go at this here: #48351 |
I added a commit to do this for Duotone only |
This is a bug that exists before this PR I believe |
It seemed redundant with the duotone one on the same page now.
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.
I made a few changes that was mostly just cleaning things up a bit.
- Slightly modified the
unset
check to fix an issue with some filter declarations not being removed from the array. - Added a comment explaining why the conditional exists.
- Renamed files to better reflect what was in them.
- Tweaked headings and descriptions in the filters screen.
@MaggieCabrera if those changes look good to you, I'd say this is ready to merge!
Thanks for the review and changes @ajlende let's get this out there! |
What?
This PR adds Global Styles controls to change duotone values for blocks.
Part of #39452
Closes #48255
Why?
Themes can define duotone presets for their blocks via theme.json but right now there's no way for users to disable or change those. This PR is a first step towards giving more agency to users.
How?
Adding a filters panel in Global Styles that contains the Duotone picker in it that overrides whatever the theme defined for the blocks.
I have disabled the custom duotone for this PR because that is not an option yet from theme.json, it only works with presets. A follow-up could bring in that functionality to this panel.
Testing Instructions
I tested this on Skatepark, which is one of the themes that adds duotone to blocks like the post featured image and the image blocks.
Go to Global Styles, select the image or post featured image block
Select filters and change the duotone color or unset it.
Save and check the block in the site editor and in the frontend, it will show your changes in both places.
Screenshots or screencast
Screen.Capture.on.2023-02-22.at.16-42-25.mp4