✂️ fix: Unclip the Share Dialog's Public Role Menu (#15177)

PR #14734 replaced PublicSharingToggle's hand-rolled reveal (which set
overflow: visible while open) with the shared Collapse, whose permanent
overflow-hidden shears the non-portaled access-roles menu to a sliver.
Adds an opt-in overflowVisibleWhenOpen prop to Collapse — clipped while
closed and during the closing tween, unclipped once open — so in-tree
popovers can escape; the menu stays non-portaled because portaled menus
inside modal OGDialogs land aria-hidden and get focus-yanked shut.
This commit is contained in:
Danny Avila 2026-08-24 20:59:07 -04:00 committed by GitHub
parent 69e7c73614
commit 6988ff5d7b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 64 additions and 3 deletions

View file

@ -85,7 +85,7 @@ export default function PublicSharingToggle({
</div>
</div>
<Collapse open={isPublic} className="pt-4">
<Collapse open={isPublic} overflowVisibleWhenOpen className="pt-4">
<div className="flex items-center justify-between bg-transparent">
<div className="flex items-center gap-3">
<div className="text-status-info">

View file

@ -26,6 +26,7 @@ describe('PublicSharingToggle', () => {
const collapse = permissionLabel.closest('[aria-hidden="true"]');
expect(collapse).toHaveClass('grid-rows-[0fr]');
expect(permissionLabel.closest('.overflow-hidden')).toBeInTheDocument();
rerender(
<PublicSharingToggle
@ -39,4 +40,18 @@ describe('PublicSharingToggle', () => {
expect(permissionLabel.closest('.bg-transparent')).not.toHaveClass('bg-surface-secondary/50');
expect(permissionLabel.closest('.grid')).toHaveClass('grid-rows-[1fr]');
});
it('does not clip the open permission row, so the inline role menu can escape the box', () => {
render(
<PublicSharingToggle
isPublic={true}
onPublicToggle={jest.fn()}
onPublicRoleChange={jest.fn()}
/>,
);
const permissionLabel = screen.getByText('com_ui_everyone_permission_level');
expect(permissionLabel.closest('.overflow-hidden')).toBeNull();
expect(permissionLabel.closest('.overflow-visible')).toBeInTheDocument();
});
});

View file

@ -5,6 +5,7 @@ interface CollapseProps {
open: boolean;
children: ReactNode;
className?: string;
overflowVisibleWhenOpen?: boolean;
}
/**
@ -14,8 +15,19 @@ interface CollapseProps {
* cross-fade smoothly without a measuring wrapper fighting nested reveals.
* Content fades to soften the swap; while closed it is `inert` (removed from tab
* order and the a11y tree) so collapsed form fields can't be focused or read.
*
* `overflowVisibleWhenOpen` lifts the clip while open so non-portaled popovers
* anchored inside (e.g. dropdown menus, which must stay in-tree within modal
* dialogs to remain inside the focus trap) aren't sheared at the box edge; the
* closed state and the closing tween still clip, and the opacity fade masks the
* un-clipped opening tween.
*/
export default function Collapse({ open, children, className }: CollapseProps) {
export default function Collapse({
open,
children,
className,
overflowVisibleWhenOpen = false,
}: CollapseProps) {
return (
<div
aria-hidden={!open || undefined}
@ -25,7 +37,12 @@ export default function Collapse({ open, children, className }: CollapseProps) {
open ? 'grid-rows-[1fr]' : 'grid-rows-[0fr]',
)}
>
<div className="min-h-0 overflow-hidden">
<div
className={cn(
'min-h-0',
open && overflowVisibleWhenOpen ? 'overflow-visible' : 'overflow-hidden',
)}
>
<div
className={cn(
'transition-opacity duration-200 ease-out motion-reduce:transition-none',

View file

@ -25,4 +25,33 @@ describe('Collapse', () => {
expect(root).toHaveClass('grid-rows-[0fr]');
expect(root).toHaveAttribute('aria-hidden', 'true');
});
test('clips content while open by default', () => {
const { container } = render(
<Collapse open={true}>
<span data-testid="child" />
</Collapse>,
);
const clipWrapper = (container.firstChild as HTMLElement).firstElementChild as HTMLElement;
expect(clipWrapper).toHaveClass('overflow-hidden');
});
test('lifts the clip while open for popover-hosting content, restoring it when closed', () => {
const { container, rerender } = render(
<Collapse open={true} overflowVisibleWhenOpen>
<span data-testid="child" />
</Collapse>,
);
const clipWrapper = (container.firstChild as HTMLElement).firstElementChild as HTMLElement;
expect(clipWrapper).toHaveClass('overflow-visible');
expect(clipWrapper).not.toHaveClass('overflow-hidden');
rerender(
<Collapse open={false} overflowVisibleWhenOpen>
<span data-testid="child" />
</Collapse>,
);
expect(clipWrapper).toHaveClass('overflow-hidden');
expect(clipWrapper).not.toHaveClass('overflow-visible');
});
});