Skip to content

Commit fec56ff

Browse files
LFDanLuLFDanLu
committed
Fix focusscope for subdialogs and add test
1 parent 9922dd7 commit fec56ff

File tree

4 files changed

+165
-16
lines changed

4 files changed

+165
-16
lines changed

‎packages/@react-aria/menu/src/useSubmenuTrigger.ts

+5-2
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {AriaMenuOptions} from './useMenu';
1515
import type {AriaPopoverProps, OverlayProps} from '@react-aria/overlays';
1616
import {FocusableElement, FocusStrategy, KeyboardEvent, Node, PressEvent, RefObject} from '@react-types/shared';
1717
import {focusWithoutScrolling, useEffectEvent, useId, useLayoutEffect} from '@react-aria/utils';
18+
import {getInteractionModality} from '@react-aria/interactions';
1819
import type {SubmenuTriggerState} from '@react-stately/menu';
1920
import {useCallback, useRef} from 'react';
2021
import {useLocale} from '@react-aria/i18n';
@@ -266,8 +267,10 @@ export function useSubmenuTrigger<T>(props: AriaSubmenuTriggerProps, state: Subm
266267
// Perhaps I could make subdialogs not have this prop but then screen readers wouldn't be able to navigate to the trigger and user won't be able to hover
267268
// the other items in the parent menu when the subdialog is open.
268269
isNonModal: true,
269-
// Only enable focusscope restore focus if we are using virtual focus, otherwise we'll be manually coercing focus back to the triggers on menu/dialog close
270-
disableFocusManagement: !isVirtualFocus,
270+
// We will manually coerce focus back to the triggers for mobile screen readers and non virtual focus use cases (aka submenus outside of autocomplete) so turn off
271+
// FocusScope then. For virtual focus use cases (Autocomplete subdialogs/menu) and subdialogs we want to keep FocusScope restoreFocus to automatically
272+
// send focus to parent subdialog input fields and/or tab containment
273+
disableFocusManagement: !isVirtualFocus && (getInteractionModality() === 'virtual' || type === 'menu'),
271274
shouldCloseOnInteractOutside
272275
}
273276
};

‎packages/react-aria-components/src/Menu.tsx

+1
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,7 @@ export const SubmenuTrigger = /*#__PURE__*/ createBranchComponent('submenutrigg
158158
);
159159
}, props => props.children[0]);
160160

