Skip to content

Commit 4902631

Browse files
devongovettdevongovett
authored
Focus first element in scope if nodeToRestore does not exist (#3385)
1 parent 09a0489 commit 4902631

File tree

12 files changed

+491
-237
lines changed

12 files changed

+491
-237
lines changed

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

Lines changed: 68 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
import {FocusableElement} from '@react-types/shared';
1414
import {focusSafely} from './focusSafely';
1515
import {isElementVisible} from './isElementVisible';
16-
import React, {ReactNode, RefObject, useContext, useEffect, useRef} from 'react';
16+
import React, {ReactNode, RefObject, useContext, useEffect, useMemo, useRef} from 'react';
1717
import {useLayoutEffect} from '@react-aria/utils';
1818

1919

@@ -85,8 +85,12 @@ export function FocusScope(props: FocusScopeProps) {
8585
let endRef = useRef<HTMLSpanElement>();
8686
let scopeRef = useRef<Element[]>([]);
8787
let ctx = useContext(FocusContext);
88-
// if there is no scopeRef on the context, then the parent is the focusScopeTree's root, represented by null
89-
let parentScope = ctx?.scopeRef ?? null;
88+
89+
// The parent scope is based on the JSX tree, using context.
90+
// However, if a new scope mounts outside the active scope (e.g. DialogContainer launched from a menu),
91+
// we want the parent scope to be the active scope instead.
92+
let ctxParent = ctx?.scopeRef ?? null;
93+
let parentScope = useMemo(() => activeScope && focusScopeTree.getTreeNode(activeScope) && !isAncestorScope(activeScope, ctxParent) ? activeScope : ctxParent, [ctxParent]);
9094

9195
useLayoutEffect(() => {
9296
// Find all rendered nodes between the sentinels and add them to the scope.
@@ -109,14 +113,18 @@ export function FocusScope(props: FocusScopeProps) {
109113
let node = focusScopeTree.getTreeNode(scopeRef);
110114
node.contain = contain;
111115

116+
useActiveScopeTracker(scopeRef, restoreFocus, contain);
112117
useFocusContainment(scopeRef, contain);
113118
useRestoreFocus(scopeRef, restoreFocus, contain);
114119
useAutoFocus(scopeRef, autoFocus);
115120

116121
// this layout effect needs to run last so that focusScopeTree cleanup happens at the last moment possible
117122
useLayoutEffect(() => {
118-
if (scopeRef && (parentScope || parentScope == null)) {
123+
if (scopeRef) {
119124
return () => {
125+
// Scope may have been re-parented.
126+
let parentScope = focusScopeTree.getTreeNode(scopeRef).parent.scopeRef;
127+
120128
// Restore the active scope on unmount if this scope or a descendant scope is active.
121129
// Parent effect cleanups run before children, so we need to check if the
122130
// parent scope actually still exists before restoring the active scope to it.
@@ -294,7 +302,7 @@ function useFocusContainment(scopeRef: RefObject<Element[]>, contain: boolean) {
294302
let onFocus = (e) => {
295303
// If focusing an element in a child scope of the currently active scope, the child becomes active.
296304
// Moving out of the active scope to an ancestor is not allowed.
297-
if (!activeScope || isAncestorScope(activeScope, scopeRef)) {
305+
if ((!activeScope || isAncestorScope(activeScope, scopeRef)) && isElementInScope(e.target, scopeRef.current)) {
298306
activeScope = scopeRef;
299307
focusedNode.current = e.target;
300308
} else if (shouldContainFocus(scopeRef) && !isElementInChildScope(e.target, scopeRef)) {
@@ -424,6 +432,47 @@ function useAutoFocus(scopeRef: RefObject<Element[]>, autoFocus: boolean) {
424432
}, [scopeRef]);
425433
}
426434

435+
function useActiveScopeTracker(scopeRef: RefObject<Element[]>, restore: boolean, contain: boolean) {
436+
// tracks the active scope, in case restore and contain are both false.
437+
// if either are true, this is tracked in useRestoreFocus or useFocusContainment.
438+
useLayoutEffect(() => {
439+
if (restore || contain) {
440+
return;
441+
}
442+
443+
let scope = scopeRef.current;
444+
445+
let onFocus = (e: FocusEvent) => {
446+
let target = e.target as Element;
447+
if (isElementInScope(target, scopeRef.current)) {
448+
activeScope = scopeRef;
449+
} else if (!isElementInAnyScope(target)) {
450+
activeScope = null;
451+
}
452+
};
453+
454+
document.addEventListener('focusin', onFocus, false);
455+
scope.forEach(element => element.addEventListener('focusin', onFocus, false));
456+
return () => {
457+
document.removeEventListener('focusin', onFocus, false);
458+
scope.forEach(element => element.removeEventListener('focusin', onFocus, false));
459+
};
460+
}, [scopeRef, restore, contain]);
461+
}
462+
463+
function shouldRestoreFocus(scopeRef: ScopeRef) {
464+
let scope = focusScopeTree.getTreeNode(activeScope);
465+
while (scope && scope.scopeRef !== scopeRef) {
466+
if (scope.nodeToRestore) {
467+
return false;
468+
}
469+
470+
scope = scope.parent;
471+
}
472+
473+
return true;
474+
}
475+
427476
function useRestoreFocus(scopeRef: RefObject<Element[]>, restoreFocus: boolean, contain: boolean) {
428477
// create a ref during render instead of useLayoutEffect so the active element is saved before a child with autoFocus=true mounts.
429478
const nodeToRestoreRef = useRef(typeof document !== 'undefined' ? document.activeElement as FocusableElement : null);
@@ -454,11 +503,12 @@ function useRestoreFocus(scopeRef: RefObject<Element[]>, restoreFocus: boolean,
454503

455504
// useLayoutEffect instead of useEffect so the active element is saved synchronously instead of asynchronously.
456505
useLayoutEffect(() => {
457-
focusScopeTree.getTreeNode(scopeRef).nodeToRestore = nodeToRestoreRef.current;
458506
if (!restoreFocus) {
459507
return;
460508
}
461509

510+
focusScopeTree.getTreeNode(scopeRef).nodeToRestore = nodeToRestoreRef.current;
511+
462512
// Handle the Tab key so that tabbing out of the scope goes to the next element
463513
// after the node that had focus when the scope mounted. This is important when
464514
// using portals for overlays, so that focus goes to the expected element when
@@ -529,7 +579,7 @@ function useRestoreFocus(scopeRef: RefObject<Element[]>, restoreFocus: boolean,
529579
&& nodeToRestore
530580
&& (
531581
isElementInScope(document.activeElement, scopeRef.current)
532-
|| (document.activeElement === document.body && activeScope === scopeRef)
582+
|| (document.activeElement === document.body && shouldRestoreFocus(scopeRef))
533583
)
534584
) {
535585
// freeze the focusScopeTree so it persists after the raf, otherwise during unmount nodes are removed from it
@@ -546,6 +596,17 @@ function useRestoreFocus(scopeRef: RefObject<Element[]>, restoreFocus: boolean,
546596
}
547597
treeNode = treeNode.parent;
548598
}
599+
600+
// If no nodeToRestore was found, focus the first element in the nearest
601+
// ancestor scope that is still in the tree.
602+
treeNode = clonedTree.getTreeNode(scopeRef);
603+
while (treeNode) {
604+
if (treeNode.scopeRef && focusScopeTree.getTreeNode(treeNode.scopeRef)) {
605+
focusFirstInScope(treeNode.scopeRef.current, true);
606+
return;
607+
}
608+
treeNode = treeNode.parent;
609+
}
549610
}
550611
});
551612
}

‎packages/@react-spectrum/actionbar/stories/Example.tsx

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ let defaultItems = [
5050
export function Example(props: any = {}) {
5151
const [selectedKeys, setSelectedKeys] = useState<Selection>(props.defaultSelectedKeys || new Set());
5252
let [items, setItems] = useState(defaultItems);
53+
5354
let ref = useRef(null);
5455
return (
5556
<ActionBarContainer height={props.containerHeight || 300}>
@@ -88,28 +89,26 @@ export function Example(props: any = {}) {
8889
}
8990
setItems(newItems);
9091
setSelectedKeys(new Set());
91-
// TODO need to solve restore focus when the row we arrived from is deleted, tableview ref is not focusable
92-
// ref.current.focus();
9392
}
9493
}
9594
})}>
96-
<Item key="edit">
95+
<Item key="edit" textValue="Edit">
9796
<Edit />
9897
<Text>Edit</Text>
9998
</Item>
100-
<Item key="copy">
99+
<Item key="copy" textValue="Copy">
101100
<Copy />
102101
<Text>Copy</Text>
103102
</Item>
104-
<Item key="delete">
103+
<Item key="delete" textValue="Delete">
105104
<Delete />
106105
<Text>Delete</Text>
107106
</Item>
108-
<Item key="move">
107+
<Item key="move" textValue="Move">
109108
<Move />
110109
<Text>Move</Text>
111110
</Item>
112-
<Item key="duplicate">
111+
<Item key="duplicate" textValue="Duplicate">
113112
<Duplicate />
114113
<Text>Duplicate</Text>
115114
</Item>

‎packages/@react-spectrum/actionbar/test/ActionBar.test.js

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,118 @@ describe('ActionBar', () => {
167167
expect(document.activeElement).toBe(rows[1]);
168168
});
169169

170+
it('should restore focus to the the new first row if the row we wanted to restore to was removed', async () => {
171+
let tree = render(<Provider theme={theme}><Example /></Provider>);
172+
act(() => {jest.runAllTimers();});
173+
174+
let table = tree.getByRole('grid');
175+
let rows = within(table).getAllByRole('row');
176+
let checkbox = within(rows[1]).getByRole('checkbox');
177+
178+
userEvent.tab();
179+
expect(document.activeElement).toBe(rows[1]);
180+
fireEvent.keyDown(document.activeElement, {key: 'Enter'});
181+
fireEvent.keyUp(document.activeElement, {key: 'Enter'});
182+
expect(checkbox).toBeChecked();
183+
act(() => jest.runAllTimers());
184+
185+
let toolbar = tree.getByRole('toolbar');
186+
expect(toolbar).toBeVisible();
187+
expect(document.activeElement).toBe(rows[1]);
188+
189+
// Simulate tabbing within the table
190+
fireEvent.keyDown(document.activeElement, {key: 'Tab'});
191+
let walker = getFocusableTreeWalker(document.body, {tabbable: true});
192+
walker.currentNode = document.activeElement;
193+
act(() => {walker.nextNode().focus();});
194+
fireEvent.keyUp(document.activeElement, {key: 'Tab'});
195+
expect(document.activeElement).toBe(within(toolbar).getAllByRole('button')[0]);
196+
197+
fireEvent.keyDown(document.activeElement, {key: 'ArrowRight'});
198+
fireEvent.keyUp(document.activeElement, {key: 'ArrowRight'});
199+
200+
fireEvent.keyDown(document.activeElement, {key: 'ArrowRight'});
201+
fireEvent.keyUp(document.activeElement, {key: 'ArrowRight'});
202+
203+
expect(document.activeElement).toBe(within(toolbar).getAllByRole('button')[2]);
204+
205+
fireEvent.keyDown(document.activeElement, {key: 'Enter'});
206+
fireEvent.keyUp(document.activeElement, {key: 'Enter'});
207+
208+
act(() => jest.runAllTimers());
209+
act(() => jest.runAllTimers());
210+
await act(async () => Promise.resolve());
211+
expect(rows[1]).not.toBeInTheDocument();
212+
213+
rows = within(table).getAllByRole('row');
214+
expect(toolbar).not.toBeInTheDocument();
215+
expect(document.activeElement).toBe(rows[1]);
216+
});
217+
218+
it('should restore focus to the new first row if the row we wanted to restore to was removed via actiongroup menu', async () => {
219+
let tree = render(<Provider theme={theme}><Example /></Provider>);
220+
act(() => {jest.runAllTimers();});
221+
222+
let table = tree.getByRole('grid');
223+
let rows = within(table).getAllByRole('row');
224+
let checkbox = within(rows[1]).getByRole('checkbox');
225+
226+
userEvent.tab();
227+
expect(document.activeElement).toBe(rows[1]);
228+
fireEvent.keyDown(document.activeElement, {key: 'Enter'});
229+
fireEvent.keyUp(document.activeElement, {key: 'Enter'});
230+
expect(checkbox).toBeChecked();
231+
act(() => jest.runAllTimers());
232+
233+
let toolbar = tree.getByRole('toolbar');
234+
expect(toolbar).toBeVisible();
235+
expect(document.activeElement).toBe(rows[1]);
236+
237+
jest.spyOn(toolbar.parentNode, 'offsetWidth', 'get').mockImplementation(() => 200);
238+
for (let action of toolbar.childNodes) {
239+
jest.spyOn(action, 'offsetWidth', 'get').mockImplementation(() => 75);
240+
}
241+
fireEvent(window, new Event('resize'));
242+
243+
// Simulate tabbing within the table
244+
fireEvent.keyDown(document.activeElement, {key: 'Tab'});
245+
let walker = getFocusableTreeWalker(document.body, {tabbable: true});
246+
walker.currentNode = document.activeElement;
247+
act(() => {walker.nextNode().focus();});
248+
fireEvent.keyUp(document.activeElement, {key: 'Tab'});
249+
expect(document.activeElement).toBe(within(toolbar).getAllByRole('button')[0]);
250+
251+
fireEvent.keyDown(document.activeElement, {key: 'ArrowRight'});
252+
fireEvent.keyUp(document.activeElement, {key: 'ArrowRight'});
253+
254+
fireEvent.keyDown(document.activeElement, {key: 'ArrowRight'});
255+
fireEvent.keyUp(document.activeElement, {key: 'ArrowRight'});
256+
257+
expect(within(toolbar).getAllByRole('button')).toHaveLength(3);
258+
expect(document.activeElement).toBe(within(toolbar).getAllByRole('button')[2]);
259+
260+
fireEvent.keyDown(document.activeElement, {key: 'Enter'});
261+
fireEvent.keyUp(document.activeElement, {key: 'Enter'});
262+
263+
let listItems = tree.getAllByRole('menuitem');
264+
expect(document.activeElement).toBe(listItems[0]);
265+
expect(document.activeElement.textContent).toBe('Delete');
266+
267+
fireEvent.keyDown(document.activeElement, {key: 'Enter'});
268+
fireEvent.keyUp(document.activeElement, {key: 'Enter'});
269+
270+
act(() => jest.runAllTimers());
271+
act(() => jest.runAllTimers());
272+
await act(async () => Promise.resolve());
273+
rows = within(table).getAllByRole('row');
274+
// row reused by the virtualizer, so it's still in dom, but its contents have been swapped out
275+
expect(rows[1].textContent).not.toBe('Foo 1Bar 1Baz 1');
276+
277+
rows = within(table).getAllByRole('row');
278+
expect(toolbar).not.toBeInTheDocument();
279+
expect(document.activeElement).toBe(rows[1]);
280+
});
281+
170282
it('should fire onAction when clicking on an action', () => {
171283
let onAction = jest.fn();
172284
let tree = render(<Provider theme={theme}><Example onAction={onAction} /></Provider>);

‎packages/@react-spectrum/actiongroup/src/ActionGroup.tsx

Lines changed: 38 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import {
2323
useStyleProps
2424
} from '@react-spectrum/utils';
2525
import {filterDOMProps, mergeProps, useId, useLayoutEffect, useResizeObserver, useValueEffect} from '@react-aria/utils';
26+
import {FocusScope} from '@react-aria/focus';
2627
import {Item, Menu, MenuTrigger} from '@react-spectrum/menu';
2728
import {ListState, useListState} from '@react-stately/list';
2829
import More from '@spectrum-icons/workflow/More';
@@ -215,42 +216,44 @@ function ActionGroup<T extends object>(props: SpectrumActionGroupProps<T>, ref:
215216
};
216217

217218
return (
218-
<div {...styleProps} style={style} className={classNames(styles, 'flex-container', styleProps.className)} ref={wrapperRef}>
219-
<div
220-
{...actionGroupProps}
221-
ref={domRef}
222-
className={
223-
classNames(
224-
styles,
225-
'flex-gap',
226-
'spectrum-ActionGroup',
227-
{
228-
'spectrum-ActionGroup--quiet': isQuiet,
229-
'spectrum-ActionGroup--vertical': isVertical,
230-
'spectrum-ActionGroup--compact': density === 'compact',
231-
'spectrum-ActionGroup--justified': isJustified && !isMeasuring,
232-
'spectrum-ActionGroup--overflowCollapse': overflowMode === 'collapse'
233-
},
234-
otherProps.UNSAFE_className
235-
)
236-
}>
237-
<Provider {...providerProps}>
238-
{children.map((item) => (
239-
<ActionGroupItem
240-
key={item.key}
241-
onAction={onAction}
242-
isDisabled={isDisabled}
243-
isEmphasized={isEmphasized}
244-
staticColor={staticColor}
245-
item={item}
246-
state={state}
247-
hideButtonText={hideButtonText}
248-
orientation={orientation} />
249-
))}
250-
{menuItem}
251-
</Provider>
219+
<FocusScope>
220+
<div {...styleProps} style={style} className={classNames(styles, 'flex-container', styleProps.className)} ref={wrapperRef}>
221+
<div
222+
{...actionGroupProps}
223+
ref={domRef}
224+
className={
225+
classNames(
226+
styles,
227+
'flex-gap',
228+
'spectrum-ActionGroup',
229+
{
230+
'spectrum-ActionGroup--quiet': isQuiet,
231+
'spectrum-ActionGroup--vertical': isVertical,
232+
'spectrum-ActionGroup--compact': density === 'compact',
233+
'spectrum-ActionGroup--justified': isJustified && !isMeasuring,
234+
'spectrum-ActionGroup--overflowCollapse': overflowMode === 'collapse'
235+
},
236+
otherProps.UNSAFE_className
237+
)
238+
}>
239+
<Provider {...providerProps}>
240+
{children.map((item) => (
241+
<ActionGroupItem
242+
key={item.key}
243+
onAction={onAction}
244+
isDisabled={isDisabled}
245+
isEmphasized={isEmphasized}
246+
staticColor={staticColor}
247+
item={item}
248+
state={state}
249+
hideButtonText={hideButtonText}
250+
orientation={orientation} />
251+
))}
252+
{menuItem}
253+
</Provider>
254+
</div>
252255
</div>
253-
</div>
256+
</FocusScope>
254257
);
255258
}
256259

0 commit comments

Comments
 (0)