Accordion: Add BlockGap support to content & panel#71461
Accordion: Add BlockGap support to content & panel#71461t-hamano merged 10 commits intoWordPress:trunkfrom
Conversation
|
Thanks for working on this PR. In #71454, the extra wrapper div has been removed from the Accordion Panel, so can you rebase this PR on top of trunk and confirm the blockGap support works? |
t-hamano
added
[Type] Bug
|
@t-hamano thanks for notifying about the change 🙌 I've merged the updates & the blockGap support works as expected Screen.Recording.2025-09-03.at.1.40.10.PM.mov |
yashjawale
requested review from
ajitbohra,
fabiankaegy,
juanmaguitar and
ndiego
as code owners
|
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. |
| style: { | ||
| ...blockProps.style, | ||
| ...spacingProps.style, | ||
| }, |
There was a problem hiding this comment.
This approach is only necessary if we want to skip serialization of block support or apply styles to elements within a block.
We might take this opportunity to simplify it to:
const blockProps = useBlockProps( {
className: clsx( {
'is-open': openByDefault || isSelected,
} ),
} );
const innerBlocksProps = useInnerBlocksProps( blockProps, {
template: [
[ 'core/accordion-header', {} ],
[
'core/accordion-panel',
{
openByDefault,
},
],
],
templateLock: 'all',
directInsert: true,
templateInsertUpdatesSelection: true,
} );| } | ||
|
|
||
| .accordion-content__heading { | ||
| :where(.accordion-content__heading) { |
There was a problem hiding this comment.
Why is this style necessary in the first place?
There was a problem hiding this comment.
This was already present before the change...
I'll check if removing that style causes any visible effect
If that doesn't change anything then the workarounds below won't be needed
|
|
||
| .is-layout-flow > .wp-block-accordion-panel, | ||
| .wp-block-accordion-panel { | ||
| :where(.is-layout-flow > .wp-block-accordion-panel, .wp-block-accordion-panel) { |
There was a problem hiding this comment.
What is the reason for this change?
There was a problem hiding this comment.
For accordion content block, this rule was preventing the margin from being applied for blockGap to work
This change lowers the specificity so that margin applied by blockGap could work
There was a problem hiding this comment.
How about just removing margin: 0 here?
.wp-block-accordion-panel is just a div element, so I'm not sure why margin:0 is necessary in the first place.
There was a problem hiding this comment.
Yes I'll try finding why was that added
ig removing margin: 0 shouldn't cause any problems & then the workarounds won't be needed too
| // Override block spacing for closed accordion panels specifically. | ||
| // Higher specificity to ensure it overrides block spacing margins. | ||
| .wp-block-accordion-content:not(.is-open) > * + .wp-block-accordion-panel { | ||
| margin-block-start: 0 !important; | ||
| } |
There was a problem hiding this comment.
As far as I've tested, I don't understand what this style is trying to fix - what specific problem does it cause?
There was a problem hiding this comment.
When we set blockGap on Accordion content, the margin still remains when the accordion is collapsed
This rule overrides it to zero so that additional margin isn't applied for collapsed accordions (user would use block gap of parent accordion block to set that spacing), so the content block's gap would just affect the spacing between header & panel
There was a problem hiding this comment.
I see, then, how about a selector like this? I'm not sure why aria-hidden is used in the editor and inert on the front end, but maybe we can unify this in a follow-up.
.wp-block-accordion-panel[inert],
.wp-block-accordion-panel[aria-hidden="true"] {
}
|
Updated ✅ |
t-hamano
left a comment
t-hamano
left a comment
There was a problem hiding this comment.
LGTM! Everything works as expected 👍
2 participants
What?
Closes #71433
This PR fixes issue of block spacing setting not working for accordion content
Why?
Currently, changing the value of Block spacing from Inspector doesn't have any effect. Ideally it should add block spacing styles to accordion content
How?
This PR adds
layoutfield toblock.jsonfor Accordion content & panel so that block spacing setting worksFor
accordion-contentblock, addtional handling via use ofuseSpacingPropswas needed for blockGap changes to reflectStyling specificity is modified so that the margin applied by blockSpacing settings of AccordionContent block don't affect the spacing when accordions are closed (this one is handled by spacing set by main parent Accordion block) & instead is only utilized for separation between Accordion header & panel.
Testing Instructions
Screenshots or screencast
Before
Screen.Recording.2025-09-03.at.12.48.38.PM.mov
After
Screen.Recording.2025-09-03.at.1.32.44.PM.mov