161+
// TODO: make SubdialogTrigger unstable
161162
export interface SubdialogTriggerProps {
162163
/**
163164
* The contents of the SubDialogTrigger. The first child should be an Item (the trigger) and the second child should be the Popover (for the subdialog).

‎packages/react-aria-components/stories/Menu.stories.tsx

+66-13
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
*/
1212

1313
import {action} from '@storybook/addon-actions';
14-
import {Button, Dialog, Header, Heading, Keyboard, Menu, MenuSection, MenuTrigger, Popover, Separator, SubmenuTrigger, Text} from 'react-aria-components';
14+
import {Button, Dialog, Header, Heading, Input, Keyboard, Label, Menu, MenuSection, MenuTrigger, Popover, Separator, SubmenuTrigger, Text, TextField} from 'react-aria-components';
1515
import {MyMenuItem} from './utils';
1616
import React from 'react';
1717
import styles from '../example/index.css';
@@ -280,6 +280,7 @@ export const SubmenuSectionsExample = (args) => (
280280
</MenuTrigger>
281281
);
282282

283+
// TODO: figure out why it is autofocusing the Menu in the SubDialog
283284
export const SubdialogExample = (args) => (
284285
<MenuTrigger>
285286
<Button aria-label="Menu">☰</Button>
@@ -296,18 +297,70 @@ export const SubdialogExample = (args) => (
296297
padding: 5
297298
}}>
298299
<Dialog>
299-
<form style={{display: 'flex', flexDirection: 'column'}}>
300-
<Heading slot="title">Sign up</Heading>
301-
<label>
302-
First Name: <input placeholder="John" />
303-
</label>
304-
<label>
305-
Last Name: <input placeholder="Smith" />
306-
</label>
307-
<Button onPress={close} style={{marginTop: 10}}>
308-
Submit
309-
</Button>
310-
</form>
300+
{({close}) => (
301+
<form style={{display: 'flex', flexDirection: 'column'}}>
302+
<Heading slot="title">Sign up</Heading>
303+
<TextField autoFocus>
304+
<Label>First Name: </Label>
305+
<Input />
306+
</TextField>
307+
<TextField>
308+
<Label>Last Name: </Label>
309+
<Input />
310+
</TextField>
311+
<Menu>
312+
<SubmenuTrigger {...args}>
313+
<MyMenuItem id="A">A</MyMenuItem>
314+
<Popover
315+
style={{
316+
background: 'Canvas',
317+
color: 'CanvasText',
318+
border: '1px solid gray',
319+
padding: 5
320+
}}>
321+
<Menu>
322+
<MyMenuItem>1</MyMenuItem>
323+
<MyMenuItem>2</MyMenuItem>
324+
<MyMenuItem>3</MyMenuItem>
325+
</Menu>
326+
</Popover>
327+
</SubmenuTrigger>
328+
<SubDialogTrigger {...args}>
329+
<MyMenuItem id="B">B</MyMenuItem>
330+
<Popover
331+
style={{
332+
background: 'Canvas',
333+
color: 'CanvasText',
334+
border: '1px solid gray',
335+
padding: 5
336+
}}>
337+
<Dialog>
338+
{({close}) => (
339+
<form style={{display: 'flex', flexDirection: 'column'}}>
340+
<Heading slot="title">Sign up</Heading>
341+
<TextField autoFocus>
342+
<Label>Email: </Label>
343+
<Input />
344+
</TextField>
345+
<TextField>
346+
<Label>Contact number: </Label>
347+
<Input />
348+
</TextField>
349+
<Button onPress={close} style={{marginTop: 10}}>
350+
Submit
351+
</Button>
352+
</form>
353+
)}
354+
</Dialog>
355+
</Popover>
356+
</SubDialogTrigger>
357+
<MyMenuItem id="C">C</MyMenuItem>
358+
</Menu>
359+
<Button onPress={close} style={{marginTop: 10}}>
360+
Submit
361+
</Button>
362+
</form>
363+
)}
311364
</Dialog>
312365
</Popover>
313366
</SubDialogTrigger>

‎packages/react-aria-components/test/Menu.test.tsx

+93-1
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,10 @@
1212

1313
import {act, fireEvent, mockClickDefault, pointerMap, render, within} from '@react-spectrum/test-utils-internal';
1414
import {AriaMenuTests} from './AriaMenu.test-util';
15-
import {Button, Collection, Header, Keyboard, Menu, MenuContext, MenuItem, MenuSection, MenuTrigger, Popover, Separator, SubmenuTrigger, Text} from '..';
15+
import {Button, Collection, Dialog, Header, Heading, Input, Keyboard, Label, Menu, MenuContext, MenuItem, MenuSection, MenuTrigger, Popover, Separator, SubmenuTrigger, Text, TextField} from '..';
1616
import React, {useState} from 'react';
1717
import {Selection, SelectionMode} from '@react-types/shared';
18+
import {SubDialogTrigger} from '../src/Menu';
1819
import {UNSTABLE_PortalProvider} from '@react-aria/overlays';
1920
import {User} from '@react-aria/test-utils';
2021
import userEvent from '@testing-library/user-event';
@@ -1095,6 +1096,97 @@ describe('Menu', () => {
10951096
});
10961097
});
10971098

