Repository navigation
Global Styles: Resolve per-level heading element styles in block inspector controls #80495
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
Changes from 4 commits
83606c8
ad93ca6
8451e7e
fb65203
f1a60ff
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 |
|---|---|---|
|
|
@@ -12,6 +12,7 @@ import { useSelect } from '@wordpress/data'; | |
| * Internal dependencies | ||
| */ | ||
| import { useResolvedStyle } from '../inherited-value-context'; | ||
| import { BlockContextProvider } from '../../block-context'; | ||
|
|
||
| import { globalStylesDataKey } from '../../../store/private-keys'; | ||
|
|
||
|
|
@@ -46,9 +47,10 @@ jest.mock( '../../../store', () => ( { | |
| } ) ); | ||
|
|
||
| // `inherited-value-context.js` imports `store as blocksStore` from | ||
| // `@wordpress/blocks` for `useOwnVariation`. The blocks store's transitive | ||
| // import chain fails under this file's `@wordpress/data` mock (missing | ||
| // `createSelector`), so stub the blocks module with just the shape needed. | ||
| // `@wordpress/blocks` for `useVariationAndElements`. The blocks store's | ||
| // transitive import chain fails under this file's `@wordpress/data` mock | ||
| // (missing `createSelector`), so stub the blocks module with just the shape | ||
| // needed. | ||
| jest.mock( '@wordpress/blocks', () => ( { | ||
| store: { name: 'core/blocks' }, | ||
| getBlockType: ( blockName ) => | ||
|
|
@@ -83,7 +85,10 @@ describe( 'useResolvedStyle hook', () => { | |
| // The hook issues two `useSelect` reads: the merged Global Styles | ||
| // payload (block-editor store) and the block's registered styles | ||
| // (blocks store). Feed both from one stub. | ||
| function mockStores( rawGlobalStyles, blockStyles = [] ) { | ||
| function mockStores( | ||
| rawGlobalStyles, | ||
| { blockStyles = [], attributes = {} } = {} | ||
|
Member
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. Can be later, but you do you think a test with
Contributor
Author
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. We've already got a |
||
| ) { | ||
| useSelect.mockImplementation( ( mapSelect ) => | ||
| mapSelect( () => ( { | ||
| // `globalStylesDataKey` holds the BARE merged styles tree | ||
|
|
@@ -96,10 +101,84 @@ describe( 'useResolvedStyle hook', () => { | |
| [ globalStylesDataKey ]: rawGlobalStyles, | ||
| } ), | ||
| getBlockStyles: () => blockStyles, | ||
| // The hook reads the block's `level` attribute to pick the | ||
| // heading element layers. No block edit context is provided | ||
| // here, so `clientId` is undefined; the stub returns whatever | ||
| // attributes the test supplies regardless. | ||
| getBlockAttributes: () => attributes, | ||
| } ) ) | ||
| ); | ||
| } | ||
|
|
||
| // Resolves a block's styles as rendered inside an Accordion, which supplies | ||
| // the heading level to every descendant through block context. | ||
| function resolveInAccordion( blockName, level = 3 ) { | ||
| render( | ||
| <BlockContextProvider | ||
| value={ { 'core/accordion-heading-level': level } } | ||
| > | ||
| <Probe blockName={ blockName } /> | ||
| </BlockContextProvider> | ||
| ); | ||
| return JSON.parse( screen.getByTestId( 'probe' ).textContent ); | ||
| } | ||
|
|
||
| // Accordion Heading resolves its level from the parent Accordion's block | ||
| // context, not from its own `level` attribute, and has no level-0 state. | ||
| test( 'accordion-heading takes its level from block context', () => { | ||
| mockStores( { | ||
| elements: { | ||
| heading: { color: { text: '#111111' } }, | ||
| h3: { typography: { fontSize: '19px' } }, | ||
| }, | ||
| } ); | ||
| const parsed = resolveInAccordion( 'core/accordion-heading' ); | ||
| expect( parsed.value.typography.fontSize ).toBe( '19px' ); | ||
| expect( parsed.value.color.text ).toBe( '#111111' ); | ||
| } ); | ||
|
|
||
| test( 'accordion-heading with no context still resolves to h3', () => { | ||
| mockStores( { | ||
| elements: { h3: { typography: { fontSize: '19px' } } }, | ||
| } ); | ||
| render( <Probe blockName="core/accordion-heading" /> ); | ||
| const parsed = JSON.parse( screen.getByTestId( 'probe' ).textContent ); | ||
| expect( parsed.value.typography.fontSize ).toBe( '19px' ); | ||
| } ); | ||
|
|
||
| test( "accordion-heading's own level attribute wins over block context", () => { | ||
| mockStores( | ||
| { | ||
| elements: { | ||
| h3: { typography: { fontSize: '19px' } }, | ||
| h5: { typography: { fontSize: '13px' } }, | ||
| }, | ||
| }, | ||
| { attributes: { level: 5 } } | ||
| ); | ||
| const parsed = resolveInAccordion( 'core/accordion-heading' ); | ||
| // The attribute is what the front end serializes (`save.js` renders | ||
| // `h${ level || 3 }`), so it takes precedence when the two disagree. | ||
| expect( parsed.value.typography.fontSize ).toBe( '13px' ); | ||
| } ); | ||
|
|
||
| // An ordinary Heading can be placed inside an Accordion Panel, which puts | ||
| // `core/accordion-heading-level` in its block context. Only the blocks that | ||
| // opt in may read that key. | ||
| test( 'a plain heading ignores an accordion level in block context', () => { | ||
| mockStores( | ||
| { | ||
| elements: { | ||
| h2: { typography: { fontSize: '24px' } }, | ||
| h3: { typography: { fontSize: '19px' } }, | ||
| }, | ||
| }, | ||
| { attributes: { level: 2 } } | ||
| ); | ||
| const parsed = resolveInAccordion( 'core/heading' ); | ||
| expect( parsed.value.typography.fontSize ).toBe( '24px' ); | ||
| } ); | ||
|
|
||
| test( 'returns empty value and sources when no block name is given', () => { | ||
| mockStores( { typography: { fontSize: '16px' } } ); | ||
| render( <Probe /> ); | ||
|
|
@@ -226,6 +305,7 @@ describe( 'useResolvedStyle – production bare-tree shape', () => { | |
| [ globalStylesDataKey ]: rawGlobalStyles, | ||
| } ), | ||
| getBlockStyles: () => [], | ||
| getBlockAttributes: () => ( {} ), | ||
| } ) ) | ||
| ); | ||
| return render( ui ); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,7 +4,7 @@ | |
|
|
||
| ### New Features | ||
|
|
||
| - Added a private `resolveStyle` API that resolves the merged Global Styles cascade (root, block type, and applied block style variation) for a block, returning the inherited value and a per-leaf source map. | ||
| - Added a private `resolveStyle` API that resolves the merged Global Styles cascade (root, root-level element layers, block type, and applied block style variation) for a block, returning the inherited value and a per-leaf source map. Element layers are supplied by the caller as an ordered `elements` list on the resolve context (low to high precedence, e.g. `[ 'heading', 'h2' ]`), keeping the engine agnostic of block-to-element mappings. | ||
|
Member
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. Needs a #80495 link I think
Contributor
Author
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. Added the PR to a separate changelog entry. I left the existing entries alone just in case it affects branch conflicts when we go to backport this (but I might be overthinking this one 😄) |
||
| - Added a private `getVariationStyle` API that returns a block style variation's styles, resolving `{ ref }` references against the Global Styles tree by default (pass `{ resolveRefs: false }` to keep them in place). | ||
|
|
||
| ## 1.18.0 (2026-07-14) | ||
|
|
||
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 think
core/accordion-headingis an h3. Should we add it?https://github.com/WordPress/gutenberg/blob/fix/global-styles-element-layers/packages/block-library/src/accordion/block.json#L77
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 think the accordion one might need a little more work, but I'll double-check tomorrow. Thanks for taking this for a spin!
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 got it working for the accordion heading: it needed different handling as it derives its heading level via context rather than it being set on the individual accordion heading (so that all accordion headings in the set of accordion items receive the same heading level). It's a bit hard-coded and possibly smelly, but I tried to keep it neat where I could. Let me know if you think it's too much, and we can roll back and look at it in a follow-up if need be.
I'm a little on the fence about all this hard-coding, but at least it's fairly internal for now, and we could always look at improving it in the future... in any case, here's how it's looking now, inheriting the heading element style:
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.
Nice! I think it's okay. Is there an alternative in code?
My 2c is that this small fix makes headings consistent, and therefore better for the user. That to me is more important.
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.
Same here. I think I was just musing out loud about the idea that at some point we might codify this in block.json in some way and make it a bit more API-ey than hard-coded.
But agreed — for now I think focusing on improving the UX while keeping the code fairly self-contained is likely the right path forward.
I haven't looked at an alternative in code, mostly I wanted to confidence check that we're okay with the hard-coding. It seems we both are for now 😄