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.

Sign up for GitHub

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

Jump to bottom

Block selectors API: use whole selectors for duotone #49436

Closed
wants to merge 5 commits into from
Closed

Block selectors API: use whole selectors for duotone #49436

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
17ab41d
Use whole selectors for duotone as well
oandregal Mar 29, 2023
7f6b31d
Backward compatibility for old API
oandregal Mar 29, 2023
8fc3110
Update the block-level duotone selector
oandregal Mar 29, 2023
dac72ae
Fix lint issues
oandregal Mar 29, 2023
70198b9
Remove __experimentalDuotone from image, no longer needed
oandregal Mar 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion lib/class-wp-duotone-gutenberg.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
$selectors = explode( ',', $duotone_selector );

$selectors_scoped = array();
Copy link
Member Author

@oandregal oandregal Mar 29, 2023 •
edited
Loading

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?

foreach ( $scopes as $outer ) {
foreach ( $selectors as $inner ) {
$outer = trim( $outer );
$inner = trim( $inner );
if ( ! empty( $outer ) && ! empty( $inner ) ) {
// unlike WP_Theme_JSON_Gutenberg::scope_selector
// this concatenates the selectors without a space.
$selectors_scoped[] = $outer . '' . $inner;
} elseif ( empty( $outer ) ) {
$selectors_scoped[] = $inner;
} elseif ( empty( $inner ) ) {
$selectors_scoped[] = $outer;
}
}
}
$selector = implode( ', ', $selectors_scoped );

// We only want to add the selector if we have it in the output already, essentially skipping 'unset'.
if ( array_key_exists( $slug, self::$output ) ) {
Expand Down
44 changes: 33 additions & 11 deletions lib/compat/wordpress-6.3/get-global-styles-and-settings.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,6 @@ function wp_get_block_css_selector( $block_type, $target = 'root', $fallback = f

$has_selectors = ! empty( $block_type->selectors );

// Duotone (No fallback selectors for Duotone).
if ( 'filter.duotone' === $target || array( 'filter', 'duotone' ) === $target ) {
// If selectors API in use, only use it's value or null.
if ( $has_selectors ) {
return _wp_array_get( $block_type->selectors, array( 'filter', 'duotone' ), null );
}

// Selectors API, not available, check for old experimental selector.
return _wp_array_get( $block_type->supports, array( 'color', '__experimentalDuotone' ), null );
}

// Root Selector.

// Calculated before returning as it can be used as fallback for
Expand All @@ -54,6 +43,39 @@ function wp_get_block_css_selector( $block_type, $target = 'root', $fallback = f
$root_selector = ".wp-block-{$block_name}";
}

// Duotone (No fallback selectors for Duotone).
if ( 'filter.duotone' === $target || array( 'filter', 'duotone' ) === $target ) {
// If selectors API in use, only use it's value or null.
if ( $has_selectors ) {
return _wp_array_get( $block_type->selectors, array( 'filter', 'duotone' ), null );
}

// Selectors API, not available, check for old experimental selector.
// The old API didn't have the whole selector, just parts of it,
// so we need to add the root here for backwards compatibility.
$partial_duotone_selector = _wp_array_get( $block_type->supports, array( 'color', '__experimentalDuotone' ), null );

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

foreach ( $root_selectors as $outer ) {
foreach ( $duotone_selectors as $inner ) {
$outer = trim( $outer );
$inner = trim( $inner );
if ( ! empty( $outer ) && ! empty( $inner ) ) {
$duotone_selector[] = $outer . ' ' . $inner;
} elseif ( empty( $outer ) ) {
$duotone_selector[] = $inner;
} elseif ( empty( $inner ) ) {
$duotone_selector[] = $outer;
}
}
}

return implode( ', ', $duotone_selector );
}

// Return selector if it's the root target we are looking for.
if ( 'root' === $target ) {
return $root_selector;
Expand Down
3 changes: 1 addition & 2 deletions packages/block-library/src/image/block.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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):

gutenberg/lib/class-wp-duotone-gutenberg.php

Line 281 in 70198b9

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

"text": false,
"background": false
},
Expand All @@ -104,7 +103,7 @@
"selectors": {
"border": ".wp-block-image img, .wp-block-image .wp-block-image__crop-area",
"filter": {
"duotone": "img, .components-placeholder"
"duotone": ".wp-block-image img, .wp-block-image .components-placeholder"
}
},
"styles": [
Expand Down