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
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
7735980
Make duotone selectors fallback and be scoped
aaronrobertshaw Mar 29, 2023
0c52d64
Use whole selectors for duotone as well
oandregal Mar 29, 2023
02bfc17
Update the block-level duotone selector
oandregal Mar 29, 2023
e5ed227
Use existing function
oandregal Mar 29, 2023
e60c4b7
scope the selectors in the post editor
scruffian Mar 29, 2023
555f7d0
Duotone: do not scope twice
oandregal Mar 29, 2023
64a505a
Simplify __experimentalDuotone backwards compatibility
Mar 29, 2023
3cd86f0
Update the __experimentalDuotoen back compat to explicitly handle eac…
Mar 29, 2023
7d370c6
Simplify adding the filter class to the selectors
Mar 29, 2023
521db54
Refactor to handle __experimentalDuotone where it was previously handled
Mar 30, 2023
fc2f0c0
Remove some dead code that I missed
Mar 30, 2023
53410e5
Fix lint
Mar 30, 2023
42e1b3f
Remove __experimentalDuotone test since it doesn't handle that anymore
Mar 30, 2023
50997ca
Remove remainder of duotone tests because it is no longer handled dif…
Mar 30, 2023
b9d16ea
Fix getBlockSelectors test because __experimentalDuotone now always r…
Mar 30, 2023
31aa06e
Fix rendering when preset doesn't exist
Mar 30, 2023
114ec2b
Fix unset filters
Mar 30, 2023
da60b58
Fix typos
aaronrobertshaw Mar 30, 2023
6a750e3
Update docs
aaronrobertshaw Mar 30, 2023
2236ce5
Tweak incorrect comment
aaronrobertshaw Mar 30, 2023
972dca7
Fix duotone support flag relocation
aaronrobertshaw Mar 30, 2023
b401c12
Add filter.duotone to block-supports docs
aaronrobertshaw Mar 30, 2023
59b135e
Add filter.duotone to block.json schema
aaronrobertshaw Mar 30, 2023
6367140
Be more precise in filterId selector comment
Mar 30, 2023
f1506d0
Fix filters in editor applying to cover block text
Mar 30, 2023
502f8b4
Remove comment I accidentally committed
Mar 30, 2023
571681e
Remove __experimentalDuotone from color supports docs
Mar 30, 2023
5f16e9c
Add example to the filter.duotone docs
Mar 30, 2023
bd02cb2
Move migration into duotone class
Mar 30, 2023
b20c08e
Add deprecation note to experimentalDuotone docs
Mar 30, 2023
a7337a7
Fix PHP lint
Mar 30, 2023
7e7bf1c
Fix PHP lint
Mar 30, 2023
3346b9a
Correct experimental BC comments and tweak flow
aaronrobertshaw Mar 31, 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
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 );
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?

$selectors = explode( ',', $duotone_selector );

