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

Selectors API: Make duotone selectors fallback and be scoped #49423

Merged
merged 33 commits into from
Mar 31, 2023

Conversation

aaronrobertshaw
Copy link
Contributor

@aaronrobertshaw aaronrobertshaw commented Mar 29, 2023

Related:

What?

This PR fixes global-level duotone (see #49393 (comment) for details) and make duotone work like any other selector.

Why?

  • Make every selector work the same.
  • Fix global-level duotone.

How?

  • Make duotone selectors work like any other selector:

    • blocks provide the whole selector via its block.json
    • it fall backs to the root block selector if requested
  • Updates the selectors API functions to scope the old supports.color.__experimentalDuotone selector automatically.

  • Stabilize duotone support flag under filter.duotone

    • Leverages block_type_metadata_settings filter to migrate old color.__experimentalDuotone flag.

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).

Front-end:

image

Post editor:

image

@aaronrobertshaw aaronrobertshaw added the [Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi label Mar 29, 2023
@aaronrobertshaw aaronrobertshaw self-assigned this Mar 29, 2023
@github-actions
Copy link

github-actions bot commented Mar 29, 2023

Size Change: +623 B (0%)

Total Size: 1.34 MB

Filename Size Change
build/block-editor/index.min.js 201 kB +153 B (0%)
build/block-editor/style-rtl.css 14.5 kB +11 B (0%)
build/block-editor/style.css 14.5 kB +12 B (0%)
build/block-library/index.min.js 202 kB +166 B (0%)
build/block-library/style-rtl.css 12.7 kB +2 B (0%)
build/block-library/style.css 12.7 kB +1 B (0%)
build/blocks/index.min.js 51.1 kB -4 B (0%)
build/components/index.min.js 208 kB +20 B (0%)
build/edit-post/index.min.js 34.9 kB +68 B (0%)
build/edit-site/index.min.js 63.2 kB +255 B (0%)
build/edit-widgets/index.min.js 17.3 kB -3 B (0%)
build/editor/index.min.js 45.8 kB +29 B (0%)
build/editor/style-rtl.css 3.49 kB -43 B (-1%)
build/editor/style.css 3.48 kB -44 B (-1%)
ℹ️ View Unchanged
Filename Size
build/a11y/index.min.js 993 B
build/annotations/index.min.js 2.78 kB
build/api-fetch/index.min.js 2.27 kB
build/autop/index.min.js 2.15 kB
build/blob/index.min.js 483 B
build/block-directory/index.min.js 7.2 kB
build/block-directory/style-rtl.css 1.04 kB
build/block-directory/style.css 1.04 kB
build/block-editor/content-rtl.css 4.11 kB
build/block-editor/content.css 4.1 kB
build/block-editor/default-editor-styles-rtl.css 403 B
build/block-editor/default-editor-styles.css 403 B
build/block-library/blocks/archives/editor-rtl.css 61 B
build/block-library/blocks/archives/editor.css 60 B
build/block-library/blocks/archives/style-rtl.css 90 B
build/block-library/blocks/archives/style.css 90 B
build/block-library/blocks/audio/editor-rtl.css 150 B
build/block-library/blocks/audio/editor.css 150 B
build/block-library/blocks/audio/style-rtl.css 122 B
build/block-library/blocks/audio/style.css 122 B
build/block-library/blocks/audio/theme-rtl.css 138 B
build/block-library/blocks/audio/theme.css 138 B
build/block-library/blocks/avatar/editor-rtl.css 116 B
build/block-library/blocks/avatar/editor.css 116 B
build/block-library/blocks/avatar/style-rtl.css 91 B
build/block-library/blocks/avatar/style.css 91 B
build/block-library/blocks/block/editor-rtl.css 305 B
build/block-library/blocks/block/editor.css 305 B
build/block-library/blocks/button/editor-rtl.css 587 B
build/block-library/blocks/button/editor.css 587 B
build/block-library/blocks/button/style-rtl.css 628 B
build/block-library/blocks/button/style.css 627 B
build/block-library/blocks/buttons/editor-rtl.css 337 B
build/block-library/blocks/buttons/editor.css 337 B
build/block-library/blocks/buttons/style-rtl.css 332 B
build/block-library/blocks/buttons/style.css 332 B
build/block-library/blocks/calendar/style-rtl.css 239 B
build/block-library/blocks/calendar/style.css 239 B
build/block-library/blocks/categories/editor-rtl.css 113 B
build/block-library/blocks/categories/editor.css 112 B
build/block-library/blocks/categories/style-rtl.css 124 B
build/block-library/blocks/categories/style.css 124 B
build/block-library/blocks/code/editor-rtl.css 53 B
build/block-library/blocks/code/editor.css 53 B
build/block-library/blocks/code/style-rtl.css 121 B
build/block-library/blocks/code/style.css 121 B
build/block-library/blocks/code/theme-rtl.css 124 B
build/block-library/blocks/code/theme.css 124 B
build/block-library/blocks/columns/editor-rtl.css 108 B
build/block-library/blocks/columns/editor.css 108 B
build/block-library/blocks/columns/style-rtl.css 406 B
build/block-library/blocks/columns/style.css 406 B
build/block-library/blocks/comment-author-avatar/editor-rtl.css 125 B
build/block-library/blocks/comment-author-avatar/editor.css 125 B
build/block-library/blocks/comment-content/style-rtl.css 92 B
build/block-library/blocks/comment-content/style.css 92 B
build/block-library/blocks/comment-template/style-rtl.css 199 B
build/block-library/blocks/comment-template/style.css 198 B
build/block-library/blocks/comments-pagination-numbers/editor-rtl.css 123 B
build/block-library/blocks/comments-pagination-numbers/editor.css 121 B
build/block-library/blocks/comments-pagination/editor-rtl.css 222 B
build/block-library/blocks/comments-pagination/editor.css 209 B
build/block-library/blocks/comments-pagination/style-rtl.css 235 B
build/block-library/blocks/comments-pagination/style.css 231 B
build/block-library/blocks/comments-title/editor-rtl.css 75 B
build/block-library/blocks/comments-title/editor.css 75 B
build/block-library/blocks/comments/editor-rtl.css 840 B
build/block-library/blocks/comments/editor.css 839 B
build/block-library/blocks/comments/style-rtl.css 637 B
build/block-library/blocks/comments/style.css 636 B
build/block-library/blocks/cover/editor-rtl.css 612 B
build/block-library/blocks/cover/editor.css 613 B
build/block-library/blocks/cover/style-rtl.css 1.6 kB
build/block-library/blocks/cover/style.css 1.59 kB
build/block-library/blocks/embed/editor-rtl.css 293 B
build/block-library/blocks/embed/editor.css 293 B
build/block-library/blocks/embed/style-rtl.css 410 B
build/block-library/blocks/embed/style.css 410 B
build/block-library/blocks/embed/theme-rtl.css 138 B
build/block-library/blocks/embed/theme.css 138 B
build/block-library/blocks/file/editor-rtl.css 300 B
build/block-library/blocks/file/editor.css 300 B
build/block-library/blocks/file/style-rtl.css 265 B
build/block-library/blocks/file/style.css 265 B
build/block-library/blocks/file/view.min.js 353 B
build/block-library/blocks/freeform/editor-rtl.css 2.44 kB
build/block-library/blocks/freeform/editor.css 2.44 kB
build/block-library/blocks/gallery/editor-rtl.css 984 B
build/block-library/blocks/gallery/editor.css 988 B
build/block-library/blocks/gallery/style-rtl.css 1.55 kB
build/block-library/blocks/gallery/style.css 1.55 kB
build/block-library/blocks/gallery/theme-rtl.css 122 B
build/block-library/blocks/gallery/theme.css 122 B
build/block-library/blocks/group/editor-rtl.css 654 B
build/block-library/blocks/group/editor.css 654 B
build/block-library/blocks/group/style-rtl.css 57 B
build/block-library/blocks/group/style.css 57 B
build/block-library/blocks/group/theme-rtl.css 78 B
build/block-library/blocks/group/theme.css 78 B
build/block-library/blocks/heading/style-rtl.css 76 B
build/block-library/blocks/heading/style.css 76 B
build/block-library/blocks/html/editor-rtl.css 332 B
build/block-library/blocks/html/editor.css 333 B
build/block-library/blocks/image/editor-rtl.css 830 B
build/block-library/blocks/image/editor.css 829 B
build/block-library/blocks/image/style-rtl.css 652 B
build/block-library/blocks/image/style.css 652 B
build/block-library/blocks/image/theme-rtl.css 137 B
build/block-library/blocks/image/theme.css 137 B
build/block-library/blocks/latest-comments/style-rtl.css 357 B
build/block-library/blocks/latest-comments/style.css 357 B
build/block-library/blocks/latest-posts/editor-rtl.css 213 B
build/block-library/blocks/latest-posts/editor.css 212 B
build/block-library/blocks/latest-posts/style-rtl.css 478 B
build/block-library/blocks/latest-posts/style.css 478 B
build/block-library/blocks/list/style-rtl.css 88 B
build/block-library/blocks/list/style.css 88 B
build/block-library/blocks/media-text/editor-rtl.css 266 B
build/block-library/blocks/media-text/editor.css 263 B
build/block-library/blocks/media-text/style-rtl.css 507 B
build/block-library/blocks/media-text/style.css 505 B
build/block-library/blocks/more/editor-rtl.css 431 B
build/block-library/blocks/more/editor.css 431 B
build/block-library/blocks/navigation-link/editor-rtl.css 716 B
build/block-library/blocks/navigation-link/editor.css 715 B
build/block-library/blocks/navigation-link/style-rtl.css 115 B
build/block-library/blocks/navigation-link/style.css 115 B
build/block-library/blocks/navigation-submenu/editor-rtl.css 299 B
build/block-library/blocks/navigation-submenu/editor.css 299 B
build/block-library/blocks/navigation/editor-rtl.css 2.13 kB
build/block-library/blocks/navigation/editor.css 2.14 kB
build/block-library/blocks/navigation/style-rtl.css 2.22 kB
build/block-library/blocks/navigation/style.css 2.21 kB
build/block-library/blocks/navigation/view-modal.min.js 2.81 kB
build/block-library/blocks/navigation/view.min.js 447 B
build/block-library/blocks/nextpage/editor-rtl.css 395 B
build/block-library/blocks/nextpage/editor.css 395 B
build/block-library/blocks/page-list/editor-rtl.css 401 B
build/block-library/blocks/page-list/editor.css 401 B
build/block-library/blocks/page-list/style-rtl.css 175 B
build/block-library/blocks/page-list/style.css 175 B
build/block-library/blocks/paragraph/editor-rtl.css 174 B
build/block-library/blocks/paragraph/editor.css 174 B
build/block-library/blocks/paragraph/style-rtl.css 279 B
build/block-library/blocks/paragraph/style.css 281 B
build/block-library/blocks/post-author/style-rtl.css 175 B
build/block-library/blocks/post-author/style.css 176 B
build/block-library/blocks/post-comments-form/editor-rtl.css 96 B
build/block-library/blocks/post-comments-form/editor.css 96 B
build/block-library/blocks/post-comments-form/style-rtl.css 501 B
build/block-library/blocks/post-comments-form/style.css 501 B
build/block-library/blocks/post-date/style-rtl.css 61 B
build/block-library/blocks/post-date/style.css 61 B
build/block-library/blocks/post-excerpt/editor-rtl.css 71 B
build/block-library/blocks/post-excerpt/editor.css 71 B
build/block-library/blocks/post-excerpt/style-rtl.css 134 B
build/block-library/blocks/post-excerpt/style.css 134 B
build/block-library/blocks/post-featured-image/editor-rtl.css 588 B
build/block-library/blocks/post-featured-image/editor.css 586 B
build/block-library/blocks/post-featured-image/style-rtl.css 322 B
build/block-library/blocks/post-featured-image/style.css 322 B
build/block-library/blocks/post-navigation-link/style-rtl.css 153 B
build/block-library/blocks/post-navigation-link/style.css 153 B
build/block-library/blocks/post-template/editor-rtl.css 99 B
build/block-library/blocks/post-template/editor.css 98 B
build/block-library/blocks/post-template/style-rtl.css 281 B
build/block-library/blocks/post-template/style.css 281 B
build/block-library/blocks/post-terms/style-rtl.css 96 B
build/block-library/blocks/post-terms/style.css 96 B
build/block-library/blocks/post-time-to-read/style-rtl.css 69 B
build/block-library/blocks/post-time-to-read/style.css 69 B
build/block-library/blocks/post-title/style-rtl.css 100 B
build/block-library/blocks/post-title/style.css 100 B
build/block-library/blocks/preformatted/style-rtl.css 103 B
build/block-library/blocks/preformatted/style.css 103 B
build/block-library/blocks/pullquote/editor-rtl.css 135 B
build/block-library/blocks/pullquote/editor.css 135 B
build/block-library/blocks/pullquote/style-rtl.css 326 B
build/block-library/blocks/pullquote/style.css 325 B
build/block-library/blocks/pullquote/theme-rtl.css 167 B
build/block-library/blocks/pullquote/theme.css 167 B
build/block-library/blocks/query-pagination-numbers/editor-rtl.css 122 B
build/block-library/blocks/query-pagination-numbers/editor.css 121 B
build/block-library/blocks/query-pagination/editor-rtl.css 221 B
build/block-library/blocks/query-pagination/editor.css 211 B
build/block-library/blocks/query-pagination/style-rtl.css 288 B
build/block-library/blocks/query-pagination/style.css 284 B
build/block-library/blocks/query-title/style-rtl.css 63 B
build/block-library/blocks/query-title/style.css 63 B
build/block-library/blocks/query/editor-rtl.css 463 B
build/block-library/blocks/query/editor.css 463 B
build/block-library/blocks/quote/style-rtl.css 222 B
build/block-library/blocks/quote/style.css 222 B
build/block-library/blocks/quote/theme-rtl.css 223 B
build/block-library/blocks/quote/theme.css 226 B
build/block-library/blocks/read-more/style-rtl.css 132 B
build/block-library/blocks/read-more/style.css 132 B
build/block-library/blocks/rss/editor-rtl.css 149 B
build/block-library/blocks/rss/editor.css 149 B
build/block-library/blocks/rss/style-rtl.css 289 B
build/block-library/blocks/rss/style.css 288 B
build/block-library/blocks/search/editor-rtl.css 165 B
build/block-library/blocks/search/editor.css 165 B
build/block-library/blocks/search/style-rtl.css 408 B
build/block-library/blocks/search/style.css 406 B
build/block-library/blocks/search/theme-rtl.css 114 B
build/block-library/blocks/search/theme.css 114 B
build/block-library/blocks/separator/editor-rtl.css 146 B
build/block-library/blocks/separator/editor.css 146 B
build/block-library/blocks/separator/style-rtl.css 234 B
build/block-library/blocks/separator/style.css 234 B
build/block-library/blocks/separator/theme-rtl.css 194 B
build/block-library/blocks/separator/theme.css 194 B
build/block-library/blocks/shortcode/editor-rtl.css 474 B
build/block-library/blocks/shortcode/editor.css 474 B
build/block-library/blocks/site-logo/editor-rtl.css 489 B
build/block-library/blocks/site-logo/editor.css 489 B
build/block-library/blocks/site-logo/style-rtl.css 203 B
build/block-library/blocks/site-logo/style.css 203 B
build/block-library/blocks/site-tagline/editor-rtl.css 86 B
build/block-library/blocks/site-tagline/editor.css 86 B
build/block-library/blocks/site-title/editor-rtl.css 116 B
build/block-library/blocks/site-title/editor.css 116 B
build/block-library/blocks/site-title/style-rtl.css 57 B
build/block-library/blocks/site-title/style.css 57 B
build/block-library/blocks/social-link/editor-rtl.css 184 B
build/block-library/blocks/social-link/editor.css 184 B
build/block-library/blocks/social-links/editor-rtl.css 674 B
build/block-library/blocks/social-links/editor.css 673 B
build/block-library/blocks/social-links/style-rtl.css 1.4 kB
build/block-library/blocks/social-links/style.css 1.39 kB
build/block-library/blocks/spacer/editor-rtl.css 332 B
build/block-library/blocks/spacer/editor.css 332 B
build/block-library/blocks/spacer/style-rtl.css 48 B
build/block-library/blocks/spacer/style.css 48 B
build/block-library/blocks/table/editor-rtl.css 433 B
build/block-library/blocks/table/editor.css 433 B
build/block-library/blocks/table/style-rtl.css 651 B
build/block-library/blocks/table/style.css 650 B
build/block-library/blocks/table/theme-rtl.css 157 B
build/block-library/blocks/table/theme.css 157 B
build/block-library/blocks/tag-cloud/style-rtl.css 251 B
build/block-library/blocks/tag-cloud/style.css 253 B
build/block-library/blocks/template-part/editor-rtl.css 404 B
build/block-library/blocks/template-part/editor.css 404 B
build/block-library/blocks/template-part/theme-rtl.css 101 B
build/block-library/blocks/template-part/theme.css 101 B
build/block-library/blocks/text-columns/editor-rtl.css 95 B
build/block-library/blocks/text-columns/editor.css 95 B
build/block-library/blocks/text-columns/style-rtl.css 166 B
build/block-library/blocks/text-columns/style.css 166 B
build/block-library/blocks/verse/style-rtl.css 99 B
build/block-library/blocks/verse/style.css 99 B
build/block-library/blocks/video/editor-rtl.css 552 B
build/block-library/blocks/video/editor.css 555 B
build/block-library/blocks/video/style-rtl.css 179 B
build/block-library/blocks/video/style.css 179 B
build/block-library/blocks/video/theme-rtl.css 139 B
build/block-library/blocks/video/theme.css 139 B
build/block-library/classic-rtl.css 179 B
build/block-library/classic.css 179 B
build/block-library/common-rtl.css 1.11 kB
build/block-library/common.css 1.11 kB
build/block-library/editor-elements-rtl.css 75 B
build/block-library/editor-elements.css 75 B
build/block-library/editor-rtl.css 11.6 kB
build/block-library/editor.css 11.6 kB
build/block-library/elements-rtl.css 54 B
build/block-library/elements.css 54 B
build/block-library/reset-rtl.css 478 B
build/block-library/reset.css 478 B
build/block-library/theme-rtl.css 698 B
build/block-library/theme.css 703 B
build/block-serialization-default-parser/index.min.js 1.13 kB
build/block-serialization-spec-parser/index.min.js 2.83 kB
build/components/style-rtl.css 11.7 kB
build/components/style.css 11.7 kB
build/compose/index.min.js 12.4 kB
build/core-data/index.min.js 16.3 kB
build/customize-widgets/index.min.js 12.2 kB
build/customize-widgets/style-rtl.css 1.41 kB
build/customize-widgets/style.css 1.41 kB
build/data-controls/index.min.js 663 B
build/data/index.min.js 8.68 kB
build/date/index.min.js 40.4 kB
build/deprecated/index.min.js 518 B
build/dom-ready/index.min.js 336 B
build/dom/index.min.js 4.72 kB
build/edit-post/classic-rtl.css 571 B
build/edit-post/classic.css 571 B
build/edit-post/style-rtl.css 7.6 kB
build/edit-post/style.css 7.59 kB
build/edit-site/style-rtl.css 10.1 kB
build/edit-site/style.css 10.1 kB
build/edit-widgets/style-rtl.css 4.56 kB
build/edit-widgets/style.css 4.56 kB
build/element/index.min.js 4.95 kB
build/escape-html/index.min.js 548 B
build/format-library/index.min.js 7.26 kB
build/format-library/style-rtl.css 557 B
build/format-library/style.css 556 B
build/hooks/index.min.js 1.66 kB
build/html-entities/index.min.js 454 B
build/i18n/index.min.js 3.79 kB
build/is-shallow-equal/index.min.js 535 B
build/keyboard-shortcuts/index.min.js 1.79 kB
build/keycodes/index.min.js 1.94 kB
build/list-reusable-blocks/index.min.js 2.14 kB
build/list-reusable-blocks/style-rtl.css 865 B
build/list-reusable-blocks/style.css 865 B
build/media-utils/index.min.js 2.99 kB
build/notices/index.min.js 977 B
build/plugins/index.min.js 1.94 kB
build/preferences-persistence/index.min.js 2.23 kB
build/preferences/index.min.js 1.35 kB
build/primitives/index.min.js 960 B
build/priority-queue/index.min.js 1.52 kB
build/private-apis/index.min.js 937 B
build/react-i18n/index.min.js 702 B
build/react-refresh-entry/index.min.js 8.44 kB
build/react-refresh-runtime/index.min.js 7.31 kB
build/redux-routine/index.min.js 2.75 kB
build/reusable-blocks/index.min.js 2.26 kB
build/reusable-blocks/style-rtl.css 265 B
build/reusable-blocks/style.css 265 B
build/rich-text/index.min.js 11.1 kB
build/server-side-render/index.min.js 2.09 kB
build/shortcode/index.min.js 1.52 kB
build/style-engine/index.min.js 1.55 kB
build/token-list/index.min.js 650 B
build/url/index.min.js 3.74 kB
build/vendors/inert-polyfill.min.js 2.48 kB
build/vendors/react-dom.min.js 41.8 kB
build/vendors/react.min.js 4.02 kB
build/viewport/index.min.js 1.09 kB
build/warning/index.min.js 280 B
build/widgets/index.min.js 7.3 kB
build/widgets/style-rtl.css 1.18 kB
build/widgets/style.css 1.18 kB
build/wordcount/index.min.js 1.06 kB

compressed-size-action

Base automatically changed from fix/selectors-api to trunk March 29, 2023 07:41
@aaronrobertshaw aaronrobertshaw force-pushed the fix/selectors-api-use-in-duotone branch from a0fde9e to 520ea49 Compare March 29, 2023 07:46
return _wp_array_get( $block_type->selectors, array( 'filter', 'duotone' ), $fallback_selector );
}

