CollapsibleCard: Prevent focus ring clipping by content overflow#77667
Merged
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. |
|
Size Change: +165 B (0%) Total Size: 7.82 MB 📦 View Changed
ℹ️ View Unchanged
|
|
Flaky tests detected in 0d5ff69. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/25104284582
|
Contributor
|
This obviously increases the gap between the header content and the card content. I suspect this is not ideal from a design POV. I'm assuming the problematic |
Member
Contributor
|
Given the outer @t-hamano do you mind trying this approach instead? |
ciampo
approved these changes
Apr 28, 2026
ciampo
left a comment
ciampo
left a comment
Contributor
There was a problem hiding this comment.
LGTM 🚀
Before merging, let's:
- update PR description
- add a CHANGELOG entry to
@wordpress/ui
Contributor
Author
|
@ciampo Thanks for the review!
Unfortunately, I realized that the animation doesn't work in Firefox when expanding content. This occurs on both Windows and MacOS. I suspect this is related to how firefox-animation.mp4Another idea is to apply overflow: visible once the expansion finishes, and remove it when collapsing starts, though it might feel a bit hacky 🤔 |
Contributor
|
Yeah, I can confirm. Doing some extra digging, it looks like Firefox is actually the only browser following the specs exactly
Toggling |
t-hamano
requested review from
ajitbohra,
ellatrix,
fabiankaegy,
gigitux,
kevin940726,
nerrad,
ntsekouras,
ntwb,
oandregal and
spacedmonkey
as code owners
github-actions
Bot
added
[Package] Components
github-actions
Bot
removed
[Package] Block editor
Contributor
Author
|
I'm a little unsure about the code, but it now works correctly in Firefox. Recording.2026-04-29.133520.mp4 |
ciampo
force-pushed
the
collapsible-card-clipping
branch
2 times, most recently
from
764a3ff to
a4de47f
Compare
Contributor
Yeah, I don't like that we're using a base ui internal. I took the initiative and landed on a solution that looks cleaner, and seems to work well on Firefox/Safari/Chrome on MacOS. It uses base ui's state exposed via a callback version of Prior to that, I tried a more verbose solution (based on @t-hamano let me know if the current solution (or the previous one) work for you! |
ciampo
removed request for
ajitbohra,
ellatrix,
fabiankaegy,
gigitux,
kevin940726,
nerrad,
ntsekouras,
ntwb,
oandregal and
spacedmonkey
…s show through Removing `overflow: hidden` outright would let descendant focus rings extend into the header/content gap, but it also gives up the clip the panel relies on while animating. Toggle a class instead: clip while collapsed or transitioning; drop the clip once the open transition has settled. The class is driven by a `transitionend` listener on the panel and a small `MutationObserver` on `data-open`, with a fallback for `prefers-reduced-motion` where no transition fires (`getAnimations().length === 0`). This makes the per-route padding workaround in the Experiments screen unnecessary, so it is removed.
Replace the local `transitionend` + `MutationObserver` machinery with the callback form of Base UI's `Collapsible.Panel.className`. The callback receives the panel's state, and `state.open && state.transitionStatus === 'idle'` is the public way to read "open and settled" without subscribing to DOM events. Rename the resulting class to `overflowVisible` to match what it actually does. The callback form is part of Base UI's documented Panel API, but `@wordpress/ui`'s `ComponentProps` re-types `className` as `string` only, so the function form needs a one-line `@ts-expect-error`.
ciampo
force-pushed
the
collapsible-card-clipping
branch
from
a4de47f to
0d5ff69
Compare
Contributor
Author
|
@ciampo Thank you! It works well in my testing 👍 |
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
4 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?
Prevent the focus ring of focusable descendants of
CollapsibleCard.Contentfrom being clipped by the panel's overflow once the card is fully expanded. The first child is the worst case — its ring extends into the gap above the content area and was being cut off.Why?
The panel needs
overflow: hiddenwhile collapsed and during the open/close height animation, otherwise its content escapes the card during the transition. But once the panel has settled open, that clip cuts off the focus ring of any focusable descendant whose ring extends past the panel's edge. Consumers were working around this in their own stylesheets (e.g. the Experiments screen).How?
Toggle the clip on a class instead of always applying it:
overflow: hiddenwhile collapsed or transitioning.overflow: visible(overflowVisibleclass) once the panel is fully expanded.The class is gated on Base UI's panel state via the
classNamecallback —state.open && state.transitionStatus === 'idle'— which is the public way to read "open and settled" without subscribing to DOM events. The callback form is documented Base UI API;@wordpress/ui'sComponentPropsre-typesclassNameasstringonly, so the function form needs a one-line@ts-expect-errorat the call site.No public API change. The per-route padding workaround in the Experiments screen is no longer needed and is removed.
Testing Instructions
WithFocusableContentstory underDesign System / Components / CollapsibleCardin Storybook.prefers-reduced-motionenabled (DevTools → Rendering → Emulate CSS media featureprefers-reduced-motion: reduce). The first child's focus ring should still be fully visible when the card is open.Testing Instructions for Keyboard
Same as above — drive the toggle and field interactions purely with
Tab,Shift+Tab,Space, andEnter.Screenshots or screencast
Use of AI Tools
The original approach was explored with Claude. Subsequent rounds (including this revision and PR description) were drafted with Claude in Cursor and reviewed by the author.