From 19765dd97661ac2469f37c1c0535607f9f6a97be Mon Sep 17 00:00:00 2001 From: John Coburn Date: Mon, 30 Sep 2024 09:28:57 -0500 Subject: [PATCH] STCOM-1335 add `StripesOverlayContext` to Modal, MCL... implement in Popper (#2351) * add StripesOverlayContext to Modal, MCL... implement in Popper * move useOverlayContainer to hooks dir * modify getHookExecutionResults, add tests for useOverlayContainer * don't replace getHookExecutionResults * include tests for null initial results and for refresh() on useOverlayContainer * remove only * just a little more coverage... * STCOM-1335 * fix falsy initial result test * log changes * use isEqual for default HookExecResult comparator * close MCL wrapping div for shortcut keys * rollback, re-wrap in overlayContext, have non-findDOMNode cake and eat it. --- CHANGELOG.md | 1 + hooks/tests/useOverlayContainer-test.js | 140 ++++++++++++++++ hooks/useOverlayContainer/index.js | 1 + .../useOverlayContainer.js | 6 +- lib/Modal/Modal.js | 2 +- lib/Modal/WrappingElement.js | 7 +- lib/MultiColumnList/MCLRenderer.js | 151 +++++++++--------- lib/Popper/Popper.js | 24 ++- tests/helpers/getHookExecutionResult.js | 63 +++++++- util/StripesOverlayContext.js | 4 + 10 files changed, 316 insertions(+), 83 deletions(-) create mode 100644 hooks/tests/useOverlayContainer-test.js create mode 100644 hooks/useOverlayContainer/index.js rename {lib/Modal => hooks/useOverlayContainer}/useOverlayContainer.js (98%) create mode 100644 util/StripesOverlayContext.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 73454122b..629205132 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,7 @@ * Apply `inert` attribute to header and siblings of `div#OverlayContainer` when modals are open. Refs STCOM-1334. * Expand focus trapping of modal to the `div#OverlayContainer` so that overlay components can function within `` using the `usePortal` prop. Refs STCOM-1334. * Render string for `FilterGroups` clear button. Refs STCOM-1337. +* Add OverlayContext for Overlay-style components rendered within Modals and MCL's. Refs STCOM-1335. * Refactored away from `findDOMNode` in codebase for React 19 preparation. Refs STCOM-1343. * AdvancedSearch - added a wrapping div to ref for a HotKeys ref. Refs STCOM-1343. * `` components `` and `` updated to use refs vs `findDOMNode`. Refs STCOM-1343. diff --git a/hooks/tests/useOverlayContainer-test.js b/hooks/tests/useOverlayContainer-test.js new file mode 100644 index 000000000..5ce0ba67d --- /dev/null +++ b/hooks/tests/useOverlayContainer-test.js @@ -0,0 +1,140 @@ +import { + describe, + beforeEach, + it, +} from 'mocha'; +import { converge } from '@folio/stripes-testing'; +import { getHookExecutionHarness } from '../../tests/helpers/getHookExecutionResult'; +import useOverlayContainer from '../useOverlayContainer'; + +import { OVERLAY_CONTAINER_ID } from '../../util/consts'; + +const Harness = ({ children }) => ( +
+
+ {children} +
+); + +const HarnessWithout = ({ children }) => ( +
+ {children} +
+); + +// We need to compare the `element` field of the result to discern difference on the re-render. +const areDomNodesEqual = (current, candidate) => { + return current?.element === candidate?.element; +} + +describe('useOverlayContainer', () => { + const overlayElement = () => document.getElementById(OVERLAY_CONTAINER_ID); + let res; + beforeEach(() => { + res = null; + }); + + describe('withoutOverlay fallback to a child of root.', () => { + beforeEach(async () => { + await getHookExecutionHarness( + useOverlayContainer, + [overlayElement()], + HarnessWithout, + (result) => { res = result.element }, + areDomNodesEqual + ); + }); + + it('should fallback to child of root element', + () => converge(() => { + if (res.parentNode?.id !== 'root') throw new Error(`expected child of root: ${res.id}`); + })); + }); + + describe('successfully resolving overlay container if it exists...', () => { + beforeEach(async () => { + await getHookExecutionHarness( + useOverlayContainer, + [overlayElement()], + Harness, + (result) => { res = result.element }, + areDomNodesEqual + ); + }); + + it('div#OverlayContainer in place, it should return it', + () => converge(() => { + if (res.id !== OVERLAY_CONTAINER_ID) throw new Error(`expected element to fallback to body: ${res.id}`); + })); + }); + + describe('successfully initial null result...', () => { + beforeEach(async () => { + await getHookExecutionHarness( + useOverlayContainer, + [null], + Harness, + (result) => { res = result.element }, + areDomNodesEqual + ); + }); + + it('div#OverlayContainer in place, it should return it', + () => converge(() => { + if (res.id !== OVERLAY_CONTAINER_ID) throw new Error(`expected element to fallback to body: ${res.id}`); + })); + }); + + describe('successfully initial result...', () => { + beforeEach(async () => { + await getHookExecutionHarness( + useOverlayContainer, + ['true'], + Harness, + (result) => { res = result.element }, + areDomNodesEqual + ); + }); + + it('div#OverlayContainer in place, it should return it', + () => converge(() => { + if (res !== 'true') throw new Error('should just return truthy value'); + })); + }); + + describe('falsy initial result...', () => { + beforeEach(async () => { + res = await getHookExecutionHarness( + useOverlayContainer, + [false], + Harness, + (result) => { res = result.element }, + areDomNodesEqual + ); + res.refresh(); + }); + + it('div#OverlayContainer in place, it should return it', + () => converge(() => { + if (res.element !== false) throw new Error('should just return false value'); + })); + }); + + describe('refresh', () => { + beforeEach(async () => { + res = await getHookExecutionHarness( + useOverlayContainer, + [null], + Harness, + (result) => { res = result }, + areDomNodesEqual + ); + res.refresh(); + }); + + it('div#OverlayContainer in place, it should return it', + () => converge(() => { + if (res.element.id !== OVERLAY_CONTAINER_ID) throw new Error(`expected element to fallback to body: ${res.id}`); + })); + }); +}); diff --git a/hooks/useOverlayContainer/index.js b/hooks/useOverlayContainer/index.js new file mode 100644 index 000000000..8ae263333 --- /dev/null +++ b/hooks/useOverlayContainer/index.js @@ -0,0 +1 @@ +export { default } from './useOverlayContainer'; diff --git a/lib/Modal/useOverlayContainer.js b/hooks/useOverlayContainer/useOverlayContainer.js similarity index 98% rename from lib/Modal/useOverlayContainer.js rename to hooks/useOverlayContainer/useOverlayContainer.js index 7ef099aa1..90d77148a 100644 --- a/lib/Modal/useOverlayContainer.js +++ b/hooks/useOverlayContainer/useOverlayContainer.js @@ -19,7 +19,7 @@ const resolveElement = (ref) => { return el; } return ref; -} +}; export default (ref) => { const [element, setElement] = useState(resolveElement(ref)); @@ -39,10 +39,10 @@ export default (ref) => { const el = resolveElement(ref); if (!el) setElement(el); } - } + }; return { element, refresh - } + }; }; diff --git a/lib/Modal/Modal.js b/lib/Modal/Modal.js index 8bd7f7b49..789017941 100644 --- a/lib/Modal/Modal.js +++ b/lib/Modal/Modal.js @@ -6,7 +6,7 @@ import PropTypes from 'prop-types'; import { Transition, TransitionGroup } from 'react-transition-group'; import { uniqueId, noop } from 'lodash'; import { getFirstFocusable } from '../../util/getFocusableElements'; -import useOverlayContainer from './useOverlayContainer'; +import useOverlayContainer from '../../hooks/useOverlayContainer'; import WrappingElement from './WrappingElement'; import { OVERLAY_CONTAINER_ID } from '../../util/consts'; import IconButton from '../IconButton'; diff --git a/lib/Modal/WrappingElement.js b/lib/Modal/WrappingElement.js index 409431c2f..2eadd29ec 100644 --- a/lib/Modal/WrappingElement.js +++ b/lib/Modal/WrappingElement.js @@ -8,6 +8,7 @@ import { import PropTypes from 'prop-types'; import * as focusTrap from 'focus-trap'; import { listen } from '../../util/listen'; +import StripesOverlayContext from '../../util/StripesOverlayContext'; import { OVERLAY_CONTAINER_SELECTOR } from '../../util/consts'; import calloutCSS from '../Callout/Callout.css'; import overlayCSS from '../Popper/Popper.css'; @@ -154,9 +155,11 @@ const WrappingElement = forwardRef(({ ref={wrappingElementRef} {...rest} > - {children} + + {children} + - ) + ); }); WrappingElement.propTypes = { diff --git a/lib/MultiColumnList/MCLRenderer.js b/lib/MultiColumnList/MCLRenderer.js index f6b233e12..0b6c09b35 100644 --- a/lib/MultiColumnList/MCLRenderer.js +++ b/lib/MultiColumnList/MCLRenderer.js @@ -18,6 +18,7 @@ import memoizeOne from 'memoize-one'; import Icon from '../Icon'; import EmptyMessage from '../EmptyMessage'; +import StripesOverlayContext from '../../util/StripesOverlayContext'; import { HotKeys } from '../HotKeys'; import SRStatus from '../SRStatus'; import css from './MCLRenderer.css'; @@ -1930,88 +1931,90 @@ class MCLRenderer extends React.Component { : css.mclRowContainer; return ( - -
- -
-
-
- {renderedHeaders} -
-
+ + +
+
+
+
+ {renderedHeaders} +
+
- {renderedRows} - {dndProvided.placeholder} +
+ {renderedRows} + {dndProvided.placeholder} +
+ { + loading && +
+
+ +
+
+ }
{ - loading && -
-
- -
-
- } + pagingType === pagingTypes.PREV_NEXT && ( + + ) + }
- { - pagingType === pagingTypes.PREV_NEXT && ( - - ) - } -
- + + ); } } diff --git a/lib/Popper/Popper.js b/lib/Popper/Popper.js index 1a87c88df..66b1d5f78 100644 --- a/lib/Popper/Popper.js +++ b/lib/Popper/Popper.js @@ -1,8 +1,11 @@ -import React from 'react'; +import React, { useContext, forwardRef } from 'react'; import ReactDOM from 'react-dom'; import PropTypes from 'prop-types'; import PopperJS from 'popper.js'; import css from './Popper.css'; +import { OVERLAY_CONTAINER_ID } from '../../util/consts'; +import useOverlayContainer from '../../hooks/useOverlayContainer'; +import StripesOverlayContext from '../../util/StripesOverlayContext'; export const OVERLAY_MODIFIERS = { flip: { boundariesElement: 'viewport', padding: 5 }, @@ -29,7 +32,22 @@ export const AVAILABLE_PLACEMENTS = [ const [DEFAULT_PLACEMENT] = AVAILABLE_PLACEMENTS; -export default class Popper extends React.Component { +function getDisplayName(WrappedComponent) { + return WrappedComponent.displayName || WrappedComponent.name || 'Component'; +} + +export function withOverlayContext(WrappedComponent) { + const WithOverlayContext = forwardRef(({ portal, ...props }, ref) => { // eslint-disable-line react/prop-types + const { usePortal } = useContext(StripesOverlayContext); + const portalRef = useOverlayContainer(document.getElementById(OVERLAY_CONTAINER_ID)); + return ; + }); + WithOverlayContext.displayName = `WithOverlayContext(${getDisplayName(WrappedComponent)})`; + + return WithOverlayContext; +} + +class Popper extends React.Component { static propTypes = { anchorRef: PropTypes.oneOfType([ PropTypes.func, @@ -161,3 +179,5 @@ export default class Popper extends React.Component { return null; } } + +export default withOverlayContext(Popper); diff --git a/tests/helpers/getHookExecutionResult.js b/tests/helpers/getHookExecutionResult.js index e486e794f..076b1e21f 100644 --- a/tests/helpers/getHookExecutionResult.js +++ b/tests/helpers/getHookExecutionResult.js @@ -1,4 +1,6 @@ -import React from 'react'; +import React, { useEffect, useState, Fragment } from 'react'; +import isEqual from 'lodash/isEqual'; +import noop from 'lodash/noop'; import { mountWithContext } from '../helpers'; /** @@ -11,6 +13,7 @@ import { mountWithContext } from '../helpers'; * returns and we need to wait for the component that calls the hook to mount and * render. */ + const getHookExecutionResult = (hook, hookArguments = []) => { let result = {}; const TestComponent = () => { @@ -32,4 +35,62 @@ const getHookExecutionResult = (hook, hookArguments = []) => { return mountWithContext().then(() => result); }; +const useHookExecutionResult = (hook, hookParams, comparator = isEqual) => { + const [result, updateResult] = useState(hook(...hookParams)); + + const candidate = hook(...hookParams); + if (!comparator(result, candidate)) { + updateResult(candidate); + } + return result; +}; + + +/** + * getHookExecutionHarness + * Test a hook by returning its result in a promise. + * + * @param {*} hook the hook to test + * @param {*} hookArguments arguments to pass to the hook + * @param {*} Wrapper React component/element to wrap rendered test component in. + * @param {*} checkEffect Function that will be called each update to result. + * @param {*} comparator Function to compare two values - a previous value and a potential candidate - + * for potentially re-rendering the text component/updating state. + * @returns hook's result, wrapped in a Promise, because that's what mountWithContext + * returns and we need to wait for the component that calls the hook to mount and + * render. + */ +export const getHookExecutionHarness = ( + hook, + hookArguments = [], + Wrapper = Fragment, + checkEffect = noop, + comparator = isEqual +) => { + let result = {}; + const TestComponent = () => { + const innerResult = useHookExecutionResult( + hook, + Array.isArray(hookArguments) + ? hookArguments + : [hookArguments], + comparator + ); + + useEffect(() => { + checkEffect(innerResult); + }, [innerResult]); + + result = innerResult; + + return <>; + }; + + return mountWithContext( + + + + ).then(() => result); +}; + export default getHookExecutionResult; diff --git a/util/StripesOverlayContext.js b/util/StripesOverlayContext.js new file mode 100644 index 000000000..19226472a --- /dev/null +++ b/util/StripesOverlayContext.js @@ -0,0 +1,4 @@ +import { createContext } from 'react'; + +const PortalContext = createContext({ usePortal: false }); +export default PortalContext;