Skip to content

Commit 8d39881

Browse files
Fix focus scope restore losing focus (#2166)
* Fix focus scope restore losing focus * Add a test for this case * reseting focus to nodeToRestore if nodeToRestore is within a parent FocusScope Co-authored-by: Daniel Lu <[email protected]>
1 parent bcb9d0a commit 8d39881

File tree

2 files changed

+61
-2
lines changed

2 files changed

+61
-2
lines changed

packages/@react-aria/focus/src/FocusScope.tsx

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -376,8 +376,14 @@ function useRestoreFocus(scopeRef: RefObject<HTMLElement[]>, restoreFocus: boole
376376
if (nextElement) {
377377
focusElement(nextElement, true);
378378
} else {
379-
// If there is no next element, blur the focused element to move focus to the body.
380-
focusedElement.blur();
379+
// If there is no next element and the nodeToRestore isn't within a FocusScope (i.e. we are leaving the top level focus scope)
380+
// then move focus to the body.
381+
// Otherwise restore focus to the nodeToRestore (e.g menu within a popover -> tabbing to close the menu should move focus to menu trigger)
382+
if (!isElementInAnyScope(nodeToRestore, scopes)) {
383+
focusedElement.blur();
384+
} else {
385+
focusElement(nodeToRestore, true);
386+
}
381387
}
382388
}
383389
};

packages/@react-spectrum/dialog/test/DialogTrigger.test.js

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {ActionButton, Button} from '@react-spectrum/button';
1515
import {ButtonGroup} from '@react-spectrum/buttongroup';
1616
import {Content} from '@react-spectrum/view';
1717
import {Dialog, DialogTrigger} from '../';
18+
import {Heading} from '@react-spectrum/text';
1819
import {Item, Menu, MenuTrigger} from '@react-spectrum/menu';
1920
import MatchMediaMock from 'jest-matchmedia-mock';
2021
import {Provider} from '@react-spectrum/provider';
@@ -28,9 +29,11 @@ import userEvent from '@testing-library/user-event';
2829
describe('DialogTrigger', function () {
2930
let matchMedia;
3031
beforeAll(() => {
32+
jest.spyOn(window.screen, 'width', 'get').mockImplementation(() => 1024);
3133
jest.useFakeTimers();
3234
});
3335
afterAll(() => {
36+
jest.clearAllMocks();
3437
jest.useRealTimers();
3538
});
3639

@@ -927,4 +930,54 @@ describe('DialogTrigger', function () {
927930

928931
expect(document.activeElement).toBe(innerInput);
929932
});
933+
934+
it('will not lose focus to body', async () => {
935+
let {getByRole, getByTestId} = render(
936+
<Provider theme={theme}>
937+
<DialogTrigger type="popover">
938+
<ActionButton>Trigger</ActionButton>
939+
<Dialog>
940+
<Heading>The Heading</Heading>
941+
<Content>
942+
<MenuTrigger>
943+
<ActionButton data-testid="innerButton">Test</ActionButton>
944+
<Menu autoFocus="first">
945+
<Item>Item 1</Item>
946+
<Item>Item 2</Item>
947+
<Item>Item 3</Item>
948+
</Menu>
949+
</MenuTrigger>
950+
</Content>
951+
</Dialog>
952+
</DialogTrigger>
953+
</Provider>
954+
);
955+
let button = getByRole('button');
956+
triggerPress(button);
957+
958+
act(() => {
959+
jest.runAllTimers();
960+
});
961+
962+
let outerDialog = getByRole('dialog');
963+
964+
await waitFor(() => {
965+
expect(outerDialog).toBeVisible();
966+
}); // wait for animation
967+
let innerButton = getByTestId('innerButton');
968+
userEvent.tab();
969+
fireEvent.keyDown(document.activeElement, {key: 'Enter'});
970+
fireEvent.keyUp(document.activeElement, {key: 'Enter'});
971+
972+
act(() => {
973+
jest.runAllTimers();
974+
});
975+
userEvent.tab();
976+
act(() => {
977+
jest.runAllTimers();
978+
});
979+
980+
expect(document.activeElement).toBe(innerButton);
981+
});
982+
930983
});

0 commit comments

Comments
 (0)