-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Changes from 8 commits
7735980
0c52d64
02bfc17
e5ed227
e60c4b7
555f7d0
64a505a
3cd86f0
7d370c6
521db54
fc2f0c0
53410e5
42e1b3f
50997ca
b9d16ea
31aa06e
114ec2b
da60b58
6a750e3
2236ce5
972dca7
b401c12
59b135e
6367140
f1506d0
502f8b4
571681e
5f16e9c
bd02cb2
b20c08e
a7337a7
7e7bf1c
3346b9a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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'; | ||
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Scoping the selector here was moved below in |
||
} | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 = []; | ||
|
@@ -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 ) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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', | ||
|
@@ -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 | ||
|
There was a problem hiding this comment.
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?