-
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
Try CSS custom properties for duotone #42870
base: trunk
Are you sure you want to change the base?
Conversation
Size Change: +519 B (0%) Total Size: 1.33 MB
ℹ️ View Unchanged
|
a185df3
to
1ccd556
Compare
Configure new duotone for image block Refactor withDuotoneStyles Add filter.duotone check to duotone hooks Add deprecated notice to __experimentalDuotone styles Apply duotone via --wp--style--filter in duotone.js Refactor gutenberg_render_duotone_support Add filter.duotone check in duotone.php Fix undefined and unset duotone editor styles Fix duotone rendering in php Support wrapper attributes in core/image block Apply duotone via --wp--style--filter in duotone.php Fix duotone selector in block editor Regex micro optimizations Refactor render_block_core_cover invert image check Support wrapper attributes in core/cover block Add check for stabalized duotone support in render_block hook Revert image block to use __experimentalDuotone Add back __experimentalDuotone for cover block Fix duotone for static blocks Add TODO comment for gutenberg_render_block_wrapper_attributes Simplify preg_match results
1ccd556
to
fb91635
Compare
Flaky tests detected in fb91635. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4232307773
|
I'm new to working on Duotone, so my first steps in investigating this PR are to understand the current state and then to work out next steps. ResearchWorking doc...
Duotone filter support (new!)This is the new way. Relies on setting a CSS custom property on the block wrapper and then - instead of relying on selectors in block.json) allowing this to be utilised as required in the block's
// Duotone styles
.wp-block-cover {
--wp--style--filter: initial;
> .wp-block-cover__image-background,
> .wp-block-cover__video-background {
filter: var(--wp--style--filter);
}
}
Legacy experimental duotone
.editor-styles-wrapper .wp-duotone-0 img {
filter: url(#wp-duotone-0);
}
Next Steps
|
@getdave There have been new developments since I opened this PR which is having me second guess this approach. In this comment thread I shared two possible approaches for how we could proceed with duotone: the approach in this PR and leaning into configuration with a stabilized At the time, I was leaning towards the CSS approach—it was closer to how other styles were applied, and I didn't want to write a bunch of global styles code to make the custom selectors work. But now that there are other block supports that also need inner selectors, like border, that chose to go the configuration options route, it might make more sense to lean on that work instead. We have to maintain backwards compatibility for the As for the next steps you listed, I think they're still pretty good. An additional one is deciding if this PR should be gutted and replaced with the
We do want to support presets in the attributes—that's one of the steps for fixing style variations in #46547.
@MaggieCabrera and I found and fixed a function ( |
I am unable to summarize from this PR, from the linked PRs and threads too, what is the problem with using block CSS for applying the CSS variable that we calculate / generate? Why is it better to add selectors to the blockjson api? Edit: in a chat with @scruffian the best reasons are:
|
FWIW, the CSS custom property approach does look nice and clean.
This combined with the fact we have to maintain backward compatibility for If there is anything in #46496 that we can tweak to make your efforts in refactoring duotone support easier, let me know. |
@ajlende Are you able to summarise if you see this PR having any future. If so what would be the next steps? Thanks 🙇 |
What?
Try implementing duotone using CSS custom properties and classes to apply the filter to blocks like other color block supports work.
Why?
In block.json, the
supports.color.__experimentalDuotone: boolean | string
property needs to be stabilized.Applying the filter to behave more like colors will allow more work to be done by the style engine.
How?
Add a new block.json property
supports.filter.duotone: boolean
to succeed the experimental API. Configuring duotone will require both the property and some additional CSS to indicate the child element to apply the filter to.Testing Instructions
__experimentalDuotone
blocks continue to work.Screenshots or screencast
TODO