Skip to content

Commit

Permalink
Use owner document in FocusScope (#5449)
Browse files Browse the repository at this point in the history
* Use owner document in FocusScope

* Fix eslint rule for @react-aria/focus

* Ignore stories for eslint

* Fix eslint rule

* Address feedback

* Add tests for focusSafely

* Address feedback

* Update packages/@react-aria/focus/test/focusSafely.test.js

Co-authored-by: Kyle Taborski <[email protected]>

* Update packages/@react-aria/focus/test/FocusScopeOwnerDocument.test.js

Co-authored-by: Kyle Taborski <[email protected]>

---------

Co-authored-by: Robert Snow <[email protected]>
Co-authored-by: Daniel Lu <[email protected]>
Co-authored-by: Kyle Taborski <[email protected]>
  • Loading branch information
4 people authored Dec 14, 2023
1 parent e105888 commit a8fc9ee
Show file tree
Hide file tree
Showing 7 changed files with 487 additions and 38 deletions.
18 changes: 17 additions & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,23 @@ module.exports = {
'jsdoc/require-description': OFF
}
}, {
files: ['packages/@react-aria/interactions/**/*.ts', 'packages/@react-aria/interactions/**/*.tsx'],
files: ['packages/@react-aria/focus/src/**/*.ts', 'packages/@react-aria/focus/src/**/*.tsx'],
rules: {
'no-restricted-globals': [
ERROR,
{
'name': 'window',
'message': 'Use getOwnerWindow from @react-aria/utils instead.'
},
{
'name': 'document',
'message': 'Use getOwnerDocument from @react-aria/utils instead.'
}
]
}
},
{
files: ['packages/@react-aria/interactions/src/**/*.ts', 'packages/@react-aria/interactions/src/**/*.tsx'],
rules: {
'no-restricted-globals': [
WARN,
Expand Down
72 changes: 40 additions & 32 deletions packages/@react-aria/focus/src/FocusScope.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,9 @@

import {FocusableElement} from '@react-types/shared';
import {focusSafely} from './focusSafely';
import {getOwnerDocument, useLayoutEffect} from '@react-aria/utils';
import {isElementVisible} from './isElementVisible';
import React, {ReactNode, RefObject, useContext, useEffect, useMemo, useRef} from 'react';
import {useLayoutEffect} from '@react-aria/utils';


export interface FocusScopeProps {
/** The contents of the focus scope. */
Expand Down Expand Up @@ -134,7 +133,7 @@ export function FocusScope(props: FocusScopeProps) {
// This needs to be an effect so that activeScope is updated after the FocusScope tree is complete.
// It cannot be a useLayoutEffect because the parent of this node hasn't been attached in the tree yet.
useEffect(() => {
let activeElement = document.activeElement;
const activeElement = getOwnerDocument(scopeRef.current ? scopeRef.current[0] : undefined).activeElement;
let scope: TreeNode | null = null;

if (isElementInScope(activeElement, scopeRef.current)) {
Expand Down Expand Up @@ -198,7 +197,7 @@ function createFocusManagerForScope(scopeRef: React.RefObject<Element[]>): Focus
focusNext(opts: FocusManagerOptions = {}) {
let scope = scopeRef.current!;
let {from, tabbable, wrap, accept} = opts;
let node = from || document.activeElement!;
let node = from || getOwnerDocument(scope[0]).activeElement!;
let sentinel = scope[0].previousElementSibling!;
let scopeRoot = getScopeRoot(scope);
let walker = getFocusableTreeWalker(scopeRoot, {tabbable, accept}, scope);
Expand All @@ -216,7 +215,7 @@ function createFocusManagerForScope(scopeRef: React.RefObject<Element[]>): Focus
focusPrevious(opts: FocusManagerOptions = {}) {
let scope = scopeRef.current!;
let {from, tabbable, wrap, accept} = opts;
let node = from || document.activeElement!;
let node = from || getOwnerDocument(scope[0]).activeElement!;
let sentinel = scope[scope.length - 1].nextElementSibling!;
let scopeRoot = getScopeRoot(scope);
let walker = getFocusableTreeWalker(scopeRoot, {tabbable, accept}, scope);
Expand Down Expand Up @@ -311,13 +310,15 @@ function useFocusContainment(scopeRef: RefObject<Element[]>, contain?: boolean)
return;
}

const ownerDocument = getOwnerDocument(scope ? scope[0] : undefined);

// Handle the Tab key to contain focus within the scope
let onKeyDown = (e) => {
if (e.key !== 'Tab' || e.altKey || e.ctrlKey || e.metaKey || !shouldContainFocus(scopeRef)) {
return;
}

let focusedElement = document.activeElement;
let focusedElement = ownerDocument.activeElement;
let scope = scopeRef.current;
if (!scope || !isElementInScope(focusedElement, scope)) {
return;
Expand Down Expand Up @@ -367,9 +368,9 @@ function useFocusContainment(scopeRef: RefObject<Element[]>, contain?: boolean)
}
raf.current = requestAnimationFrame(() => {
// Use document.activeElement instead of e.relatedTarget so we can tell if user clicked into iframe
if (document.activeElement && shouldContainFocus(scopeRef) && !isElementInChildScope(document.activeElement, scopeRef)) {
if (ownerDocument.activeElement && shouldContainFocus(scopeRef) && !isElementInChildScope(ownerDocument.activeElement, scopeRef)) {
activeScope = scopeRef;
if (document.body.contains(e.target)) {
if (ownerDocument.body.contains(e.target)) {
focusedNode.current = e.target;
focusedNode.current?.focus();
} else if (activeScope.current) {
Expand All @@ -379,13 +380,13 @@ function useFocusContainment(scopeRef: RefObject<Element[]>, contain?: boolean)
});
};

document.addEventListener('keydown', onKeyDown, false);
document.addEventListener('focusin', onFocus, false);
ownerDocument.addEventListener('keydown', onKeyDown, false);
ownerDocument.addEventListener('focusin', onFocus, false);
scope?.forEach(element => element.addEventListener('focusin', onFocus, false));
scope?.forEach(element => element.addEventListener('focusout', onBlur, false));
return () => {
document.removeEventListener('keydown', onKeyDown, false);
document.removeEventListener('focusin', onFocus, false);
ownerDocument.removeEventListener('keydown', onKeyDown, false);
ownerDocument.removeEventListener('focusin', onFocus, false);
scope?.forEach(element => element.removeEventListener('focusin', onFocus, false));
scope?.forEach(element => element.removeEventListener('focusout', onBlur, false));
};
Expand Down Expand Up @@ -488,7 +489,8 @@ function useAutoFocus(scopeRef: RefObject<Element[]>, autoFocus?: boolean) {
useEffect(() => {
if (autoFocusRef.current) {
activeScope = scopeRef;
if (!isElementInScope(document.activeElement, activeScope.current) && scopeRef.current) {
const ownerDocument = getOwnerDocument(scopeRef.current ? scopeRef.current[0] : undefined);
if (!isElementInScope(ownerDocument.activeElement, activeScope.current) && scopeRef.current) {
focusFirstInScope(scopeRef.current);
}
}
Expand All @@ -505,6 +507,7 @@ function useActiveScopeTracker(scopeRef: RefObject<Element[]>, restore?: boolean
}

let scope = scopeRef.current;
const ownerDocument = getOwnerDocument(scope ? scope[0] : undefined);

let onFocus = (e) => {
let target = e.target as Element;
Expand All @@ -515,10 +518,10 @@ function useActiveScopeTracker(scopeRef: RefObject<Element[]>, restore?: boolean
}
};

document.addEventListener('focusin', onFocus, false);
ownerDocument.addEventListener('focusin', onFocus, false);
scope?.forEach(element => element.addEventListener('focusin', onFocus, false));
return () => {
document.removeEventListener('focusin', onFocus, false);
ownerDocument.removeEventListener('focusin', onFocus, false);
scope?.forEach(element => element.removeEventListener('focusin', onFocus, false));
};
}, [scopeRef, restore, contain]);
Expand All @@ -539,12 +542,14 @@ function shouldRestoreFocus(scopeRef: ScopeRef) {

function useRestoreFocus(scopeRef: RefObject<Element[]>, restoreFocus?: boolean, contain?: boolean) {
// create a ref during render instead of useLayoutEffect so the active element is saved before a child with autoFocus=true mounts.
const nodeToRestoreRef = useRef(typeof document !== 'undefined' ? document.activeElement as FocusableElement : null);
// eslint-disable-next-line no-restricted-globals
const nodeToRestoreRef = useRef(typeof document !== 'undefined' ? getOwnerDocument(scopeRef.current ? scopeRef.current[0] : undefined).activeElement as FocusableElement : null);

// restoring scopes should all track if they are active regardless of contain, but contain already tracks it plus logic to contain the focus
// restoring-non-containing scopes should only care if they become active so they can perform the restore
useLayoutEffect(() => {
let scope = scopeRef.current;
const ownerDocument = getOwnerDocument(scope ? scope[0] : undefined);
if (!restoreFocus || contain) {
return;
}
Expand All @@ -553,22 +558,24 @@ function useRestoreFocus(scopeRef: RefObject<Element[]>, restoreFocus?: boolean,
// If focusing an element in a child scope of the currently active scope, the child becomes active.
// Moving out of the active scope to an ancestor is not allowed.
if ((!activeScope || isAncestorScope(activeScope, scopeRef)) &&
isElementInScope(document.activeElement, scopeRef.current)
isElementInScope(ownerDocument.activeElement, scopeRef.current)
) {
activeScope = scopeRef;
}
};

document.addEventListener('focusin', onFocus, false);
ownerDocument.addEventListener('focusin', onFocus, false);
scope?.forEach(element => element.addEventListener('focusin', onFocus, false));
return () => {
document.removeEventListener('focusin', onFocus, false);
ownerDocument.removeEventListener('focusin', onFocus, false);
scope?.forEach(element => element.removeEventListener('focusin', onFocus, false));
};
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [scopeRef, contain]);

useLayoutEffect(() => {
const ownerDocument = getOwnerDocument(scopeRef.current ? scopeRef.current[0] : undefined);

if (!restoreFocus) {
return;
}
Expand All @@ -582,7 +589,7 @@ function useRestoreFocus(scopeRef: RefObject<Element[]>, restoreFocus?: boolean,
return;
}

let focusedElement = document.activeElement as FocusableElement;
let focusedElement = ownerDocument.activeElement as FocusableElement;
if (!isElementInScope(focusedElement, scopeRef.current)) {
return;
}
Expand All @@ -593,13 +600,13 @@ function useRestoreFocus(scopeRef: RefObject<Element[]>, restoreFocus?: boolean,
let nodeToRestore = treeNode.nodeToRestore;

// Create a DOM tree walker that matches all tabbable elements
let walker = getFocusableTreeWalker(document.body, {tabbable: true});
let walker = getFocusableTreeWalker(ownerDocument.body, {tabbable: true});

// Find the next tabbable element after the currently focused element
walker.currentNode = focusedElement;
let nextElement = (e.shiftKey ? walker.previousNode() : walker.nextNode()) as FocusableElement;

if (!nodeToRestore || !document.body.contains(nodeToRestore) || nodeToRestore === document.body) {
if (!nodeToRestore || !ownerDocument.body.contains(nodeToRestore) || nodeToRestore === ownerDocument.body) {
nodeToRestore = undefined;
treeNode.nodeToRestore = undefined;
}
Expand Down Expand Up @@ -632,18 +639,20 @@ function useRestoreFocus(scopeRef: RefObject<Element[]>, restoreFocus?: boolean,
};

if (!contain) {
document.addEventListener('keydown', onKeyDown, true);
ownerDocument.addEventListener('keydown', onKeyDown, true);
}

return () => {
if (!contain) {
document.removeEventListener('keydown', onKeyDown, true);
ownerDocument.removeEventListener('keydown', onKeyDown, true);
}
};
}, [scopeRef, restoreFocus, contain]);

// useLayoutEffect instead of useEffect so the active element is saved synchronously instead of asynchronously.
useLayoutEffect(() => {
const ownerDocument = getOwnerDocument(scopeRef.current ? scopeRef.current[0] : undefined);

if (!restoreFocus) {
return;
}
Expand All @@ -653,7 +662,6 @@ function useRestoreFocus(scopeRef: RefObject<Element[]>, restoreFocus?: boolean,
return;
}
treeNode.nodeToRestore = nodeToRestoreRef.current ?? undefined;

return () => {
let treeNode = focusScopeTree.getTreeNode(scopeRef);
if (!treeNode) {
Expand All @@ -667,19 +675,19 @@ function useRestoreFocus(scopeRef: RefObject<Element[]>, restoreFocus?: boolean,
&& nodeToRestore
&& (
// eslint-disable-next-line react-hooks/exhaustive-deps
isElementInScope(document.activeElement, scopeRef.current)
|| (document.activeElement === document.body && shouldRestoreFocus(scopeRef))
isElementInScope(ownerDocument.activeElement, scopeRef.current)
|| (ownerDocument.activeElement === ownerDocument.body && shouldRestoreFocus(scopeRef))
)
) {
// freeze the focusScopeTree so it persists after the raf, otherwise during unmount nodes are removed from it
let clonedTree = focusScopeTree.clone();
requestAnimationFrame(() => {
// Only restore focus if we've lost focus to the body, the alternative is that focus has been purposefully moved elsewhere
if (document.activeElement === document.body) {
if (ownerDocument.activeElement === ownerDocument.body) {
// look up the tree starting with our scope to find a nodeToRestore still in the DOM
let treeNode = clonedTree.getTreeNode(scopeRef);
while (treeNode) {
if (treeNode.nodeToRestore && document.body.contains(treeNode.nodeToRestore)) {
if (treeNode.nodeToRestore && treeNode.nodeToRestore.isConnected) {
focusElement(treeNode.nodeToRestore);
return;
}
Expand Down Expand Up @@ -709,7 +717,7 @@ function useRestoreFocus(scopeRef: RefObject<Element[]>, restoreFocus?: boolean,
*/
export function getFocusableTreeWalker(root: Element, opts?: FocusManagerOptions, scope?: Element[]) {
let selector = opts?.tabbable ? TABBABLE_ELEMENT_SELECTOR : FOCUSABLE_ELEMENT_SELECTOR;
let walker = document.createTreeWalker(
let walker = getOwnerDocument(root).createTreeWalker(
root,
NodeFilter.SHOW_ELEMENT,
{
Expand Down Expand Up @@ -750,7 +758,7 @@ export function createFocusManager(ref: RefObject<Element>, defaultOptions: Focu
return null;
}
let {from, tabbable = defaultOptions.tabbable, wrap = defaultOptions.wrap, accept = defaultOptions.accept} = opts;
let node = from || document.activeElement;
let node = from || getOwnerDocument(root).activeElement;
let walker = getFocusableTreeWalker(root, {tabbable, accept});
if (root.contains(node)) {
walker.currentNode = node!;
Expand All @@ -771,7 +779,7 @@ export function createFocusManager(ref: RefObject<Element>, defaultOptions: Focu
return null;
}
let {from, tabbable = defaultOptions.tabbable, wrap = defaultOptions.wrap, accept = defaultOptions.accept} = opts;
let node = from || document.activeElement;
let node = from || getOwnerDocument(root).activeElement;
let walker = getFocusableTreeWalker(root, {tabbable, accept});
if (root.contains(node)) {
walker.currentNode = node!;
Expand Down
7 changes: 4 additions & 3 deletions packages/@react-aria/focus/src/focusSafely.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
*/

import {FocusableElement} from '@react-types/shared';
import {focusWithoutScrolling, runAfterTransition} from '@react-aria/utils';
import {focusWithoutScrolling, getOwnerDocument, runAfterTransition} from '@react-aria/utils';
import {getInteractionModality} from '@react-aria/interactions';

/**
Expand All @@ -24,11 +24,12 @@ export function focusSafely(element: FocusableElement) {
// the page before shifting focus. This avoids issues with VoiceOver on iOS
// causing the page to scroll when moving focus if the element is transitioning
// from off the screen.
const ownerDocument = getOwnerDocument(element);
if (getInteractionModality() === 'virtual') {
let lastFocusedElement = document.activeElement;
let lastFocusedElement = ownerDocument.activeElement;
runAfterTransition(() => {
// If focus did not move and the element is still in the document, focus it.
if (document.activeElement === lastFocusedElement && document.contains(element)) {
if (ownerDocument.activeElement === lastFocusedElement && element.isConnected) {
focusWithoutScrolling(element);
}
});
Expand Down
5 changes: 4 additions & 1 deletion packages/@react-aria/focus/src/isElementVisible.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,11 @@
* governing permissions and limitations under the License.
*/

import {getOwnerWindow} from '@react-aria/utils';

function isStyleVisible(element: Element) {
if (!(element instanceof HTMLElement) && !(element instanceof SVGElement)) {
const windowObject = getOwnerWindow(element);
if (!(element instanceof windowObject.HTMLElement) && !(element instanceof windowObject.SVGElement)) {
return false;
}

Expand Down
3 changes: 2 additions & 1 deletion packages/@react-aria/focus/test/FocusScope.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,8 @@ describe('FocusScope', function () {
expect(document.activeElement).toBe(input2);
});

it('uses document.activeElement instead of e.relatedTarget on blur to determine if focus is still in scope', function () {
// This test setup is a bit contrived to just purely simulate the blur/focus events that would happen in a case like this
it('focus properly moves into child iframe on click', function () {
let {getByTestId} = render(
<div>
<FocusScope contain>
Expand Down
Loading

1 comment on commit a8fc9ee

@rspbot
Copy link

@rspbot rspbot commented on a8fc9ee Dec 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.