From a10931455a547d2bc91e84d1203396b119e0f913 Mon Sep 17 00:00:00 2001 From: Anton Korzunov Date: Sat, 24 Aug 2024 22:15:07 +1000 Subject: [PATCH 1/2] fix: auto-focus in case of activeElement disappeareance, fixes #321 --- .size-limit.json | 6 +-- _tests/keep-focus.spec.js | 92 ++++++++++++++++++++++++++++++++++++--- src/Trap.js | 30 ++++++++++++- 3 files changed, 118 insertions(+), 10 deletions(-) diff --git a/.size-limit.json b/.size-limit.json index 705b8a8..e3b6b05 100644 --- a/.size-limit.json +++ b/.size-limit.json @@ -1,7 +1,7 @@ [ { "path": "dist/cjs/UI.js", - "limit": "3.8 KB", + "limit": "3.9 KB", "ignore": [ "prop-types", "@babel/runtime", @@ -10,7 +10,7 @@ }, { "path": "dist/es2015/sidecar.js", - "limit": "4.6 KB", + "limit": "4.8 KB", "ignore": [ "prop-types", "@babel/runtime", @@ -19,7 +19,7 @@ }, { "path": "dist/es2015/index.js", - "limit": "6.9 KB", + "limit": "7.1 KB", "ignore": [ "prop-types", "@babel/runtime", diff --git a/_tests/keep-focus.spec.js b/_tests/keep-focus.spec.js index ee0a507..0b98d18 100644 --- a/_tests/keep-focus.spec.js +++ b/_tests/keep-focus.spec.js @@ -1,20 +1,102 @@ import React from 'react'; import { - render, screen, + render, } from '@testing-library/react'; import { expect } from 'chai'; import FocusLock from '../src'; +import { deferAction } from '../src/util'; -describe('Focus restoration', () => { +describe('Focus restoration', async () => { it('maintains focus on element removal', async () => { + const { rerender } = render( + + + , + ); + // + expect(document.activeElement.innerHTML).to.be.equal('btn1'); + + rerender( + + + , + ); + + // wait + await new Promise(deferAction); + + expect(document.activeElement.innerHTML).to.be.equal('new button'); + }); + + it('handles disabled elements', async () => { + const { rerender } = render( + + + + , + ); + // + expect(document.activeElement.innerHTML).to.be.equal('btn1'); + + + // https://github.com/jsdom/jsdom/issues/3029 - jsdom does trigger blur on disabled + document.activeElement.blur(); + document.body.focus(); + + rerender( + + + + , + ); + + // wait + await new Promise(deferAction); + + expect(document.activeElement.innerHTML).to.be.equal('btn2'); + }); + + it('moves focus to the nearest element', async () => { render( - + + + + + , ); // - expect(document.activeElement).to.be.equal(screen.getByRole('button')); + const middleButton = document.getElementById('middle-button'); + middleButton.focus(); + expect(document.activeElement).to.be.equal(middleButton); + + middleButton.parentElement.removeChild(middleButton); + // wait + await new Promise(deferAction); + + // btn4 "replaces" bnt3 in visual order + expect(document.activeElement.innerHTML).to.be.equal('btn4'); }); - it.todo('selects closes element to restore focus'); + it('moves focus to the nearest element before', async () => { + render( + + + + + , + ); + // + const middleButton = document.getElementById('middle-button'); + middleButton.focus(); + expect(document.activeElement).to.be.equal(middleButton); + + middleButton.parentElement.removeChild(middleButton); + // wait + await new Promise(deferAction); + + // btn2 is just before bnt3 + expect(document.activeElement.innerHTML).to.be.equal('btn2'); + }); }); diff --git a/src/Trap.js b/src/Trap.js index e6bbd3a..489a10a 100644 --- a/src/Trap.js +++ b/src/Trap.js @@ -4,7 +4,9 @@ import PropTypes from 'prop-types'; import withSideEffect from 'react-clientside-effect'; import { moveFocusInside, focusInside, - focusIsHidden, expandFocusableNodes, + focusIsHidden, + expandFocusableNodes, + getFocusableNodes, focusNextElement, focusPrevElement, focusFirstElement, @@ -22,6 +24,7 @@ const isFreeFocus = () => focusOnBody() || focusIsHidden(); let lastActiveTrap = null; let lastActiveFocus = null; +let tryRestoreFocus = () => null; let lastPortaledElement = null; @@ -86,6 +89,10 @@ const withinHost = (activeElement, workingArea) => ( workingArea.some(area => checkInHost(activeElement, area, area)) ); +const isNotFocusable = node => ( + !getFocusableNodes([node.parentNode], new Map()).some(el => el.node === node) +); + const activateTrap = () => { let result = false; if (lastActiveTrap) { @@ -93,6 +100,24 @@ const activateTrap = () => { observed, persistentFocus, autoFocus, shards, crossFrame, focusOptions, } = lastActiveTrap; const workingNode = observed || (lastPortaledElement && lastPortaledElement.portaledElement); + + // check if lastActiveFocus is still reachable + if (focusOnBody() && lastActiveFocus) { + if ( + // it was removed + !document.body.contains(lastActiveFocus) + // or not focusable (this is expensive operation)! + || isNotFocusable(lastActiveFocus) + ) { + lastActiveFocus = null; + + const newTarget = tryRestoreFocus(); + if (newTarget) { + newTarget.focus(); + } + } + } + const activeElement = document && document.activeElement; if (workingNode) { const workingArea = [ @@ -130,11 +155,12 @@ const activateTrap = () => { } focusWasOutsideWindow = false; lastActiveFocus = document && document.activeElement; + tryRestoreFocus = captureFocusRestore(lastActiveFocus); } } if (document - // element was changed + // element was changed by moveFocusInside && activeElement !== document.activeElement // fast check for any auto-guard && document.querySelector('[data-focus-auto-guard]')) { From 72b4d4be6f0a7b2be8fb64027a6e6cc0871da165 Mon Sep 17 00:00:00 2001 From: Anton Korzunov Date: Sun, 25 Aug 2024 10:09:19 +1000 Subject: [PATCH 2/2] fix: better manage cross-frame relations --- .size-limit.json | 4 ++-- src/Lock.js | 1 + src/Trap.js | 29 ++++++++++++++++++++++++++--- stories/Iframe.js | 16 +++++++++++++++- 4 files changed, 44 insertions(+), 6 deletions(-) diff --git a/.size-limit.json b/.size-limit.json index e3b6b05..2307021 100644 --- a/.size-limit.json +++ b/.size-limit.json @@ -10,7 +10,7 @@ }, { "path": "dist/es2015/sidecar.js", - "limit": "4.8 KB", + "limit": "4.85 KB", "ignore": [ "prop-types", "@babel/runtime", @@ -19,7 +19,7 @@ }, { "path": "dist/es2015/index.js", - "limit": "7.1 KB", + "limit": "7.2 KB", "ignore": [ "prop-types", "@babel/runtime", diff --git a/src/Lock.js b/src/Lock.js index e513b24..46a7d09 100644 --- a/src/Lock.js +++ b/src/Lock.js @@ -173,6 +173,7 @@ const FocusLock = React.forwardRef(function FocusLockUI(props, parentRef) { onDeactivation={onDeactivation} returnFocus={returnFocus} focusOptions={focusOptions} + noFocusGuards={noFocusGuards} /> )} null; let lastPortaledElement = null; let focusWasOutsideWindow = false; +let windowFocused = false; const defaultWhitelist = () => true; @@ -89,15 +90,16 @@ const withinHost = (activeElement, workingArea) => ( workingArea.some(area => checkInHost(activeElement, area, area)) ); +const getNodeFocusables = nodes => getFocusableNodes(nodes, new Map()); const isNotFocusable = node => ( - !getFocusableNodes([node.parentNode], new Map()).some(el => el.node === node) + !getNodeFocusables([node.parentNode]).some(el => el.node === node) ); const activateTrap = () => { let result = false; if (lastActiveTrap) { const { - observed, persistentFocus, autoFocus, shards, crossFrame, focusOptions, + observed, persistentFocus, autoFocus, shards, crossFrame, focusOptions, noFocusGuards, } = lastActiveTrap; const workingNode = observed || (lastPortaledElement && lastPortaledElement.portaledElement); @@ -125,9 +127,24 @@ const activateTrap = () => { ...shards.map(extractRef).filter(Boolean), ]; + const shouldForceRestoreFocus = () => { + // force restoration happens when + // - focus is not inside now + // - focusWasOutside + // - there are go guards + // - the last active element was the first or the last focusable one + if (!focusWasOutside(crossFrame) || !noFocusGuards || !lastActiveFocus || windowFocused) { + return false; + } + const nodes = getNodeFocusables(workingArea); + const lastIndex = nodes.findIndex(({ node }) => node === lastActiveFocus); + + return lastIndex === 0 || lastIndex === nodes.length - 1; + }; + if (!activeElement || focusWhitelisted(activeElement)) { if ( - (persistentFocus || focusWasOutside(crossFrame)) + (persistentFocus || shouldForceRestoreFocus()) || !isFreeFocus() || (!lastActiveFocus && autoFocus) ) { @@ -215,7 +232,11 @@ FocusTrap.propTypes = { children: PropTypes.node.isRequired, }; +const onWindowFocus = () => { + windowFocused = true; +}; const onWindowBlur = () => { + windowFocused = false; focusWasOutsideWindow = 'just'; // using setTimeout to set this variable after React/sidecar reaction deferAction(() => { @@ -226,12 +247,14 @@ const onWindowBlur = () => { const attachHandler = () => { document.addEventListener('focusin', onTrap); document.addEventListener('focusout', onBlur); + window.addEventListener('focus', onWindowFocus); window.addEventListener('blur', onWindowBlur); }; const detachHandler = () => { document.removeEventListener('focusin', onTrap); document.removeEventListener('focusout', onBlur); + window.removeEventListener('focus', onWindowFocus); window.removeEventListener('blur', onWindowBlur); }; diff --git a/stories/Iframe.js b/stories/Iframe.js index 93bf29d..2b6d8c5 100644 --- a/stories/Iframe.js +++ b/stories/Iframe.js @@ -74,9 +74,23 @@ export const IFrame = props => ( {' '} outside +
+ +
+ -