Skip to content

Commit 4f7caab

Browse files
Merge branch 'main' into austin/connected-accounts-actions
2 parents 0c60764 + b9b0beb commit 4f7caab

13 files changed

Lines changed: 316 additions & 27 deletions

File tree

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
---
2+
---
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
---
2+
---

‎packages/headless/src/hooks/use-return-focus.test.ts‎

Lines changed: 65 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import type { FloatingEvents, OpenChangeReason } from '@floating-ui/react';
22
import { renderHook } from '@testing-library/react';
33
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
44

5+
import { withInteractionOrigin } from '../utils/interaction-origin';
56
import { useReturnFocus } from './use-return-focus';
67

78
function createEvents(): FloatingEvents & { close: (event?: Event, reason?: OpenChangeReason) => void } {
@@ -26,7 +27,7 @@ function createEvents(): FloatingEvents & { close: (event?: Event, reason?: Open
2627
};
2728
}
2829

29-
function renderReturnFocus(trigger: HTMLElement) {
30+
function renderReturnFocus(trigger: HTMLElement | null) {
3031
const events = createEvents();
3132

3233
const { result, rerender } = renderHook(
@@ -35,7 +36,14 @@ function renderReturnFocus(trigger: HTMLElement) {
3536
{ initialProps: { open: false } },
3637
);
3738

38-
return { events, result, open: (open: boolean) => rerender({ open }) };
39+
return {
40+
events,
41+
result,
42+
open: (open: boolean) => {
43+
events.emit('openchange', { open });
44+
rerender({ open });
45+
},
46+
};
3947
}
4048

4149
let trigger: HTMLElement;
@@ -105,4 +113,59 @@ describe('useReturnFocus', () => {
105113

106114
expect(result.current.current).toBe(trigger);
107115
});
116+
117+
describe('opened from inside another floating element', () => {
118+
let menuTrigger: HTMLElement;
119+
120+
beforeEach(() => {
121+
menuTrigger = document.createElement('button');
122+
document.body.append(menuTrigger);
123+
});
124+
125+
afterEach(() => menuTrigger.remove());
126+
127+
it('falls back to the interaction origin when it has no trigger of its own', () => {
128+
const { result, open } = renderReturnFocus(null);
129+
130+
withInteractionOrigin(menuTrigger, () => open(true));
131+
132+
expect(result.current.current).toBe(menuTrigger);
133+
});
134+
135+
it('prefers its own trigger over the interaction origin', () => {
136+
const { result, open } = renderReturnFocus(trigger);
137+
138+
withInteractionOrigin(menuTrigger, () => open(true));
139+
140+
expect(result.current.current).toBe(trigger);
141+
});
142+
143+
it('falls back to the origin once its own trigger has left the page', () => {
144+
const { result, open } = renderReturnFocus(trigger);
145+
withInteractionOrigin(menuTrigger, () => open(true));
146+
147+
trigger.remove();
148+
149+
expect(result.current.current).toBe(menuTrigger);
150+
});
151+
152+
it('resolves to nothing once the origin has left the page too', () => {
153+
const { result, open } = renderReturnFocus(null);
154+
withInteractionOrigin(menuTrigger, () => open(true));
155+
156+
menuTrigger.remove();
157+
158+
expect(result.current.current).toBeNull();
159+
});
160+
161+
it('forgets the origin on an open that has none', () => {
162+
const { result, open } = renderReturnFocus(null);
163+
withInteractionOrigin(menuTrigger, () => open(true));
164+
open(false);
165+
166+
open(true);
167+
168+
expect(result.current.current).toBeNull();
169+
});
170+
});
108171
});

‎packages/headless/src/hooks/use-return-focus.ts‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { useFloatingParentNodeId, useFloatingTree } from '@floating-ui/react';
55
import { useEffect, useMemo, useRef } from 'react';
66

77
import { isKeyboardEvent } from '../utils/interaction-modality';
8+
import { currentInteractionOrigin } from '../utils/interaction-origin';
89

910
/**
1011
* The element `FloatingFocusManager` restores focus to when the floating element closes.
@@ -18,8 +19,10 @@ import { isKeyboardEvent } from '../utils/interaction-modality';
1819
* A floating element opened from inside another one — a dialog from a menu item — may have no
1920
* trigger of its own, or one that is gone by the time it closes: the item unmounted with the menu.
2021
* Focus then goes to the nearest ancestor in the floating tree whose reference is still on the
21-
* page, which for a menu is its trigger. Resolved lazily, at restore time, since that is when it is
22-
* known whether the trigger survived.
22+
* page, which for a menu is its trigger. A dialog mounted outside that tree — one instance at the
23+
* end of a table, opened from any row's menu — has no ancestor to walk, so the interaction origin
24+
* the menu item published while its `onClick` ran is recorded on open and used instead. Resolved
25+
* lazily, at restore time, since that is when it is known whether the trigger survived.
2326
*
2427
* Pass the result to `FloatingFocusManager`'s `returnFocus`. On `null` it falls back to the
2528
* hidden guard element it keeps next to the trigger, so the tab position survives; verify that
@@ -32,6 +35,7 @@ export function useReturnFocus(
3235
const tree = useFloatingTree();
3336
const parentId = useFloatingParentNodeId();
3437
const triggerRef = useRef<HTMLElement | null>(null);
38+
const originRef = useRef<HTMLElement | null>(null);
3539
const dismissedByPointerRef = useRef(false);
3640
const trigger = elements.domReference;
3741

@@ -48,7 +52,9 @@ export function useReturnFocus(
4852
// forwarded without a reason — a Close button press — keeps the trigger, and programmatic
4953
// closes carry no event at all.
5054
function onOpenChange({ open, event, reason }: { open: boolean; event?: Event; reason?: OpenChangeReason }) {
51-
if (!open && event && reason && !isKeyboardEvent(event)) {
55+
if (open) {
56+
originRef.current = currentInteractionOrigin();
57+
} else if (event && reason && !isKeyboardEvent(event)) {
5258
dismissedByPointerRef.current = true;
5359
}
5460
}
@@ -67,6 +73,10 @@ export function useReturnFocus(
6773
if (own?.isConnected) {
6874
return own;
6975
}
76+
const origin = originRef.current;
77+
if (origin?.isConnected) {
78+
return origin;
79+
}
7080
return ancestorReference(tree, parentId);
7181
},
7282
set current(element: HTMLElement | null) {

‎packages/headless/src/primitives/dialog/dialog.test.tsx‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -814,6 +814,64 @@ describe('Dialog', () => {
814814
});
815815
});
816816

817+
describe('opened from a menu item into a dialog mounted elsewhere', () => {
818+
function HoistedFixture() {
819+
const handle = React.useMemo(() => Dialog.createHandle<string>(), []);
820+
return (
821+
<>
822+
<Menu.Root>
823+
<Menu.Trigger>Actions</Menu.Trigger>
824+
<Menu.Positioner>
825+
<Menu.Popup>
826+
<Menu.Item
827+
label='Remove'
828+
onClick={() => handle.open('item-1')}
829+
>
830+
Remove
831+
</Menu.Item>
832+
</Menu.Popup>
833+
</Menu.Positioner>
834+
</Menu.Root>
835+
<Dialog.Root handle={handle}>
836+
{({ payload }) => (
837+
<Dialog.Popup>
838+
<Dialog.Title>Remove {payload}?</Dialog.Title>
839+
<Dialog.Close>Cancel</Dialog.Close>
840+
</Dialog.Popup>
841+
)}
842+
</Dialog.Root>
843+
</>
844+
);
845+
}
846+
847+
it('returns focus to the menu trigger on Escape', async () => {
848+
const user = userEvent.setup();
849+
render(<HoistedFixture />);
850+
851+
const trigger = screen.getByRole('button', { name: 'Actions' });
852+
trigger.focus();
853+
await user.keyboard('{Enter}');
854+
await user.keyboard('{Enter}');
855+
expect(screen.getByRole('dialog', { name: 'Remove item-1?' })).toBeInTheDocument();
856+
857+
await user.keyboard('{Escape}');
858+
859+
expect(document.activeElement).toBe(trigger);
860+
});
861+
862+
it('returns focus to the menu trigger on Close press', async () => {
863+
const user = userEvent.setup();
864+
render(<HoistedFixture />);
865+
866+
const trigger = screen.getByRole('button', { name: 'Actions' });
867+
await user.click(trigger);
868+
await user.click(screen.getByRole('menuitem', { name: 'Remove' }));
869+
await user.click(screen.getByRole('button', { name: 'Cancel' }));
870+
871+
expect(document.activeElement).toBe(trigger);
872+
});
873+
});
874+
817875
it('resolves the function form with an empty type on programmatic close', () => {
818876
const handle = Dialog.createHandle();
819877
const finalFocus = vi.fn(() => undefined);

‎packages/headless/src/primitives/menu/menu-item.tsx‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { useFloatingTree, useListItem } from '@floating-ui/react';
44
import React from 'react';
55

66
import { type ComponentProps, type DefaultProps, mergeProps, useRender } from '../../utils';
7+
import { withInteractionOrigin } from '../../utils/interaction-origin';
78
import { useMenuContext } from './menu-context';
89

910
export interface MenuItemProps extends ComponentProps<'button'> {
@@ -14,12 +15,21 @@ export interface MenuItemProps extends ComponentProps<'button'> {
1415

1516
export const MenuItem = React.forwardRef<HTMLButtonElement, MenuItemProps>(function MenuItem(props, ref) {
1617
const { render, label, disabled, closeOnClick = true, onClick: consumerOnClick, ...otherProps } = props;
18+
const { activeIndex, getItemProps, refs } = useMenuContext();
1719
// When disabled, omit the consumer onClick entirely so mergeProps doesn't chain it.
1820
// When not disabled, add it back only if it's a function (avoids spreading onClick: undefined
19-
// which would overwrite the internal getItemProps handler via mergeProps).
21+
// which would overwrite the internal getItemProps handler via mergeProps). It runs with the
22+
// menu's trigger as the interaction origin, so a dialog it opens can return focus there.
2023
const safeOtherProps =
21-
!disabled && typeof consumerOnClick === 'function' ? { ...otherProps, onClick: consumerOnClick } : otherProps;
22-
const { activeIndex, getItemProps } = useMenuContext();
24+
!disabled && typeof consumerOnClick === 'function'
25+
? {
26+
...otherProps,
27+
onClick: (event: React.MouseEvent<HTMLButtonElement>) => {
28+
const reference = refs.domReference.current;
29+
withInteractionOrigin(reference instanceof HTMLElement ? reference : null, () => consumerOnClick(event));
30+
},
31+
}
32+
: otherProps;
2333
const tree = useFloatingTree();
2434
const item = useListItem({ label: disabled ? null : label });
2535
const isActive = item.index === activeIndex;
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
import { describe, expect, it } from 'vitest';
2+
3+
import { currentInteractionOrigin, withInteractionOrigin } from './interaction-origin';
4+
5+
describe('withInteractionOrigin', () => {
6+
it('exposes the element only while the callback runs', () => {
7+
const trigger = document.createElement('button');
8+
let seen: HTMLElement | null = null;
9+
10+
withInteractionOrigin(trigger, () => {
11+
seen = currentInteractionOrigin();
12+
});
13+
14+
expect(seen).toBe(trigger);
15+
expect(currentInteractionOrigin()).toBeNull();
16+
});
17+
18+
it('restores the enclosing origin when nested', () => {
19+
const outer = document.createElement('button');
20+
const inner = document.createElement('button');
21+
let afterInner: HTMLElement | null = null;
22+
23+
withInteractionOrigin(outer, () => {
24+
withInteractionOrigin(inner, () => undefined);
25+
afterInner = currentInteractionOrigin();
26+
});
27+
28+
expect(afterInner).toBe(outer);
29+
});
30+
31+
it('clears the origin when the callback throws', () => {
32+
const trigger = document.createElement('button');
33+
34+
expect(() =>
35+
withInteractionOrigin(trigger, () => {
36+
throw new Error('boom');
37+
}),
38+
).toThrow('boom');
39+
40+
expect(currentInteractionOrigin()).toBeNull();
41+
});
42+
43+
it('returns what the callback returns', () => {
44+
expect(withInteractionOrigin(null, () => 'value')).toBe('value');
45+
});
46+
});
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
/**
2+
* The element a user interaction is happening on behalf of, for the synchronous span of that
3+
* interaction. A menu item sets its menu's trigger here around the consumer's `onClick`; a
4+
* floating element opened during that span records it as somewhere focus can return to once its
5+
* own trigger is gone — the item unmounts with the menu, the trigger does not.
6+
*
7+
* Module-level on purpose: the item and the dialog it opens share no ancestor, so no context or
8+
* prop can carry this between them. It is only ever set inside {@link withInteractionOrigin} and
9+
* is cleared before that returns, so anything read after an `await` sees nothing.
10+
*/
11+
let origin: HTMLElement | null = null;
12+
13+
/** Runs `callback` with `element` as the current interaction origin, restoring the previous one after. */
14+
export function withInteractionOrigin<T>(element: HTMLElement | null, callback: () => T): T {
15+
const previous = origin;
16+
origin = element;
17+
try {
18+
return callback();
19+
} finally {
20+
origin = previous;
21+
}
22+
}
23+
24+
/** The element the current interaction is on behalf of, or `null` outside one. */
25+
export function currentInteractionOrigin(): HTMLElement | null {
26+
return origin;
27+
}

‎packages/swingset/src/stories/user-button.mdx‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,13 @@ trigger, no selection when personal is hidden and none is active, the account ot
8383
storyModule={UserButtonStories}
8484
/>
8585

86+
A user's avatar on its own rounds the trigger fully to match it.
87+
88+
<Story
89+
name='UserAvatarOnly'
90+
storyModule={UserButtonStories}
91+
/>
92+
8693
`renderTriggerBadge={false}` keeps the name. The badge is part of the label, so it needs both.
8794

8895
<Story

‎packages/swingset/src/stories/user-button.stories.tsx‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,18 @@ export function AvatarOnly(_args: Record<string, unknown>) {
278278
);
279279
}
280280

281+
export function UserAvatarOnly(_args: Record<string, unknown>) {
282+
const prototype = usePrototype();
283+
284+
return (
285+
<UserButtonView
286+
{...prototype}
287+
mode='user'
288+
renderTriggerLabel={false}
289+
/>
290+
);
291+
}
292+
281293
export function WithoutTriggerBadge(_args: Record<string, unknown>) {
282294
const prototype = usePrototype();
283295

0 commit comments

Comments
 (0)