Skip to content
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

Block selectors API: use whole selectors for duotone #49436

Closed
wants to merge 5 commits into from

Conversation

oandregal
Copy link
Member

@oandregal oandregal commented Mar 29, 2023

What

Tentative fix for #49393 (comment)

Why

The Block Selectors API is meant to land in 15.5 but duotone works differently than any other selector. The global-level duotone style is also broken.

How

  • The block selectors API for duotone takes the whole selector, not a partial one 17ab41d
  • wp_get_block_css_selector returns the whole selector for duotone, even if the old API is in use 7f6b31d
  • The duotone block-level is updated to work with whole selectors, not with partial ones 8fc3110

TODO

  • Make it work in the front-end.
  • Make it work in the post editor.
  • Make it work in the site editor.

Test

  • Create a post that contains two images (uses selectors.filter.duotone) and two cover blocks (supports.color.__experimentalDuotone).
  • Block level duotone: set the purple-green duotone for one of the images and one of the covers.
  • Global level duotone: go to the site editor > global styles > blocks. Set the midnight duotone for the image and cover blocks.

The expected result is that the blocks with block-level duotone have it applied and the other ones use the global-level duotone in any context (front-end, post editor, site editor).

No other img elements of the page have any duotone applied (check the user avatar at the top-right, for example).

image

@oandregal oandregal changed the title Use whole selectors for duotone as well Block selectors API: use whole selectors for duotone Mar 29, 2023
The old API didn't have the whole selector, just parts of it,
so we need to add the root.
@github-actions
Copy link

github-actions bot commented Mar 29, 2023

Flaky tests detected in dac72ae.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4553951873
📝 Reported issues:

@oandregal oandregal self-assigned this Mar 29, 2023
@oandregal oandregal added [Type] Bug An existing feature does not function as intended Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Feature] Block API API that allows to express the block paradigm. labels Mar 29, 2023
@oandregal
Copy link
Member Author

Current status in post editor:

  • block-level duotone for image not working
  • global-level duotone for cover not working

image

$scopes = explode( ',', '.' . $filter_id );
$selectors = explode( ',', $duotone_selector );

$selectors_scoped = array();
Copy link
Member Author

@oandregal oandregal Mar 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wish I hadn't to paste this here, but I need this to work differently than the WP_Theme_JSON_Gutenberg::scope_selector.

Ideas for a follow-up PR:

  • Create a utility css_scope_selector( $scope, $selector ) function somewhere and make it work like this code (no spaces added).
  • The consumer should be responsible for adding or not adding spaces:
    • css_scope_selector( $scope . ' ', $selector ) if the consumer wants spaces.
    • css_scope_selector( $scope . ' ', $selector ) if the consumer doesn't want spaces.

Thoughts?

$root_selectors = explode( ',', $root_selector );
$duotone_selectors = explode( ',', $partial_duotone_selector );

$duotone_selector = array();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could use the existing WP_Theme_JSON_Gutenberg::scope_selectors function, I guess. This function merits to be its own utility, given how much it has expanded: settings, duotone, WP_Theme_JSON class.

@@ -85,7 +85,6 @@
"supports": {
"anchor": true,
"color": {
"__experimentalDuotone": true,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is something I found by reviewing the duotone code (not related to this PR):

$duotone_support = (bool) $duotone_selector;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is needed to enable the filter controls in the block toolbar.

@oandregal
Copy link
Member Author

Closing in favor of #49423

@oandregal oandregal closed this Mar 29, 2023
@oandregal oandregal deleted the fix/duotone-selectors-global-and-block-level branch March 29, 2023 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Block API API that allows to express the block paradigm. Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants