Skip to content

Commit e3b217a

Browse files
authored
feat: Automatically render popovers as dialogs (#7813)
* feat: Automatically render popovers as dialogs * remove disableFocusManagement * fix example * Update account menu and searchable select examples Dialog is no longer needed * cleanup tests
1 parent 85b3419 commit e3b217a

21 files changed

+516
-556
lines changed

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

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ 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';
1918
import type {SubmenuTriggerState} from '@react-stately/menu';
2019
import {useCallback, useRef} from 'react';
2120
import {useLocale} from '@react-aria/i18n';
@@ -99,6 +98,12 @@ export function useSubmenuTrigger<T>(props: AriaSubmenuTriggerProps, state: Subm
9998
}, [cancelOpenTimeout]);
10099

101100
let submenuKeyDown = (e: KeyboardEvent) => {
101+
// If focus is not within the menu, assume virtual focus is being used.
102+
// This means some other input element is also within the popover, so we shouldn't close the menu.
103+
if (!e.currentTarget.contains(document.activeElement)) {
104+
return;
105+
}
106+
102107
switch (e.key) {
103108
case 'ArrowLeft':
104109
if (direction === 'ltr' && e.currentTarget.contains(e.target as Element)) {
@@ -250,10 +255,6 @@ export function useSubmenuTrigger<T>(props: AriaSubmenuTriggerProps, state: Subm
250255
submenuProps,
251256
popoverProps: {
252257
isNonModal: true,
253-
// 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
254-
// FocusScope then. For virtual focus use cases (Autocomplete subdialogs/menu) and subdialogs we want to keep FocusScope restoreFocus to automatically
255-
// send focus to parent subdialog input fields and/or tab containment
256-
disableFocusManagement: !shouldUseVirtualFocus && (getInteractionModality() === 'virtual' || type === 'menu'),
257258
shouldCloseOnInteractOutside
258259
}
259260
};

packages/@react-aria/overlays/src/Overlay.tsx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,10 @@ export interface OverlayProps {
3232
* implement focus containment and restoration to ensure the overlay is keyboard accessible.
3333
*/
3434
disableFocusManagement?: boolean,
35+
/**
36+
* Whether to contain focus within the overlay.
37+
*/
38+
shouldContainFocus?: boolean,
3539
/**
3640
* Whether the overlay is currently performing an exit animation. When true,
3741
* focus is allowed to move outside.
@@ -63,7 +67,7 @@ export function Overlay(props: OverlayProps) {
6367
let contents = props.children;
6468
if (!props.disableFocusManagement) {
6569
contents = (
66-
<FocusScope restoreFocus contain={contain && !isExiting}>
70+
<FocusScope restoreFocus contain={(props.shouldContainFocus || contain) && !isExiting}>
6771
{contents}
6872
</FocusScope>
6973
);

packages/@react-aria/overlays/src/usePopover.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ export function usePopover(props: AriaPopoverProps, state: OverlayTriggerState):
8787
...otherProps
8888
} = props;
8989

90-
let isSubmenu = otherProps['trigger'] === 'SubmenuTrigger' || otherProps['trigger'] === 'SubDialogTrigger';
90+
let isSubmenu = otherProps['trigger'] === 'SubmenuTrigger';
9191

9292
let {overlayProps, underlayProps} = useOverlay(
9393
{

packages/@react-spectrum/menu/test/MenuTrigger.test.js

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -784,6 +784,36 @@ describe('MenuTrigger', function () {
784784
expect(tree.getByRole('menu').closest('[data-testid="custom-container"]')).toBe(tree.getByTestId('custom-container'));
785785
});
786786
});
787+
788+
it('closes if menu is tabbed away from', async function () {
789+
let tree = render(
790+
<Provider theme={theme}>
791+
<MenuTrigger>
792+
<Button variant="primary">
793+
{triggerText}
794+
</Button>
795+
<Menu>
796+
<Item id="1">One</Item>
797+
<Item id="2">Two</Item>
798+
<Item id="3">Three</Item>
799+
</Menu>
800+
</MenuTrigger>
801+
</Provider>
802+
);
803+
let menuTester = testUtilUser.createTester('Menu', {user, root: tree.container});
804+
menuTester.setInteractionType('keyboard');
805+
806+
await menuTester.open();
807+
act(() => {jest.runAllTimers();});
808+
809+
let menu = menuTester.menu;
810+
811+
await user.tab();
812+
act(() => {jest.runAllTimers();});
813+
act(() => {jest.runAllTimers();});
814+
expect(menu).not.toBeInTheDocument();
815+
expect(document.activeElement).toBe(menuTester.trigger);
816+
});
787817
});
788818

789819
function SelectionStatic(props) {

packages/@react-spectrum/menu/test/SubMenuTrigger.test.tsx

Lines changed: 6 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -443,7 +443,7 @@ describe('Submenu', function () {
443443
expect(document.activeElement).toBe(submenuTrigger2);
444444
});
445445

446-
it('should shift focus to the prev/next element adjacent to the menu trigger when pressing Tab', async function () {
446+
it('should contain focus when pressing Tab', async function () {
447447
async function openSubMenus() {
448448
await user.keyboard('[ArrowDown]');
449449
act(() => {jest.runAllTimers();});
@@ -468,30 +468,18 @@ describe('Submenu', function () {
468468
}
469469

470470
let tree = render(<TabBehaviorStory />);
471-
let inputleft = tree.getByTestId('inputleft');
472-
let inputright = tree.getByTestId('inputright');
473-
let triggerButton = tree.getByRole('button');
474471
await user.tab();
475472
await user.tab();
476473
await openSubMenus();
477474

478-
// Tab should close all menus and move focus to the next input relative to the original menu trigger
475+
476+
// Tab do nothing.
479477
await user.tab();
478+
let activeElement = document.activeElement;
480479
act(() => {jest.runAllTimers();});
481480
let menus = tree.queryAllByRole('menu', {hidden: true});
482-
expect(menus).toHaveLength(0);
483-
expect(document.activeElement).toBe(inputright);
484-
485-
await user.tab({shift: true});
486-
expect(document.activeElement).toBe(triggerButton);
487-
await openSubMenus();
488-
489-
// Shift + Tab should close all menus and move focus to the prev input relative to the original menu trigger
490-
await user.tab({shift: true});
491-
act(() => {jest.runAllTimers();});
492-
menus = tree.queryAllByRole('menu', {hidden: true});
493-
expect(menus).toHaveLength(0);
494-
expect(document.activeElement).toBe(inputleft);
481+
expect(menus).toHaveLength(3);
482+
expect(document.activeElement).toBe(activeElement);
495483
});
496484
});
497485

packages/react-aria-components/docs/ComboBox.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ import {ComboBox, Label, Input, Button, Popover, ListBox, ListBoxItem} from 'rea
140140
}
141141

142142
.react-aria-ListBoxItem {
143-
padding: 0.286rem 0.571rem 0.286rem 1.571rem;
143+
padding: 0 0.571rem 0 1.571rem;
144144

145145
&[data-focus-visible] {
146146
outline: none;

packages/react-aria-components/docs/Menu.mdx

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ import {MenuTrigger, Button, Popover, Menu, MenuItem} from 'react-aria-component
6868
```css hidden
6969
@import './Button.mdx' layer(button);
7070
@import './Popover.mdx' layer(popover);
71+
@import './SearchField.mdx' layer(searchfield);
7172
```
7273

7374
```css
@@ -842,6 +843,52 @@ let items = [
842843

843844
</details>
844845

846+
### Complex content
847+
848+
Submenu popovers can also include components other than menus. This example uses an [Autocomplete](Autocomplete.html) to make the submenu searchable.
849+
850+
```tsx example
851+
import {Menu, Popover, SubmenuTrigger, Autocomplete, useFilter} from 'react-aria-components';
852+
import {MySearchField} from './SearchField';
853+
854+
function Example() {
855+
let {contains} = useFilter({sensitivity: 'base'});
856+
857+
return (
858+
<MyMenuButton label="Actions">
859+
<MyItem>Cut</MyItem>
860+
<MyItem>Copy</MyItem>
861+
<MyItem>Delete</MyItem>
862+
<SubmenuTrigger>
863+
<MyItem>Add tag...</MyItem>
864+
<Popover>
865+
<Autocomplete filter={contains}>
866+
<MySearchField label="Search tags" autoFocus />
867+
<Menu>
868+
<MyItem>News</MyItem>
869+
<MyItem>Travel</MyItem>
870+
<MyItem>Shopping</MyItem>
871+
<MyItem>Business</MyItem>
872+
<MyItem>Entertainment</MyItem>
873+
<MyItem>Food</MyItem>
874+
<MyItem>Technology</MyItem>
875+
<MyItem>Health</MyItem>
876+
<MyItem>Science</MyItem>
877+
</Menu>
878+
</Autocomplete>
879+
</Popover>
880+
</SubmenuTrigger>
881+
</MyMenuButton>
882+
);
883+
}
884+
```
885+
886+
```css hidden
887+
.react-aria-Popover[data-trigger=SubmenuTrigger] .react-aria-SearchField {
888+
margin: 4px 8px;
889+
}
890+
```
891+
845892
## Custom trigger
846893

847894
`MenuTrigger` works out of the box with any pressable React Aria component (e.g. [Button](Button.html), [Link](Link.html), etc.). Custom trigger elements such as third party components and other DOM elements are also supported by wrapping them with the `<Pressable>` component, or using the [usePress](usePress.html) hook.

packages/react-aria-components/docs/Select.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ import {Select, SelectValue, Label, Button, Popover, ListBox, ListBoxItem} from
137137
}
138138

139139
.react-aria-ListBoxItem {
140-
padding: 0.286rem 0.571rem 0.286rem 1.571rem;
140+
padding: 0 0.571rem 0 1.571rem;
141141

142142
&[data-focus-visible] {
143143
outline: none;

packages/react-aria-components/docs/examples/account-menu.mdx

Lines changed: 21 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -39,39 +39,37 @@ import './tailwind.global.css';
3939
```
4040

4141
```tsx example standalone
42-
import {DialogTrigger, Button, Popover, Dialog, Menu, MenuItem, Separator, Switch, composeRenderProps} from 'react-aria-components';
42+
import {MenuTrigger, Button, Popover, Menu, MenuItem, Separator, Switch, composeRenderProps} from 'react-aria-components';
4343
import type {MenuItemProps, SwitchProps} from 'react-aria-components';
4444

4545
function AccountMenuExample() {
4646
return (
4747
<div className="p-8 bg-gray-50 dark:bg-zinc-900 rounded-lg flex items-start justify-center">
48-
<DialogTrigger>
48+
<MenuTrigger>
4949
<Button aria-label="Account" className="inline-flex items-center justify-center rounded-md p-1.5 text-white bg-transparent border-none hover:bg-gray-200 pressed:bg-gray-300 dark:hover:bg-zinc-800 dark:pressed:bg-zinc-700 transition-colors cursor-default outline-hidden focus-visible:ring-2 focus-visible:ring-blue-600">
5050
<img alt="" src="https://i.imgur.com/xIe7Wlb.png" className="w-7 h-7 rounded-full" />
5151
</Button>
52-
<Popover placement="bottom end" className="p-2 overflow-auto rounded-lg bg-white dark:bg-zinc-950 shadow-lg ring-1 ring-black/10 dark:ring-white/15 entering:animate-in entering:fade-in entering:placement-bottom:slide-in-from-top-1 entering:placement-top:slide-in-from-bottom-1 exiting:animate-out exiting:fade-out exiting:placement-bottom:slide-out-to-top-1 exiting:placement-top:slide-out-to-bottom-1 fill-mode-forwards origin-top-left">
53-
<Dialog className="outline-hidden">
54-
<div className="flex gap-2 items-center mx-3 mt-2">
55-
<img alt="" src="https://i.imgur.com/xIe7Wlb.png" className="w-16 h-16 rounded-full" />
56-
<div className="flex flex-col gap-1">
57-
<div className="text-[15px] font-bold text-gray-900 dark:text-gray-100 leading-none">Marissa Whitaker</div>
58-
<div className="text-base text-gray-900 dark:text-gray-100 leading-none mb-1">[email protected]</div>
59-
<MySwitch>Dark Mode</MySwitch>
60-
</div>
52+
<Popover className="p-2 overflow-auto outline-hidden rounded-lg bg-white dark:bg-zinc-950 shadow-lg ring-1 ring-black/10 dark:ring-white/15 entering:animate-in entering:fade-in entering:placement-bottom:slide-in-from-top-1 entering:placement-top:slide-in-from-bottom-1 exiting:animate-out exiting:fade-out exiting:placement-bottom:slide-out-to-top-1 exiting:placement-top:slide-out-to-bottom-1 fill-mode-forwards origin-top-left">
53+
<div className="flex gap-2 items-center mx-3 mt-2">
54+
<img alt="" src="https://i.imgur.com/xIe7Wlb.png" className="w-16 h-16 rounded-full" />
55+
<div className="flex flex-col gap-1">
56+
<div className="text-[15px] font-bold text-gray-900 dark:text-gray-100 leading-none">Marissa Whitaker</div>
57+
<div className="text-base text-gray-900 dark:text-gray-100 leading-none mb-1">[email protected]</div>
58+
<MySwitch>Dark Mode</MySwitch>
6159
</div>
62-
<Separator className="border-none bg-gray-300 dark:bg-zinc-600 h-[1px] mx-3 mt-4 mb-2" />
63-
<Menu className="outline-hidden">
64-
<MyMenuItem id="new">Account Settings</MyMenuItem>
65-
<MyMenuItem id="open">Support</MyMenuItem>
66-
<Separator className="bg-gray-300 dark:bg-zinc-600 h-[1px] mx-3 my-2" />
67-
<MyMenuItem id="save">Legal notices</MyMenuItem>
68-
<MyMenuItem id="save-as">About</MyMenuItem>
69-
<Separator className="bg-gray-300 dark:bg-zinc-600 h-[1px] mx-3 my-2" />
70-
<MyMenuItem id="print">Sign out</MyMenuItem>
71-
</Menu>
72-
</Dialog>
60+
</div>
61+
<Separator className="border-none bg-gray-300 dark:bg-zinc-600 h-[1px] mx-3 mt-4 mb-2" />
62+
<Menu className="outline-hidden">
63+
<MyMenuItem id="new">Account Settings</MyMenuItem>
64+
<MyMenuItem id="open">Support</MyMenuItem>
65+
<Separator className="bg-gray-300 dark:bg-zinc-600 h-[1px] mx-3 my-2" />
66+
<MyMenuItem id="save">Legal notices</MyMenuItem>
67+
<MyMenuItem id="save-as">About</MyMenuItem>
68+
<Separator className="bg-gray-300 dark:bg-zinc-600 h-[1px] mx-3 my-2" />
69+
<MyMenuItem id="print">Sign out</MyMenuItem>
70+
</Menu>
7371
</Popover>
74-
</DialogTrigger>
72+
</MenuTrigger>
7573
</div>
7674
);
7775
}

packages/react-aria-components/docs/examples/searchable-select.mdx

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ const languages = [
8787

8888
```tsx example standalone
8989
import type {ListBoxItemProps} from 'react-aria-components';
90-
import {Autocomplete, Select, Label, Button, SelectValue, Popover, Dialog, ListBox, ListBoxItem, SearchField, Input, useFilter} from 'react-aria-components';
90+
import {Autocomplete, Select, Label, Button, SelectValue, Popover, ListBox, ListBoxItem, SearchField, Input, useFilter} from 'react-aria-components';
9191
import {ChevronsUpDownIcon, CheckIcon, CheckIcon, SearchIcon, XIcon} from 'lucide-react';
9292

9393
function SelectExample() {
@@ -101,21 +101,19 @@ function SelectExample() {
101101
<SelectValue className="flex-1 truncate" />
102102
<ChevronsUpDownIcon className="w-4 h-4" />
103103
</Button>
104-
<Popover className="!max-h-80 w-(--trigger-width) rounded-md bg-white text-base shadow-lg ring-1 ring-black/5 entering:animate-in entering:fade-in exiting:animate-out exiting:fade-out">
105-
<Dialog className="flex flex-col max-h-[inherit]">
106-
<Autocomplete filter={contains}>
107-
<SearchField aria-label="Search" autoFocus className="group flex items-center bg-white forced-colors:bg-[Field] border-2 border-gray-300 has-focus:border-sky-600 rounded-full m-1">
108-
<SearchIcon aria-hidden className="w-4 h-4 ml-2 text-gray-600 forced-colors:text-[ButtonText]" />
109-
<Input placeholder="Search languages" className="px-2 py-1 flex-1 min-w-0 border-none outline outline-0 bg-white text-base text-gray-800 placeholder-gray-500 font-[inherit] [&::-webkit-search-cancel-button]:hidden" />
110-
<Button className="text-sm text-center transition rounded-full border-0 p-1 flex items-center justify-center text-gray-600 bg-transparent hover:bg-black/[5%] pressed:bg-black/10 mr-1 w-6 group-empty:invisible">
111-
<XIcon aria-hidden className="w-4 h-4" />
112-
</Button>
113-
</SearchField>
114-
<ListBox items={languages} className="outline-hidden p-1 overflow-auto flex-1 scroll-pb-1">
115-
{item => <SelectItem>{item.name}</SelectItem>}
116-
</ListBox>
117-
</Autocomplete>
118-
</Dialog>
104+
<Popover className="!max-h-80 w-(--trigger-width) flex flex-col rounded-md bg-white text-base shadow-lg ring-1 ring-black/5 entering:animate-in entering:fade-in exiting:animate-out exiting:fade-out">
105+
<Autocomplete filter={contains}>
106+
<SearchField aria-label="Search" autoFocus className="group flex items-center bg-white forced-colors:bg-[Field] border-2 border-gray-300 has-focus:border-sky-600 rounded-full m-1">
107+
<SearchIcon aria-hidden className="w-4 h-4 ml-2 text-gray-600 forced-colors:text-[ButtonText]" />
108+
<Input placeholder="Search languages" className="px-2 py-1 flex-1 min-w-0 border-none outline outline-0 bg-white text-base text-gray-800 placeholder-gray-500 font-[inherit] [&::-webkit-search-cancel-button]:hidden" />
109+
<Button className="text-sm text-center transition rounded-full border-0 p-1 flex items-center justify-center text-gray-600 bg-transparent hover:bg-black/[5%] pressed:bg-black/10 mr-1 w-6 group-empty:invisible">
110+
<XIcon aria-hidden className="w-4 h-4" />
111+
</Button>
112+
</SearchField>
113+
<ListBox items={languages} className="outline-hidden p-1 overflow-auto flex-1 scroll-pb-1">
114+
{item => <SelectItem>{item.name}</SelectItem>}
115+
</ListBox>
116+
</Autocomplete>
119117
</Popover>
120118
</Select>
121119
</div>

packages/react-aria-components/src/Dialog.tsx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,11 @@ export function DialogTrigger(props: DialogTriggerProps) {
6161
[OverlayTriggerStateContext, state],
6262
[RootMenuTriggerStateContext, state],
6363
[DialogContext, overlayProps],
64-
[PopoverContext, {trigger: 'DialogTrigger', triggerRef: buttonRef}]
64+
[PopoverContext, {
65+
trigger: 'DialogTrigger',
66+
triggerRef: buttonRef,
67+
'aria-labelledby': overlayProps['aria-labelledby']
68+
}]
6569
]}>
6670
<PressResponder {...triggerProps} ref={buttonRef} isPressed={state.isOpen}>
6771
{props.children}

0 commit comments

Comments
 (0)