CARVIEW |
Navigation Menu
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Update: Accordion heading level synchronization. #71895
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
Update: Accordion heading level synchronization. #71895
Conversation
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
ffc9308
to
bc1d388
Compare
This PR works as one approach, but I'm wondering if there's a way to sync headings other than using effects. Another approach I'm thinking of is the following, does that make sense? In other words, the markup saved is just temporary, and the heading levels are actually set server-side: export default function save( { attributes } ) {
const { title } = attributes;
const blockProps = useBlockProps.save();
return (
// The heading level is set in the Accordion block, but the save function cannot reference the context.
// Therefore, the heading level is hard-coded here. The actual heading level will be changed server-side.
<h3 { ...blockProps }>
<button className="wp-block-accordion-header__toggle">
<RichText.Content value={ title } />
</button>
</h3>
);
} Can anyone think of any other approaches? |
bc1d388
to
2d03be6
Compare
I would like to ask contributors who are familiar with block development for help if they have any ideas. @aaronrobertshaw @andrewserong @ntsekouras @scruffian @ramonjd We are trying to synchronize the heading levels of the Accordion Header block within the Accordion block:
Even if we add a level control to the Accordion block and provide it as context, we cannot reference the context in the save function. Therefore, the only way we can think of to synchronize the levels frequently is via useEffect. Do you have any ideas? |
In the Accordian block, you could use Since the Accordian header already has a Here's a working example: Diffdiff --git a/packages/block-library/src/accordion-header/block.json b/packages/block-library/src/accordion-header/block.json
index 7181ef80b3..4d3baec8b7 100644
--- a/packages/block-library/src/accordion-header/block.json
+++ b/packages/block-library/src/accordion-header/block.json
@@ -9,8 +9,7 @@
"parent": [ "core/accordion-content" ],
"usesContext": [
"core/accordion-icon-position",
- "core/accordion-show-icon",
- "core/accordion-heading-level"
+ "core/accordion-show-icon"
],
"supports": {
"anchor": true,
diff --git a/packages/block-library/src/accordion-header/edit.js b/packages/block-library/src/accordion-header/edit.js
index 545af878aa..b6cf869266 100644
--- a/packages/block-library/src/accordion-header/edit.js
+++ b/packages/block-library/src/accordion-header/edit.js
@@ -10,13 +10,12 @@ import {
} from '@wordpress/block-editor';
export default function Edit( { attributes, setAttributes, context } ) {
- const { title } = attributes;
+ const { title, level } = attributes;
const {
'core/accordion-icon-position': iconPosition,
'core/accordion-show-icon': showIcon,
- 'core/accordion-heading-level': headingLevel,
} = context;
- const TagName = 'h' + headingLevel;
+ const TagName = 'h' + ( level || 3 );
// Set icon attributes.
useEffect( () => {
@@ -28,13 +27,6 @@ export default function Edit( { attributes, setAttributes, context } ) {
}
}, [ iconPosition, showIcon, setAttributes ] );
- // Sync heading level from context.
- useEffect( () => {
- if ( headingLevel !== undefined && headingLevel !== attributes.level ) {
- setAttributes( { level: headingLevel } );
- }
- }, [ headingLevel, attributes.level, setAttributes ] );
-
const blockProps = useBlockProps();
const spacingProps = useSpacingProps( attributes );
diff --git a/packages/block-library/src/accordion/block.json b/packages/block-library/src/accordion/block.json
index 37136a9742..89294106cd 100644
--- a/packages/block-library/src/accordion/block.json
+++ b/packages/block-library/src/accordion/block.json
@@ -78,8 +78,7 @@
},
"providesContext": {
"core/accordion-icon-position": "iconPosition",
- "core/accordion-show-icon": "showIcon",
- "core/accordion-heading-level": "headingLevel"
+ "core/accordion-show-icon": "showIcon"
},
"allowedBlocks": [ "core/accordion-content" ],
"textdomain": "default",
diff --git a/packages/block-library/src/accordion/edit.js b/packages/block-library/src/accordion/edit.js
index 38a74aeb77..d93a131244 100644
--- a/packages/block-library/src/accordion/edit.js
+++ b/packages/block-library/src/accordion/edit.js
@@ -19,7 +19,7 @@ import {
ToolbarButton,
ToolbarGroup,
} from '@wordpress/components';
-import { useDispatch } from '@wordpress/data';
+import { useDispatch, useSelect, useRegistry } from '@wordpress/data';
import { createBlock } from '@wordpress/blocks';
/**
@@ -45,6 +45,9 @@ export default function Edit( {
isSelected: isSingleSelected,
} ) {
const blockProps = useBlockProps();
+ const registry = useRegistry();
+ const { getBlockOrder } = useSelect( blockEditorStore );
+ const { updateBlockAttributes } = useDispatch( blockEditorStore );
const dropdownMenuProps = useToolsPanelDropdownMenuProps();
const { insertBlock } = useDispatch( blockEditorStore );
@@ -60,6 +63,30 @@ export default function Edit( {
insertBlock( newAccordionContent, undefined, clientId );
};
+ /**
+ * Update all child Accordion Header blocks with a new heading level
+ * based on the accordion group setting.
+ * @param {number} newHeadingLevel The new heading level to set
+ */
+ function updateHeadingLevel( newHeadingLevel ) {
+ const innerBlockClientIds = getBlockOrder( clientId );
+
+ // Get all accordion-header blocks from all accordion-content blocks
+ const accordionHeaderClientIds = [];
+ innerBlockClientIds.forEach( ( contentClientId ) => {
+ const headerClientIds = getBlockOrder( contentClientId );
+ accordionHeaderClientIds.push( ...headerClientIds );
+ } );
+
+ // Update own and child block heading levels
+ registry.batch( () => {
+ setAttributes( { headingLevel: newHeadingLevel } );
+ updateBlockAttributes( accordionHeaderClientIds, {
+ level: newHeadingLevel,
+ } );
+ } );
+ }
+
return (
<>
{ isSingleSelected && (
@@ -69,9 +96,7 @@ export default function Edit( {
<HeadingLevelDropdown
value={ headingLevel }
options={ levelOptions }
- onChange={ ( newLevel ) =>
- setAttributes( { headingLevel: newLevel } )
- }
+ onChange={ updateHeadingLevel }
/>
</ToolbarGroup>
</BlockControls>
Kapture.2025-10-01.at.12.11.08.mp4Does that help? |
@ramonjd Thanks for the feedback! The tricky part is that the heading levels have to be synchronized even when a new Accordion Content block is inserted π |
Sorry, I'm not quite sure what the requirements are then. What is the example use case? |
After applying your patch to this PR, please do the following:
b87cee75ece9d8df9461b2afa25b3f7e.mp4 |
Ah, thanks for explaining it π€¦π» I get it now π I guess you could use a combination of approaches:
But then you might have to ditch the default value Not sure, sorry! |
Wait, when adding a new Accordion Content block, can't we simply reference the context and insert a block with the appropriate heading? If so, it might be best to add this approach to @ramonjd's patch. |
Yeah, using context for new blocks plus Ramon's diff for updating existing blocks sounds like a good approach, as it means no |
Yeah, that's what I was getting at. ππ» One of my sentences was cut off in the above comment (now edited), I wanted to write:
|
Thanks for the feedback, everyone! @levinbaria, could you resolve the conflicts and implement the suggested approach? |
2d03be6
to
3381b21
Compare
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.
LGTM! Everything seems to be working fine π
Note: The failing Create Block Checks aren't related to this PR. That's the GitHub problem itself. See: https://wordpress.slack.com/archives/C02QB2JS7/p1759320105871839 Let's wait a bit longer and if it doesn't fix the issue, merge this PR. |
@levinbaria sorry, it looks like there's a conflict again π
could you run Update: never mind, I was able to resolve the conflict on GitHub. |
Thanks @t-hamano for taking care of that, appreciate it. |
Nice work, folks! |
What?
Closes: #71776
This PR changes the Accordion blocks so the heading level is controlled from the Accordion Group instead of from each individual Accordion Header.
Adds a headingLevel attribute to the Accordion Group.
Removes the heading-level control from individual Accordion Header toolbars in the editor UI.
Accordion Header rendering now uses the group headingLevel so all headers within the same group have the same semantic level in output.
This keeps the UI simpler (one place to choose the heading level) and fixes inconsistent heading levels and accessibility issues
Why?
Currently each Accordion Header exposes its own heading-level control in the toolbar. That causes two problems:
How?
Adds a
headingLevel
attribute to the Accordion Group and expose it in the block settings/inspector.Remove the per-header heading-level control from each Accordion Header in the editor UI.
Pass the group headingLevel to child header components so they render the correct both in the editor preview and on save.
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast