Skip to content

Commit 7018c13

Browse files
devongovettLFDanLu
andauthored
Fix FocusScope issues (#2223)
* Handle nested focus scopes without contain * Fix tabbing without restoreFocus * Fixes * Restore active scope to the correct scope on unmount * Fix FocusScope in portal Co-authored-by: Daniel Lu <[email protected]>
1 parent 4c95488 commit 7018c13

File tree

3 files changed

+286
-58
lines changed

3 files changed

+286
-58
lines changed

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

Lines changed: 69 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -54,10 +54,16 @@ interface FocusManager {
5454
focusPrevious(opts?: FocusManagerOptions): HTMLElement
5555
}
5656

57-
const FocusContext = React.createContext<FocusManager>(null);
57+
type ScopeRef = RefObject<HTMLElement[]>;
58+
interface IFocusContext {
59+
scopeRef: ScopeRef,
60+
focusManager: FocusManager
61+
}
62+
63+
const FocusContext = React.createContext<IFocusContext>(null);
5864

59-
let activeScope: RefObject<HTMLElement[]> = null;
60-
let scopes: Set<RefObject<HTMLElement[]>> = new Set();
65+
let activeScope: ScopeRef = null;
66+
let scopes: Map<ScopeRef, ScopeRef | null> = new Map();
6167

6268
// This is a hacky DOM-based implementation of a FocusScope until this RFC lands in React:
6369
// https://github.com/reactjs/rfcs/pull/109
@@ -76,6 +82,8 @@ export function FocusScope(props: FocusScopeProps) {
7682
let startRef = useRef<HTMLSpanElement>();
7783
let endRef = useRef<HTMLSpanElement>();
7884
let scopeRef = useRef<HTMLElement[]>([]);
85+
let ctx = useContext(FocusContext);
86+
let parentScope = ctx?.scopeRef;
7987

8088
useLayoutEffect(() => {
8189
// Find all rendered nodes between the sentinels and add them to the scope.
@@ -87,11 +95,23 @@ export function FocusScope(props: FocusScopeProps) {
8795
}
8896

8997
scopeRef.current = nodes;
90-
scopes.add(scopeRef);
98+
}, [children, parentScope]);
99+
100+
useLayoutEffect(() => {
101+
scopes.set(scopeRef, parentScope);
91102
return () => {
103+
// Restore the active scope on unmount if this scope or a descendant scope is active.
104+
// Parent effect cleanups run before children, so we need to check if the
105+
// parent scope actually still exists before restoring the active scope to it.
106+
if (
107+
(scopeRef === activeScope || isAncestorScope(scopeRef, activeScope)) &&
108+
(!parentScope || scopes.has(parentScope))
109+
) {
110+
activeScope = parentScope;
111+
}
92112
scopes.delete(scopeRef);
93113
};
94-
}, [children]);
114+
}, [scopeRef, parentScope]);
95115