1099+
describe('Subdialog', function () {
1100+
it('should contain focus for subdialogs', async () => {
1101+
let onAction = jest.fn();
1102+
let {getByRole, getAllByRole} = render(
1103+
<MenuTrigger>
1104+
<Button aria-label="Menu">☰</Button>
1105+
<Popover>
1106+
<Menu onAction={onAction}>
1107+
<MenuItem id="open">Open</MenuItem>
1108+
<MenuItem id="rename">Rename…</MenuItem>
1109+
<MenuItem id="duplicate">Duplicate</MenuItem>
1110+
<SubDialogTrigger>
1111+
<MenuItem id="share">Share…</MenuItem>
1112+
<Popover>
1113+
<Dialog>
1114+
{({close}) => (
1115+
<form style={{display: 'flex', flexDirection: 'column'}}>
1116+
<Heading slot="title">Sign up</Heading>
1117+
<TextField>
1118+
<Label>First Name: </Label>
1119+
<Input />
1120+
</TextField>
1121+
<TextField>
1122+
<Label>Last Name: </Label>
1123+
<Input />
1124+
</TextField>
1125+
<Button onPress={close}>
1126+
Submit
1127+
</Button>
1128+
</form>
1129+
)}
1130+
</Dialog>
1131+
</Popover>
1132+
</SubDialogTrigger>
1133+
<MenuItem id="delete">Delete…</MenuItem>
1134+
</Menu>
1135+
</Popover>
1136+
</MenuTrigger>
1137+
);
1138+
1139+
let button = getByRole('button');
1140+
expect(button).not.toHaveAttribute('data-pressed');
1141+
1142+
await user.click(button);
1143+
expect(button).toHaveAttribute('data-pressed');
1144+
1145+
let menu = getAllByRole('menu')[0];
1146+
expect(getAllByRole('menuitem')).toHaveLength(5);
1147+
1148+
let popover = menu.closest('.react-aria-Popover');
1149+
expect(popover).toBeInTheDocument();
1150+
expect(popover).toHaveAttribute('data-trigger', 'MenuTrigger');
1151+
1152+
let triggerItem = getAllByRole('menuitem')[3];
1153+
expect(triggerItem).toHaveTextContent('Share…');
1154+
expect(triggerItem).toHaveAttribute('aria-haspopup', 'dialog');
1155+
expect(triggerItem).toHaveAttribute('aria-expanded', 'false');
1156+
// TODO: should this have a different data attribute aka has-subdialog?
1157+
expect(triggerItem).toHaveAttribute('data-has-submenu', 'true');
1158+
expect(triggerItem).not.toHaveAttribute('data-open');
1159+
1160+
// Open the subdialog
1161+
await user.pointer({target: triggerItem});
1162+
act(() => {jest.runAllTimers();});
1163+
expect(triggerItem).toHaveAttribute('data-hovered', 'true');
1164+
expect(triggerItem).toHaveAttribute('aria-expanded', 'true');
1165+
expect(triggerItem).toHaveAttribute('data-open', 'true');
1166+
let subdialog = getAllByRole('dialog')[0];
1167+
expect(subdialog).toBeInTheDocument();
1168+
1169+
let subdialogPopover = subdialog.closest('.react-aria-Popover') as HTMLElement;
1170+
expect(subdialogPopover).toBeInTheDocument();
1171+
expect(subdialogPopover).toHaveAttribute('data-trigger', 'SubDialogTrigger');
1172+
1173+
let inputs = within(subdialogPopover).getAllByRole('textbox');
1174+
let buttons = within(subdialogPopover).getAllByRole('button');
1175+
await user.click(inputs[0]);
1176+
expect(document.activeElement).toBe(inputs[0]);
1177+
await user.tab();
1178+
expect(document.activeElement).toBe(inputs[1]);
1179+
await user.tab();
1180+
expect(document.activeElement).toBe(buttons[0]);
1181+
await user.tab();
1182+
expect(document.activeElement).toBe(inputs[0]);
1183+
});
1184+
1185+
// TODO: add more tests
1186+
// nested subdialogs
1187+
// test ESC
1188+
});
1189+
10981190
describe('portalContainer', () => {
10991191
function InfoMenu(props) {
11001192
return (

0 commit comments

Comments
 (0)