$selectors_scoped = array();
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
3 changes: 1 addition & 2 deletions lib/class-wp-theme-json-gutenberg.php
Original file line number Diff line number Diff line change
Expand Up @@ -2389,8 +2389,7 @@ function( $pseudo_selector ) use ( $selector ) {

// 3. Generate and append the rules that use the duotone selector.
if ( isset( $block_metadata['duotone'] ) && ! empty( $declarations_duotone ) ) {
$selector_duotone = static::scope_selector( $block_metadata['selector'], $block_metadata['duotone'] );
$block_rules .= static::to_ruleset( $selector_duotone, $declarations_duotone );
$block_rules .= static::to_ruleset( $block_metadata['duotone'], $declarations_duotone );
aaronrobertshaw marked this conversation as resolved.
Show resolved Hide resolved
}

// 4. Generate Layout block gap styles.
Expand Down
64 changes: 25 additions & 39 deletions lib/compat/wordpress-6.3/get-global-styles-and-settings.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,21 +25,7 @@ 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
// feature selectors later on.
// Root Selector ( can be used as a fallback ).
$root_selector = null;

if ( $has_selectors && isset( $block_type->selectors['root'] ) ) {
Expand All @@ -59,16 +45,34 @@ function wp_get_block_css_selector( $block_type, $target = 'root', $fallback = f
return $root_selector;
}

// If target is not `root` or `duotone` we have a feature or subfeature
// as the target. If the target is a string convert to an array.
$fallback_selector = $fallback ? $root_selector : null;

// If target is not `root` we have a feature or subfeature as the target.
// If the target is a string convert to an array.
if ( is_string( $target ) ) {
$target = explode( '.', $target );
}

// Feature Selectors ( May fallback to root selector ).
if ( 1 === count( $target ) ) {
$fallback_selector = $fallback ? $root_selector : null;
// Backwards compatibility for supports.__experimentalDuotone selectors.
if ( array( 'filter', 'duotone' ) === $target && null === _wp_array_get( $block_type->selectors, $target ) ) {
$duotone_selector = _wp_array_get( $block_type->supports, array( 'color', '__experimentalDuotone' ) );

// String color.__experimentalDuotone values need to be scoped to the root selector.
if ( is_string( $duotone_selector ) ) {
return WP_Theme_JSON_Gutenberg::scope_selector( $root_selector, $duotone_selector );
}

// When color.__experimentalDuotone is true, the root selector should be used.
if ( true === $duotone_selector ) {
return $root_selector;
}

// Experimental duotone support is not enabled.
return null;
}

// Feature Selectors ( may fallback to root selector ).
if ( 1 === count( $target ) ) {
// Prefer the selectors API if available.
if ( $has_selectors ) {
// Look for selector under `feature.root`.
Expand All @@ -95,25 +99,7 @@ function wp_get_block_css_selector( $block_type, $target = 'root', $fallback = f
}

// Scope the feature selector by the block's root selector.
$scopes = explode( ',', $root_selector );
$selectors = explode( ',', $feature_selector );

$selectors_scoped = array();
foreach ( $scopes as $outer ) {
foreach ( $selectors as $inner ) {
$outer = trim( $outer );
$inner = trim( $inner );
if ( ! empty( $outer ) && ! empty( $inner ) ) {
$selectors_scoped[] = $outer . ' ' . $inner;
} elseif ( empty( $outer ) ) {
$selectors_scoped[] = $inner;
} elseif ( empty( $inner ) ) {
$selectors_scoped[] = $outer;
}
}
}

return implode( ', ', $selectors_scoped );
return WP_Theme_JSON_Gutenberg::scope_selector( $root_selector, $feature_selector );
}

// Subfeature selector
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,21 +34,7 @@ export function getBlockCSSSelector(
const hasSelectors = ! isEmpty( selectors );
const path = Array.isArray( target ) ? target.join( '.' ) : target;

// Duotone ( no fallback selectors for Duotone ).
if ( path === 'filter.duotone' ) {
// If selectors API in use, only use its value or null.
if ( hasSelectors ) {
return get( selectors, path, null );
}

// Selectors API, not available, check for old experimental selector.
return get( supports, 'color.__experimentalDuotone', null );
}

// Root selector.

// Calculated before returning as it can be used as a fallback for feature
// selectors later on.
// Root selector ( can be used as fallback ).
let rootSelector = null;

if ( hasSelectors && selectors.root ) {
Expand All @@ -68,14 +54,32 @@ export function getBlockCSSSelector(
return rootSelector;
}

const fallbackSelector = fallback ? rootSelector : null;

// Backwards compatibility for supports.__experimentalDuotone selectors.
if ( path === 'filter.duotone' && ! get( selectors, path ) ) {
const duotoneSelector = get( supports, 'color.__experimentalDuotone' );

// String color.__experimentalDuotone values need to be scoped to the root selector.
if ( typeof duotoneSelector === 'string' ) {
return scopeSelector( rootSelector, duotoneSelector );
}

// When color.__experimentalDuotone is true, the root selector should be used.
if ( duotoneSelector === true ) {
return rootSelector;
}

// Experimental duotone support is not enabled.
return null;
}

// If target is not `root` or `duotone` we have a feature or subfeature
// as the target. If the target is a string convert to an array.
const pathArray = Array.isArray( target ) ? target : target.split( '.' );

// Feature selectors ( may fallback to root selector );
if ( pathArray.length === 1 ) {
const fallbackSelector = fallback ? rootSelector : null;

// Prefer the selectors API if available.
if ( hasSelectors ) {
// Get selector from either `feature.root` or shorthand path.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { getCSSRules } from '@wordpress/style-engine';
/**
* Internal dependencies
*/
import { PRESET_METADATA, ROOT_BLOCK_SELECTOR, scopeSelector } from './utils';
import { PRESET_METADATA, ROOT_BLOCK_SELECTOR } from './utils';
import { getBlockCSSSelector } from './get-block-css-selector';
import { getTypographyFontSizeValue } from './typography-utils';
import { GlobalStylesContext } from './context';
Expand Down Expand Up @@ -859,10 +859,9 @@ export const toStyles = (
if ( duotoneDeclarations.length > 0 ) {
ruleset =
ruleset +
`${ scopeSelector(
selector,
duotoneSelector
) }{${ duotoneDeclarations.join( ';' ) };}`;
`${ duotoneSelector }{${ duotoneDeclarations.join(
';'
) };}`;
Comment on lines +863 to +865
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.

Scoping the selector here was moved below in getBlockSelectors where it was easier.

}
}

Expand Down
66 changes: 28 additions & 38 deletions packages/block-editor/src/hooks/duotone.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import {
__unstableDuotoneStylesheet as DuotoneStylesheet,
__unstableDuotoneUnsetStylesheet as DuotoneUnsetStylesheet,
} from '../components/duotone';
import { getBlockCSSSelector } from '../components/global-styles/get-block-css-selector';
import { store as blockEditorStore } from '../store';

const EMPTY_ARRAY = [];
Expand Down Expand Up @@ -222,37 +223,6 @@ const withDuotoneControls = createHigherOrderComponent(
'withDuotoneControls'
);

/**
* Function that scopes a selector with another one. This works a bit like
* SCSS nesting except the `&` operator isn't supported.
*
* @example
* ```js
* const scope = '.a, .b .c';
* const selector = '> .x, .y';
* const merged = scopeSelector( scope, selector );
* // merged is '.a > .x, .a .y, .b .c > .x, .b .c .y'
* ```
*
* @param {string} scope Selector to scope to.
* @param {string} selector Original selector.
*
* @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.

const scopes = scope.split( ',' );
const selectors = selector.split( ',' );

const selectorsScoped = [];
scopes.forEach( ( outer ) => {
selectors.forEach( ( inner ) => {
selectorsScoped.push( `${ outer.trim() } ${ inner.trim() }` );
} );
} );

return selectorsScoped.join( ', ' );
}

function BlockDuotoneStyles( { name, duotoneStyle, id } ) {
const duotonePalette = useMultiOriginPresets( {
presetSetting: 'color.duotone',
Expand All @@ -273,17 +243,37 @@ function BlockDuotoneStyles( { name, duotoneStyle, id } ) {
colors = getColorsFromDuotonePreset( colors, duotonePalette );
}

const duotoneSupportSelectors =
getBlockType( name ).selectors?.filter?.duotone ||
getBlockSupport( name, 'color.__experimentalDuotone' );
const duotoneSupportSelectors = getBlockCSSSelector(
getBlockType( name ),
'filter.duotone'
);

// Extra .editor-styles-wrapper specificity is needed in the editor
// since we're not using inline styles to apply the filter. We need to
// override duotone applied by global styles and theme.json.
const selectorsGroup = scopeSelector(
`.editor-styles-wrapper .${ id }`,
duotoneSupportSelectors
);

// This is a JavaScript implementation of the PHP function in lib/class-wp-duotone-gutenberg.php
const scopes = ( '.' + id ).split( ',' );
const selectors = duotoneSupportSelectors.split( ',' );

const selectorsScoped = [];
scopes.forEach( ( outer ) => {
selectors.forEach( ( inner ) => {
outer = outer.trim();
inner = inner.trim();
if ( outer && inner ) {
// unlike scopeSelector this concatenates the selectors without a space.
selectorsScoped.push(
'.editor-styles-wrapper ' + outer + '' + inner
);
} else if ( outer ) {
selectorsScoped.push( '.editor-styles-wrapper ' + inner );
} else if ( inner ) {
selectorsScoped.push( '.editor-styles-wrapper ' + outer );
}
} );
} );
const selectorsGroup = selectorsScoped.join( ',' );

return createPortal(
<InlineDuotone
Expand Down
2 changes: 1 addition & 1 deletion packages/block-library/src/image/block.json
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,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
24 changes: 23 additions & 1 deletion phpunit/class-wp-get-block-css-selectors-test.php
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ public function test_get_duotone_selector_via_experimental_property() {
);

$selector = wp_get_block_css_selector( $block_type, 'filter.duotone' );
$this->assertEquals( '.experimental-duotone', $selector );
$this->assertEquals( '.wp-block-test-experimental-duotone-selector .experimental-duotone', $selector );
}

public function test_no_duotone_selector_set() {
Expand All @@ -115,6 +115,28 @@ public function test_no_duotone_selector_set() {
$this->assertEquals( null, $selector );
}

public function test_fallback_duotone_selector() {
$block_type = self::register_test_block(
'test/fallback-duotone-selector',
array( 'root' => '.fallback-root-selector' ),
null
);

$selector = wp_get_block_css_selector( $block_type, 'filter.duotone', true );
$this->assertEquals( '.fallback-root-selector', $selector );
}

public function test_fallback_duotone_selector_to_generated_class() {
$block_type = self::register_test_block(
'test/fallback-duotone-selector',
array(),
null
);

$selector = wp_get_block_css_selector( $block_type, 'filter.duotone', true );
$this->assertEquals( '.wp-block-test-fallback-duotone-selector', $selector );
}

public function test_get_feature_selector_via_selectors_api() {
$block_type = self::register_test_block(
'test/feature-selector',
Expand Down