From c8b46c9f8bcc8cc64b48864bcb26079ac9147e44 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Tue, 15 Sep 2020 10:17:32 -0400 Subject: [PATCH] Use ref callback to initialize FullScreen modal focus trap **Why**: This may resolve an issue where focus trap's event handling is not removed after a FullScreen dialog is hidden, under the assumption that the previous implementation could leave potential for desync between modalRef and useEffect, or between the useEffect's unsubscribe behavior and trapRef.current.deactivate. --- .../components/full-screen.jsx | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/app/javascript/packages/document-capture/components/full-screen.jsx b/app/javascript/packages/document-capture/components/full-screen.jsx index 454bd45ad39..992000313fc 100644 --- a/app/javascript/packages/document-capture/components/full-screen.jsx +++ b/app/javascript/packages/document-capture/components/full-screen.jsx @@ -1,4 +1,4 @@ -import React, { useRef, useEffect } from 'react'; +import React, { useRef, useEffect, useCallback } from 'react'; import createFocusTrap from 'focus-trap'; import useI18n from '../hooks/use-i18n'; import useAsset from '../hooks/use-asset'; @@ -26,7 +26,6 @@ let activeInstances = 0; function FullScreen({ onRequestClose = () => {}, children }) { const { t } = useI18n(); const { getAssetPath } = useAsset(); - const modalRef = useRef(/** @type {?HTMLDivElement} */ (null)); const trapRef = useRef(/** @type {?import('focus-trap').FocusTrap} */ (null)); const onRequestCloseRef = useRef(onRequestClose); useEffect(() => { @@ -35,13 +34,18 @@ function FullScreen({ onRequestClose = () => {}, children }) { // to reference in the deactivation. onRequestCloseRef.current = onRequestClose; }, [onRequestClose]); - useEffect(() => { - if (modalRef.current) { - trapRef.current = createFocusTrap(modalRef.current, { + + const setFocusTrapRef = useCallback((node) => { + if (trapRef.current) { + trapRef.current.deactivate(); + } + + if (node) { + trapRef.current = createFocusTrap(node, { onDeactivate: () => onRequestCloseRef.current(), }); + trapRef.current.activate(); - return trapRef.current.deactivate; } }, []); @@ -58,7 +62,7 @@ function FullScreen({ onRequestClose = () => {}, children }) { }, []); return ( -
+