96116
useFocusContainment(scopeRef, contain);
97117
useRestoreFocus(scopeRef, restoreFocus, contain);
@@ -100,7 +120,7 @@ export function FocusScope(props: FocusScopeProps) {
100120
let focusManager = createFocusManagerForScope(scopeRef);
101121

102122
return (
103-
<FocusContext.Provider value={focusManager}>
123+
<FocusContext.Provider value={{scopeRef, focusManager}}>
104124
<span data-focus-scope-start hidden ref={startRef} />
105125
{children}
106126
<span data-focus-scope-end hidden ref={endRef} />
@@ -114,7 +134,7 @@ export function FocusScope(props: FocusScopeProps) {
114134
* a FocusScope, e.g. in response to user events like keyboard navigation.
115135
*/
116136
export function useFocusManager(): FocusManager {
117-
return useContext(FocusContext);
137+
return useContext(FocusContext)?.focusManager;
118138
}
119139

120140
function createFocusManagerForScope(scopeRef: React.RefObject<HTMLElement[]>): FocusManager {
@@ -185,19 +205,20 @@ function useFocusContainment(scopeRef: RefObject<HTMLElement[]>, contain: boolea
185205
let focusedNode = useRef<HTMLElement>();
186206

187207
let raf = useRef(null);
188-
useEffect(() => {
208+
useLayoutEffect(() => {
189209
let scope = scopeRef.current;
190210
if (!contain) {
191211
return;
192212
}
193213

194214
// Handle the Tab key to contain focus within the scope
195215
let onKeyDown = (e) => {
196-
if (e.key !== 'Tab' || e.altKey || e.ctrlKey || e.metaKey) {
216+
if (e.key !== 'Tab' || e.altKey || e.ctrlKey || e.metaKey || scopeRef !== activeScope) {
197217
return;
198218
}
199219

200220
let focusedElement = document.activeElement as HTMLElement;
221+
let scope = scopeRef.current;
201222
if (!isElementInScope(focusedElement, scope)) {
202223
return;
203224
}
@@ -217,17 +238,20 @@ function useFocusContainment(scopeRef: RefObject<HTMLElement[]>, contain: boolea
217238
};
218239

219240
let onFocus = (e) => {
220-
// If a focus event occurs outside the active scope (e.g. user tabs from browser location bar),
221-
// restore focus to the previously focused node or the first tabbable element in the active scope.
222-
let isInAnyScope = isElementInAnyScope(e.target, scopes);
223-
if (!isInAnyScope) {
241+
// If focusing an element in a child scope of the currently active scope, the child becomes active.
242+
// Moving out of the active scope to an ancestor is not allowed.
243+
if (!activeScope || isAncestorScope(activeScope, scopeRef)) {
244+
activeScope = scopeRef;
245+
focusedNode.current = e.target;
246+
} else if (scopeRef === activeScope && !isElementInChildScope(e.target, scopeRef)) {
247+
// If a focus event occurs outside the active scope (e.g. user tabs from browser location bar),
248+
// restore focus to the previously focused node or the first tabbable element in the active scope.
224249
if (focusedNode.current) {
225250
focusedNode.current.focus();
226251
} else if (activeScope) {
227252
focusFirstInScope(activeScope.current);
228253
}
229-
} else {
230-
activeScope = scopeRef;
254+
} else if (scopeRef === activeScope) {
231255
focusedNode.current = e.target;
232256
}
233257
};
@@ -236,9 +260,7 @@ function useFocusContainment(scopeRef: RefObject<HTMLElement[]>, contain: boolea
236260
// Firefox doesn't shift focus back to the Dialog properly without this
237261
raf.current = requestAnimationFrame(() => {
238262
// Use document.activeElement instead of e.relatedTarget so we can tell if user clicked into iframe
239-
let isInAnyScope = isElementInAnyScope(document.activeElement, scopes);
240-
241-
if (!isInAnyScope) {
263+
if (scopeRef === activeScope && !isElementInChildScope(document.activeElement, scopeRef)) {
242264
activeScope = scopeRef;
243265
focusedNode.current = e.target;
244266
focusedNode.current.focus();
@@ -264,34 +286,42 @@ function useFocusContainment(scopeRef: RefObject<HTMLElement[]>, contain: boolea
264286
}, [raf]);
265287
}
266288

267-
function isElementInAnyScope(element: Element, scopes: Set<RefObject<HTMLElement[]>>) {
268-
for (let scope of scopes.values()) {
289+
function isElementInAnyScope(element: Element) {
290+
for (let scope of scopes.keys()) {
269291
if (isElementInScope(element, scope.current)) {
270292
return true;
271293
}
272294
}
273295
return false;
274296
}
275297

276-
const focusScopeDataAttrNames = [
277-
'data-focus-scope-start',
278-
'data-focus-scope-end'
279-
];
280-
281-
function isFocusScopeDirectChild(scope: HTMLElement) {
282-
return focusScopeDataAttrNames.some(name => scope.getAttribute(name) !== null);
298+
function isElementInScope(element: Element, scope: HTMLElement[]) {
299+
return scope.some(node => node.contains(element));
283300
}
284301

285-
function isFocusScopeNestedChild(scope: HTMLElement) {
286-
return focusScopeDataAttrNames.some(name => scope.querySelector(`[${name}]`));
287-
}
302+
function isElementInChildScope(element: Element, scope: ScopeRef) {
303+
// node.contains in isElementInScope covers child scopes that are also DOM children,
304+
// but does not cover child scopes in portals.
305+
for (let s of scopes.keys()) {
306+
if ((s === scope || isAncestorScope(scope, s)) && isElementInScope(element, s.current)) {
307+
return true;
308+
}
309+
}
288310

289-
function isFocusScopeInScope(scopes: HTMLElement[]) {
290-
return scopes.some(scope => isFocusScopeDirectChild(scope) || isFocusScopeNestedChild(scope));
311+
return false;
291312
}
292313

293-
function isElementInScope(element: Element, scope: HTMLElement[]) {
294-
return !isFocusScopeInScope(scope) && scope.some(node => node.contains(element));
314+
function isAncestorScope(ancestor: ScopeRef, scope: ScopeRef) {
315+
let parent = scopes.get(scope);
316+
if (!parent) {
317+
return false;
318+
}
319+
320+
if (parent === ancestor) {
321+
return true;
322+
}
323+
324+
return isAncestorScope(ancestor, parent);
295325
}
296326

297327
function focusElement(element: HTMLElement | null, scroll = false) {
@@ -333,6 +363,10 @@ function useAutoFocus(scopeRef: RefObject<HTMLElement[]>, autoFocus: boolean) {
333363
function useRestoreFocus(scopeRef: RefObject<HTMLElement[]>, restoreFocus: boolean, contain: boolean) {
334364
// useLayoutEffect instead of useEffect so the active element is saved synchronously instead of asynchronously.
335365
useLayoutEffect(() => {
366+
if (!restoreFocus) {
367+
return;
368+
}
369+
336370
let scope = scopeRef.current;
337371
let nodeToRestore = document.activeElement as HTMLElement;
338372

@@ -379,7 +413,7 @@ function useRestoreFocus(scopeRef: RefObject<HTMLElement[]>, restoreFocus: boole
379413
// If there is no next element and the nodeToRestore isn't within a FocusScope (i.e. we are leaving the top level focus scope)
380414
// then move focus to the body.
381415
// 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)) {
416+
if (!isElementInAnyScope(nodeToRestore)) {
383417
focusedElement.blur();
384418
} else {
385419
focusElement(nodeToRestore, true);

0 commit comments

Comments
 (0)