From 55f1dbef66562b3f888b0b83529cede1cb443dc5 Mon Sep 17 00:00:00 2001 From: Oleksandr Fediashov Date: Wed, 30 Oct 2024 15:30:37 +0100 Subject: [PATCH] chore(react-portal): refactor portals creation --- .../library/etc/react-portal.api.md | 4 +- .../src/components/Portal/Portal.test.tsx | 66 +++++++++++--- .../src/components/Portal/Portal.types.ts | 20 ++-- .../Portal/__snapshots__/Portal.test.tsx.snap | 11 --- .../src/components/Portal/renderPortal.tsx | 11 ++- .../src/components/Portal/usePortal.ts | 43 +++++++-- .../Portal/usePortalMountNode.test.tsx | 37 -------- .../components/Portal/usePortalMountNode.ts | 91 ------------------- 8 files changed, 116 insertions(+), 167 deletions(-) delete mode 100644 packages/react-components/react-portal/library/src/components/Portal/__snapshots__/Portal.test.tsx.snap delete mode 100644 packages/react-components/react-portal/library/src/components/Portal/usePortalMountNode.test.tsx delete mode 100644 packages/react-components/react-portal/library/src/components/Portal/usePortalMountNode.ts diff --git a/packages/react-components/react-portal/library/etc/react-portal.api.md b/packages/react-components/react-portal/library/etc/react-portal.api.md index f0f44c50972578..16cd2d9dbcf898 100644 --- a/packages/react-components/react-portal/library/etc/react-portal.api.md +++ b/packages/react-components/react-portal/library/etc/react-portal.api.md @@ -4,9 +4,11 @@ ```ts +import type { ComponentState } from '@fluentui/react-utilities'; import { elementContains } from '@fluentui/react-utilities'; import * as React_2 from 'react'; import { setVirtualParent } from '@fluentui/react-utilities'; +import type { Slot } from '@fluentui/react-utilities'; export { elementContains } @@ -23,7 +25,7 @@ export type PortalProps = { }; // @public (undocumented) -export type PortalState = Pick & { +export type PortalState = ComponentState & Pick & { mountNode: HTMLElement | null | undefined; virtualParentRootRef: React_2.MutableRefObject; }; diff --git a/packages/react-components/react-portal/library/src/components/Portal/Portal.test.tsx b/packages/react-components/react-portal/library/src/components/Portal/Portal.test.tsx index a7317a2094f1cc..0640111fcd620f 100644 --- a/packages/react-components/react-portal/library/src/components/Portal/Portal.test.tsx +++ b/packages/react-components/react-portal/library/src/components/Portal/Portal.test.tsx @@ -6,14 +6,11 @@ import * as React from 'react'; import { Portal } from './Portal'; describe('Portal', () => { - /** - * Note: see more visual regression tests for Portal in /apps/vr-tests. - */ - it('renders a default state', () => { - const children = 'test'; - const { getByText } = render({children}); - - expect(getByText(children)).toMatchSnapshot(); + it('creates an element and attaches it to "document.body"', () => { + const { getByText } = render(Test); + const element = getByText('Test'); + + expect(document.body.children).toContain(element); }); it('applies "dir" attribute based on a context value', () => { @@ -33,13 +30,16 @@ describe('Portal', () => { expect(getByText('RTL')).toHaveAttribute('dir', 'rtl'); }); + it('applies "className"', () => { + const { getByText } = render(Test); + + expect(getByText('Test')).toHaveClass('foo'); + }); + it('applies "zIndex" style', () => { const { getByText } = render(Test); - const element = getByText('Test'); - expect(element).toHaveStyle({ - zIndex: 1000000, - }); + expect(getByText('Test')).toHaveStyle({ zIndex: 1000000 }); }); it('should not set virtual parent if mount node contains virtual parent', () => { @@ -59,4 +59,46 @@ describe('Portal', () => { const mountNode = container.querySelector('#container'); expect((getParent(mountNode) as HTMLElement).id).toBe('parent'); }); + + describe('mountNode', () => { + it('renders portal content into the specified mount node', () => { + const mountNode = document.createElement('div'); + + mountNode.id = 'mount-node'; + document.body.appendChild(mountNode); + + const { getByText } = render( + + Test + , + ); + const portalEl = getByText('Test'); + + expect(portalEl).toBeInstanceOf(HTMLSpanElement); + expect(portalEl.parentElement).toBe(mountNode); + }); + + it('does not add attributes to a mount node', () => { + const mountNode = document.createElement('div'); + + mountNode.id = 'mount-node'; + document.body.appendChild(mountNode); + + render( + + Test + , + ); + + expect(mountNode).toMatchInlineSnapshot(` +
+ + Test + +
+ `); + }); + }); }); diff --git a/packages/react-components/react-portal/library/src/components/Portal/Portal.types.ts b/packages/react-components/react-portal/library/src/components/Portal/Portal.types.ts index 32cc3590138de0..18dc69908db581 100644 --- a/packages/react-components/react-portal/library/src/components/Portal/Portal.types.ts +++ b/packages/react-components/react-portal/library/src/components/Portal/Portal.types.ts @@ -1,5 +1,10 @@ +import type { ComponentState, Slot } from '@fluentui/react-utilities'; import * as React from 'react'; +export type PortalInternalSlots = { + root?: Slot<'div'>; +}; + export type PortalProps = { /** * React children @@ -14,11 +19,12 @@ export type PortalProps = { mountNode?: HTMLElement | null | { element?: HTMLElement | null; className?: string }; }; -export type PortalState = Pick & { - mountNode: HTMLElement | null | undefined; +export type PortalState = ComponentState & + Pick & { + mountNode: HTMLElement | null | undefined; - /** - * Ref to the root span element as virtual parent - */ - virtualParentRootRef: React.MutableRefObject; -}; + /** + * Ref to the root span element as virtual parent + */ + virtualParentRootRef: React.MutableRefObject; + }; diff --git a/packages/react-components/react-portal/library/src/components/Portal/__snapshots__/Portal.test.tsx.snap b/packages/react-components/react-portal/library/src/components/Portal/__snapshots__/Portal.test.tsx.snap deleted file mode 100644 index 97c165d2a52a99..00000000000000 --- a/packages/react-components/react-portal/library/src/components/Portal/__snapshots__/Portal.test.tsx.snap +++ /dev/null @@ -1,11 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`Portal renders a default state 1`] = ` -
- test -
-`; diff --git a/packages/react-components/react-portal/library/src/components/Portal/renderPortal.tsx b/packages/react-components/react-portal/library/src/components/Portal/renderPortal.tsx index f94cfafc2f64dc..b8f027630568e0 100644 --- a/packages/react-components/react-portal/library/src/components/Portal/renderPortal.tsx +++ b/packages/react-components/react-portal/library/src/components/Portal/renderPortal.tsx @@ -1,14 +1,21 @@ +/** @jsxRuntime automatic */ +/** @jsxImportSource @fluentui/react-jsx-runtime */ +import { assertSlots } from '@fluentui/react-utilities'; import * as ReactDOM from 'react-dom'; import * as React from 'react'; -import type { PortalState } from './Portal.types'; + +import type { PortalState, PortalInternalSlots } from './Portal.types'; /** * Render the final JSX of Portal */ export const renderPortal_unstable = (state: PortalState): React.ReactElement => { + assertSlots(state); + return ( ); }; diff --git a/packages/react-components/react-portal/library/src/components/Portal/usePortal.ts b/packages/react-components/react-portal/library/src/components/Portal/usePortal.ts index a38848d50de6ec..50d805f64f8916 100644 --- a/packages/react-components/react-portal/library/src/components/Portal/usePortal.ts +++ b/packages/react-components/react-portal/library/src/components/Portal/usePortal.ts @@ -1,9 +1,15 @@ -import { setVirtualParent } from '@fluentui/react-utilities'; +import { mergeClasses } from '@griffel/react'; +import { + useFluent_unstable as useFluent, + useThemeClassName_unstable as useThemeClassName, +} from '@fluentui/react-shared-contexts'; +import { useFocusVisible } from '@fluentui/react-tabster'; +import { setVirtualParent, slot, useMergedRefs } from '@fluentui/react-utilities'; import * as React from 'react'; import { toMountNodeProps } from '../../utils/toMountNodeProps'; -import { usePortalMountNode } from './usePortalMountNode'; import type { PortalProps, PortalState } from './Portal.types'; +import { usePortalMountNodeStylesStyles } from './usePortalMountNodeStyles.styles'; /** * Create the state required to render Portal. @@ -15,17 +21,42 @@ import type { PortalProps, PortalState } from './Portal.types'; export const usePortal_unstable = (props: PortalProps): PortalState => { const { element, className } = toMountNodeProps(props.mountNode); + const { dir, targetDocument } = useFluent(); const virtualParentRootRef = React.useRef(null); - const fallbackElement = usePortalMountNode({ disabled: !!element, className }); - const mountNode = element ?? fallbackElement; + const classes = usePortalMountNodeStylesStyles(); + const themeClassName = useThemeClassName(); + + const ref = useMergedRefs(useFocusVisible()); const state: PortalState = { + components: { + root: 'div', + }, + children: props.children, - mountNode, + root: slot.optional( + { + className: mergeClasses(themeClassName, classes.root, className), + dir, + ref, + + 'data-portal-node': true, + }, + { elementType: 'div' }, + ), + + mountNode: targetDocument?.body, virtualParentRootRef, }; + if (element) { + state.mountNode = element; + state.root = undefined; + } + React.useEffect(() => { + const mountNode = element ?? ref.current; + if (!mountNode) { return; } @@ -80,7 +111,7 @@ export const usePortal_unstable = (props: PortalProps): PortalState => { setVirtualParent(mountNode, undefined); }; } - }, [virtualParentRootRef, mountNode]); + }, [virtualParentRootRef, element, ref]); return state; }; diff --git a/packages/react-components/react-portal/library/src/components/Portal/usePortalMountNode.test.tsx b/packages/react-components/react-portal/library/src/components/Portal/usePortalMountNode.test.tsx deleted file mode 100644 index 897b6490e40aac..00000000000000 --- a/packages/react-components/react-portal/library/src/components/Portal/usePortalMountNode.test.tsx +++ /dev/null @@ -1,37 +0,0 @@ -import { renderHook } from '@testing-library/react-hooks'; -import { PortalMountNodeProvider } from '@fluentui/react-shared-contexts'; -import * as React from 'react'; - -import { usePortalMountNode } from './usePortalMountNode'; - -describe('usePortalMountNode', () => { - it('creates an element and attaches it to "document.body"', () => { - const { result } = renderHook(() => usePortalMountNode({})); - - expect(result.current).toBeInstanceOf(HTMLDivElement); - expect(result.current).toHaveAttribute('data-portal-node', 'true'); - expect(document.body.contains(result.current)).toBeTruthy(); - }); - - it('creates an element and attaches it to "mountNode"', () => { - const mountNode = document.createElement('div'); - const { result } = renderHook(() => usePortalMountNode({}), { - wrapper: props => , - }); - - expect(result.current).toBeInstanceOf(HTMLDivElement); - expect(mountNode.contains(result.current)).toBeTruthy(); - }); - - it('applies classes to an element', () => { - const { result } = renderHook(() => usePortalMountNode({ className: 'foo' })); - - expect(result.current?.classList).toContain('foo'); - }); - - it('does not create an element if is disabled', () => { - const { result } = renderHook(() => usePortalMountNode({ disabled: true })); - - expect(result.current).toBeNull(); - }); -}); diff --git a/packages/react-components/react-portal/library/src/components/Portal/usePortalMountNode.ts b/packages/react-components/react-portal/library/src/components/Portal/usePortalMountNode.ts deleted file mode 100644 index 403411e3dc9dc9..00000000000000 --- a/packages/react-components/react-portal/library/src/components/Portal/usePortalMountNode.ts +++ /dev/null @@ -1,91 +0,0 @@ -import * as React from 'react'; -import { - useThemeClassName_unstable as useThemeClassName, - useFluent_unstable as useFluent, - usePortalMountNode as usePortalMountNodeContext, -} from '@fluentui/react-shared-contexts'; -import { mergeClasses } from '@griffel/react'; -import { useFocusVisible } from '@fluentui/react-tabster'; -import { useDisposable } from 'use-disposable'; - -import { usePortalMountNodeStylesStyles } from './usePortalMountNodeStyles.styles'; - -const useInsertionEffect = (React as never)['useInsertion' + 'Effect'] as typeof React.useLayoutEffect | undefined; - -export type UsePortalMountNodeOptions = { - /** - * Since hooks cannot be called conditionally use this flag to disable creating the node - */ - disabled?: boolean; - - className?: string; -}; - -/** - * Creates a new element on a "document.body" to mount portals. - */ -export const usePortalMountNode = (options: UsePortalMountNodeOptions): HTMLElement | null => { - 'use no memo'; - - const { targetDocument, dir } = useFluent(); - const mountNode = usePortalMountNodeContext(); - - const focusVisibleRef = useFocusVisible() as React.MutableRefObject; - const classes = usePortalMountNodeStylesStyles(); - const themeClassName = useThemeClassName(); - - const className = mergeClasses(themeClassName, classes.root, options.className); - const targetNode: HTMLElement | ShadowRoot | undefined = mountNode ?? targetDocument?.body; - - const element = useDisposable(() => { - if (targetNode === undefined || options.disabled) { - return [null, () => null]; - } - - const newElement = targetNode.ownerDocument.createElement('div'); - targetNode.appendChild(newElement); - return [newElement, () => newElement.remove()]; - }, [targetNode]); - - if (useInsertionEffect) { - // eslint-disable-next-line react-hooks/rules-of-hooks - useInsertionEffect(() => { - if (!element) { - return; - } - - const classesToApply = className.split(' ').filter(Boolean); - - element.classList.add(...classesToApply); - element.setAttribute('dir', dir); - element.setAttribute('data-portal-node', 'true'); - - focusVisibleRef.current = element; - - return () => { - element.classList.remove(...classesToApply); - element.removeAttribute('dir'); - }; - }, [className, dir, element, focusVisibleRef]); - } else { - // This useMemo call is intentional for React 17 - // We don't want to re-create the portal element when its attributes change. - // This also should not be done in an effect because, changing the value of css variables - // after initial mount can trigger interesting CSS side effects like transitions. - // eslint-disable-next-line react-hooks/rules-of-hooks - React.useMemo(() => { - if (!element) { - return; - } - - // Force replace all classes - element.className = className; - element.setAttribute('dir', dir); - element.setAttribute('data-portal-node', 'true'); - - focusVisibleRef.current = element; - }, [className, dir, element, focusVisibleRef]); - } - - return element; -};