diff --git a/packages/fluentui/react-northstar/src/components/Portal/Portal.tsx b/packages/fluentui/react-northstar/src/components/Portal/Portal.tsx index 57ad9c05ebb552..ce9364f7d225d1 100644 --- a/packages/fluentui/react-northstar/src/components/Portal/Portal.tsx +++ b/packages/fluentui/react-northstar/src/components/Portal/Portal.tsx @@ -85,7 +85,7 @@ export interface PortalProps extends ChildrenComponentProps, ContentComponentPro export const Portal: React.FC & FluentComponentStaticProps = props => { const context = useFluentContext(); - const { children, content, trapFocus, trigger, triggerAccessibility } = props; + const { children, content, trapFocus, trigger, triggerAccessibility = {} } = props; const portalRef = React.useRef(); const triggerRef = React.useRef(); @@ -193,7 +193,3 @@ Portal.handledProps = Object.keys(Portal.propTypes) as any; Portal.create = createShorthandFactory({ Component: Portal, }); - -Portal.defaultProps = { - triggerAccessibility: {}, -}; diff --git a/packages/fluentui/react-northstar/src/components/Portal/PortalInner.tsx b/packages/fluentui/react-northstar/src/components/Portal/PortalInner.tsx index ccb29e3e3efb0a..a1ae76ff0d9387 100644 --- a/packages/fluentui/react-northstar/src/components/Portal/PortalInner.tsx +++ b/packages/fluentui/react-northstar/src/components/Portal/PortalInner.tsx @@ -41,7 +41,7 @@ export const PortalInner: React.FC = props => { const { target, rtl } = useFluentContext(); const registerPortalEl = usePortalCompat(); - const box = usePortalBox({ className, target, rtl }); + const box = usePortalBox({ className, targetNode: target?.body, rtl }); // PortalInner should render elements even without a context // eslint-disable-next-line const container: HTMLElement | null = isBrowser() ? mountNode || box || document.body : null; diff --git a/packages/fluentui/react-northstar/src/components/Portal/usePortalBox.ts b/packages/fluentui/react-northstar/src/components/Portal/usePortalBox.ts index c55f514bd108d5..1839bbf549e532 100644 --- a/packages/fluentui/react-northstar/src/components/Portal/usePortalBox.ts +++ b/packages/fluentui/react-northstar/src/components/Portal/usePortalBox.ts @@ -1,58 +1,146 @@ -import { useIsomorphicLayoutEffect } from '@fluentui/react-bindings'; import * as React from 'react'; -import { isBrowser } from '../../utils/isBrowser'; +const useInsertionEffect = + // eslint-disable-next-line no-useless-concat + ((React as never)['useInsertion' + 'Effect'] as typeof React.useLayoutEffect | undefined) ?? React.useLayoutEffect; -type UsePortalBoxOptions = { - className: string; +export type UsePortalBoxOptions = { + className?: string; rtl: boolean; - target: Document | undefined; + targetNode: HTMLElement | undefined; }; -export const usePortalBox = (options: UsePortalBoxOptions): HTMLDivElement | null => { - const { className, rtl, target } = options; +const initializeElementFactory = () => { + let currentElement: HTMLDivElement | undefined = undefined; - const element: HTMLDivElement | null = React.useMemo(() => { - const newElement = isBrowser() && target ? target.createElement('div') : null; + function get(targetRoot: HTMLElement, forceCreation: boolean): HTMLDivElement | undefined { + if (currentElement) { + return currentElement; + } - // Element should be attached to DOM during render to make elements that will be rendered - // inside accessible in effects of child components - if (newElement) { - target.body.appendChild(newElement); + if (forceCreation) { + currentElement = targetRoot.ownerDocument.createElement('div'); + targetRoot.appendChild(currentElement); } - return newElement; - }, [target]); + return currentElement; + } + + function dispose() { + if (currentElement) { + currentElement.remove(); + currentElement = undefined; + } + } - useIsomorphicLayoutEffect(() => { - const classes = className.split(' ').filter(Boolean); + return { + get, + dispose, + }; +}; - if (element) { - element.classList.add(...classes); +/** + * This is a modern element factory for React 18 and above. It is safe for concurrent rendering. + * + * It abuses the fact that React will mount DOM once (unlike hooks), so by using a proxy we can intercept: + * - the `remove()` method (we call it in `useEffect()`) and remove the element only when the portal is unmounted + * - all other methods (and properties) will be called by React once a portal is mounted + */ +export const usePortalBox = (options: UsePortalBoxOptions): HTMLDivElement | null => { + const { className, rtl, targetNode } = options; + const [elementFactory] = React.useState(initializeElementFactory); - if (rtl) { - element.setAttribute('dir', 'rtl'); - } else { - element.removeAttribute('dir'); - } + const elementProxy = React.useMemo(() => { + if (targetNode === undefined) { + return null; } + return new Proxy({} as HTMLDivElement, { + get(_, property: keyof HTMLDivElement) { + // Heads up! + // `createPortal()` performs a check for `nodeType` property to determine if the mount node is a valid DOM node + // before mounting the portal. We hardcode the value to `Node.ELEMENT_NODE` to pass this check and avoid + // premature node creation + if (property === 'nodeType') { + return Node.ELEMENT_NODE; + } + + // Heads up! + // We intercept the `remove()` method to remove the mount node only when portal has been unmounted already. + if (property === 'remove') { + const targetElement = elementFactory.get(targetNode, false); + + if (targetElement) { + // If the mountElement has children, the portal is still mounted, otherwise we can dispose of it + const portalHasNoChildren = targetElement.childNodes.length === 0; + + if (portalHasNoChildren) { + elementFactory.dispose(); + } + } + + return () => { + // Always return a no-op function to avoid errors in the code + }; + } + + const targetElement = elementFactory.get(targetNode, true); + const targetProperty = targetElement ? targetElement[property] : undefined; + + if (typeof targetProperty === 'function') { + return targetProperty.bind(targetElement); + } + + return targetProperty; + }, + + set(_, property: keyof HTMLDivElement | '_virtual' | 'focusVisible', value) { + const ignoredProperty = property === '_virtual' || property === 'focusVisible'; + + // We should use the `elementFactory.get(targetNode, !ignoredProperty)`, + // but TypeScript requires a literal `true` or `false` for the overload signature. + // This workaround ensures the correct overload is called and avoids TypeScript errors. + const targetElement = ignoredProperty + ? elementFactory.get(targetNode, false) + : elementFactory.get(targetNode, true); + + if (ignoredProperty && !targetElement) { + // We ignore the `_virtual` and `focusVisible` properties to avoid conflicts with the proxy + return true; + } + + if (targetElement) { + Object.assign(targetElement, { [property]: value }); + return true; + } + + return false; + }, + }); + }, [elementFactory, targetNode]); + + useInsertionEffect!(() => { + if (!elementProxy) { + return () => {}; + } + + const classesToApply = className.split(' ').filter(Boolean); + + elementProxy.classList.add(...classesToApply); + elementProxy.setAttribute('dir', rtl ? 'rtl' : 'ltr'); + elementProxy.setAttribute('data-portal-node', 'true'); + return () => { - if (element) { - element.classList.remove(...classes); - element.removeAttribute('dir'); - } + elementProxy.classList.remove(...classesToApply); + elementProxy.removeAttribute('dir'); }; - }, [className, element, rtl]); + }, [className, elementProxy, rtl]); - // This effect should always last as it removes element from HTML tree React.useEffect(() => { return () => { - if (element) { - target.body.removeChild(element); - } + elementProxy?.remove(); }; - }, [element, target]); + }, [elementProxy]); - return element; + return elementProxy; }; diff --git a/packages/fluentui/react-northstar/test/specs/components/Portal/PortalInner-test.tsx b/packages/fluentui/react-northstar/test/specs/components/Portal/PortalInner-test.tsx index 2c2b562ff317c9..210369eac3b63f 100644 --- a/packages/fluentui/react-northstar/test/specs/components/Portal/PortalInner-test.tsx +++ b/packages/fluentui/react-northstar/test/specs/components/Portal/PortalInner-test.tsx @@ -97,7 +97,7 @@ describe('PortalInner', () => { ); expect(register).toHaveBeenCalledTimes(1); - expect(register).toHaveBeenCalledWith(expect.any(HTMLElement)); + expect(register).toHaveBeenCalledWith(expect.any(Object)); wrapper.unmount(); expect(unregister).toHaveBeenCalledTimes(1);