// Selectors API, not available, check for old experimental selector.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it worth saying that this is left in for backwards compatibility, and should be removed one day?

@oandregal oandregal force-pushed the fix/selectors-api-use-in-duotone branch from 520ea49 to 4f4fc3c Compare March 29, 2023 14:46
@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 labels Mar 29, 2023
@@ -349,7 +349,26 @@ public static function render_duotone_support( $block_content, $block ) {
$filter_id = gutenberg_get_duotone_filter_id( array( 'slug' => $slug ) );

// Build the CSS selectors to which the filter will be applied.
$selector = WP_Theme_JSON_Gutenberg::scope_selector( '.' . $filter_id, $duotone_selector );
$scopes = explode( ',', '.' . $filter_id );
Copy link
Member

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 this needs to work differently than the WP_Theme_JSON_Gutenberg::scope_selector (do not add an extra space).

Ideas for a follow-up PR:

  • The usage of this internal utility has expanded to: settings, duotone, WP_Theme_JSON class. It's worth creating a utility function somewhere. Where? wp_scope_css_selector( $scope, $selector ) as a name?

  • The consumer should be responsible for adding or not adding spaces, not the function itself:

    • wp_scope_css_selector( $scope . ' ', $selector ) if the consumer wants spaces.
    • wp_scope_css_selector( $scope . ' ', $selector ) if the consumer doesn't want spaces.

Thoughts?

@github-actions
Copy link

github-actions bot commented Mar 29, 2023

Flaky tests detected in 7e7bf1c.
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/4567826409
📝 Reported issues:

@oandregal oandregal marked this pull request as ready for review March 29, 2023 16:00
Copy link
Member

@oandregal oandregal left a comment

Choose a reason for hiding this comment

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

This is working as expected.

@oandregal oandregal force-pushed the fix/selectors-api-use-in-duotone branch from cc6e325 to 64a505a Compare March 29, 2023 16:09
*
* @return {string} Scoped selector.
*/
function scopeSelector( scope, 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 it probably makes more sense to keep this function but just modify it.

@@ -450,3 +471,4 @@ function gutenberg_render_duotone_support( $block_content, $block ) {
add_action( 'wp_enqueue_scripts', array( 'WP_Duotone_Gutenberg', 'output_global_styles' ), 11 );
add_action( 'wp_footer', array( 'WP_Duotone_Gutenberg', 'output_footer_assets' ), 10 );
add_filter( 'block_editor_settings_all', array( 'WP_Duotone_Gutenberg', 'add_editor_settings' ), 10 );
add_filter( 'block_type_metadata_settings', 'gutenberg_migrate_experimental_duotone_support_flag', 10, 2 );
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like a useful technique for migrating more settings over.

Copy link
Contributor

@ajlende ajlende Mar 30, 2023

Choose a reason for hiding this comment

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

The only problem is that there's no longer any way of knowing if you have experimental support or not. It's causing me problems because the selector that we choose is different depending on whether or not it's experimental.

// May be true even if `filter.duotone` isn't set.
const duotoneSupport = getBlockSupport(
	blockType,
	'filter.duotone',
	false
);

// May be set even if `filter.duotone` is also set.
const experimentalDuotone = getBlockSupport(
	blockType,
	'color.__experimentalDuotone',
	false
);

// Will always produce a selector even if `filter` or `filter.duotone` isn't set.
const duotoneSelector = getBlockCSSSelector(
	blockType,
	'filter.duotone',
	{ fallback: true }
);

EDIT: We could have color.__experimentalDuotone override filter.duotone, but that seems wrong to me. If they're both set, I would expect filter.duotone to override color.__experimentalDuotone.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure I follow.

In the filter, we were only setting filter.duotone if it hasn't already been set and the color.experimentalDuotone has been.

A block author shouldn't be using both properties in their raw block.json so if we have color.experimentalDuotone set at all, then it should be a case from when duotone was experimental.

  • In your first example, the check for filter.duotone tells us if there is support.
  • The second example if the default is removed or changed to null would tell us if the duotone support was the experimental version.
  • The third snippet with fallback true would always produce a selector regardless of any support check. We would guard such a call with a check that the color.experimentalDuotone support flag wasn't set.

Copy link
Contributor Author

@aaronrobertshaw aaronrobertshaw Mar 30, 2023

Choose a reason for hiding this comment

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

My thinking was that by leaving the __experimentalDuotone property we could determine when to use that as the selector. I can see from this discussion where I missed adding the guard check to the duotone.js HoC.

Copy link
Contributor

@scruffian scruffian left a comment

Choose a reason for hiding this comment

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

This seems to be working as expected to me, and the code looks good, but I'd rather wait on @ajlende to merge it.

Copy link
Contributor

@ajlende ajlende left a comment

Choose a reason for hiding this comment

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

@aaronrobertshaw The changes I made ended up being kind of big again, so I didn't want to merge right away without someone else taking a look. I have nothing else to add, so if things are working well for you, go ahead and merge it.

Comment on lines 315 to 320
// Backwards compatibility for color.__experimentalDuotone. This will
// have priority over filter.duotone support. Unfortunately we can't
// prefer filter.duotone because it gets set when __experimentalDuotone
// is set via a block_type_metadata_settings hook. It shouldn't be too
// much of a problem because I would expect consumers to not use both
// at the same time.
Copy link
Contributor

Choose a reason for hiding this comment

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

I had to do this to fix the experimental selectors in the editor being applied to the root of the block instead of the selector set in __experimentalSelector.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure we're on the same page here. The supports.filter.duotone property will only get set if it hasn't been set already and we have supports.color.__experimentalDuotone set.

So supports.filter.duotone will tell us if we have duotone support one way or another. Checking if we have the supports.color.__experimentalDuotone flag set should tell us if we need to handle an experimental duotone selector or use the selectors API requesting selectors.filter.duotone

@aaronrobertshaw
Copy link
Contributor Author

I've updated the comments added around backwards compatibility of supports.color.__experimentalDuotone to match the filter that only updates supports.filter.duotone if it wasn't already set.

We can also return earlier if there is no duotone support so I relocated those guard conditions as well in 3346b9a

I've retested this:

✅ Global styles can be applied to all current blocks opting into __experimentalDuotone
✅ Global styles are applied correctly for the Image block that uses supports.filter.duotone
✅ Individual block styles in the editor can be applied for blocks using __experimentalDuotone
✅ Individual block styles work for the Image block using supports.filter.duotone
✅ Duotone filters via global styles apply for all blocks on frontend
✅ Individual block duotone filters override global duotone filters on the frontend

I'll get some fresh eyes on this PR and look to merge it later this afternoon.

Copy link
Contributor

@glendaviesnz glendaviesnz left a comment

Choose a reason for hiding this comment

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

I have retested after your latest changes @aaronrobertshaw and the following still works for me:

✅ Global styles can be applied to all current blocks opting into __experimentalDuotone
✅ Global styles are applied correctly for the Image block that uses supports.filter.duotone
✅ Individual block styles in the editor can be applied for blocks using __experimentalDuotone
✅ Individual block styles work for the Image block using supports.filter.duotone
✅ Duotone filters via global styles apply for all blocks on frontend
✅ Individual block duotone filters override global duotone filters on the frontend

@aaronrobertshaw aaronrobertshaw merged commit c2b4b1e into trunk Mar 31, 2023
@aaronrobertshaw aaronrobertshaw deleted the fix/selectors-api-use-in-duotone branch March 31, 2023 02:23
@github-actions github-actions bot added this to the Gutenberg 15.6 milestone Mar 31, 2023
@ryanwelcher
Copy link
Contributor

I just cherry-picked this PR to the release/15.5 branch to get it included in the next release: fa6e2a6

@ryanwelcher ryanwelcher removed the Backport to Gutenberg RC Pull request that needs to be backported to a Gutenberg release candidate (RC) label Apr 5, 2023
@ramonjd ramonjd added the Needs PHP backport Needs PHP backport to Core label Jun 5, 2023
@ramonjd ramonjd removed the Needs PHP backport Needs PHP backport to Core label Jun 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi 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.

7 participants