Stabilize Accordion block#72036
Merged
Merged
Conversation
t-hamano
added
[Type] Enhancement
t-hamano
force-pushed
the
stabilize-accordion
branch
from
9c45b6e to
474561c
Compare
t-hamano
requested review from
ajitbohra,
fabiankaegy and
juanmaguitar
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. |
github-project-automation
Bot
moved this to 🔎 Needs Review
in WordPress 6.9 Editor Tasks
t-hamano
requested review from
a team and
jorgefilipecosta
and removed request for
a team
9 tasks
|
Size Change: -17 B (0%) Total Size: 1.95 MB
ℹ️ View Unchanged
|
t-hamano
force-pushed
the
stabilize-accordion
branch
from
474561c to
d933127
Compare
t-hamano
commented
Oct 3, 2025
| await expect( | ||
| page.getByText( | ||
| '2 results found, use up and down arrow keys to navigate.' | ||
| '3 results found, use up and down arrow keys to navigate.' |
Contributor
Author
There was a problem hiding this comment.
This E2E test tests the number that appears when you enter /heading. This number changes because the Accordion block contains the "headings" keyword in its description.
t-hamano
commented
Oct 3, 2025
Comment on lines
+39
to
+41
| await settingsPanel | ||
| .getByRole( 'button', { name: 'Heading', exact: true } ) | ||
| .click(); |
Contributor
Author
There was a problem hiding this comment.
To avoid the Accordion "Heading" block
|
Flaky tests detected in db0d187. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/18221968617
|
Contributor
|
This is very cool!! Quick drive-by comment: we probably want to add |
Member
|
Thank you for your great work on it @t-hamano 🙌 |
ntsekouras
reviewed
Oct 3, 2025
ntsekouras
reviewed
Oct 3, 2025
ntsekouras
approved these changes
Oct 3, 2025
Contributor
ntsekouras
left a comment
ntsekouras
left a comment
There was a problem hiding this comment.
Thanks @t-hamano! Besides the small comment about the fixtures this looks good!
I believe it's fine to add contentRole in follow up to properly focus on the behavior we want.
Also noting that here we've caused block invalidation in the editor, but I guess it's fine since the block was experimental and the front end seems to work fine.
|
Thanks @t-hamano for working on this, this is coming together so well! I tested this PR in Playground. Here are my findings:
to something like
I wouldn't mention the plus, because when open, it's a cross/X. |
Contributor
Author
|
Thank you everyone for your feedback! I'll collect all the feedback suggested here and submit it as a new issue later. All tasks should be addressed in the 6.9 release. To @ntsekouras:
Yes, this is expected. That's why I didn't stabilize this block until the very end. Once stabilized, we need to maintain backward compatibility. To @hanneslsm:
This is expected behavior, as the current toggle icon is not editable. In the future, we may consider customizing the icon.
I think we should remove the content width support from the Accordion Item block. The parent Accordion block supports content width, and users will typically want to change the width of all accordion items at once.
This is likely due to the following style being applied: h1.has-background, h2.has-background, h3.has-background, h4.has-background, h5.has-background, h6.has-background {
padding: 1.25em 2.375em;
}In my opinion, this padding should not be applied to the Accordion Heading block. |
Contributor
These sound good. Let's do it. |
Contributor
Done here #72045 |
Contributor
Done in Accordion Heading: Remove the padding from the heading when it has a background color |
Contributor
Author
|
@scruffian Thanks for the quick fix! The only remaining task may be to add |
github-project-automation
Bot
moved this from 🔎 Needs Review
to ✅ Done
in WordPress 6.9 Editor Tasks
Contributor
Author
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What?
Closes #71732
Stabilize Accordion Block for the 6.9 release.
Why?
We've identified and resolved the tasks that need to be resolved before stabilizing this block. There are a few open sub-issues, but we believe they're all minor bugs that will be resolved in the 6.9 release.
I hope you'll take a fresh look at this block and see if it's worth stabilizing. We look forward to your feedback!
cc @mikachan @hanneslsm @joedolson @WordPress/gutenberg-core
How?
Testing Instructions