diff --git a/change/@fluentui-react-portal-323fba1d-79e4-4b55-af4f-cfa9444a2bcf.json b/change/@fluentui-react-portal-323fba1d-79e4-4b55-af4f-cfa9444a2bcf.json new file mode 100644 index 0000000000000..6d6ebf4c917df --- /dev/null +++ b/change/@fluentui-react-portal-323fba1d-79e4-4b55-af4f-cfa9444a2bcf.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "refactor: usePortalMount node updates mount node attributes in memo", + "packageName": "@fluentui/react-portal", + "email": "lingfangao@hotmail.com", + "dependentChangeType": "patch" +} diff --git a/change/@fluentui-react-positioning-b6d61cdf-e434-43ff-a859-ab563e6a47f5.json b/change/@fluentui-react-positioning-b6d61cdf-e434-43ff-a859-ab563e6a47f5.json new file mode 100644 index 0000000000000..cfb0b890bbe45 --- /dev/null +++ b/change/@fluentui-react-positioning-b6d61cdf-e434-43ff-a859-ab563e6a47f5.json @@ -0,0 +1,7 @@ +{ + "type": "minor", + "comment": "feat: position updates are handled out of react lifecycle", + "packageName": "@fluentui/react-positioning", + "email": "lingfangao@hotmail.com", + "dependentChangeType": "patch" +} diff --git a/packages/react-components/react-portal/src/components/Portal/usePortalMountNode.ts b/packages/react-components/react-portal/src/components/Portal/usePortalMountNode.ts index 89558727cc0b9..04c6f0ebdf1f3 100644 --- a/packages/react-components/react-portal/src/components/Portal/usePortalMountNode.ts +++ b/packages/react-components/react-portal/src/components/Portal/usePortalMountNode.ts @@ -1,5 +1,4 @@ import * as React from 'react'; -import { useIsomorphicLayoutEffect } from '@fluentui/react-utilities'; import { useThemeClassName_unstable as useThemeClassName, useFluent_unstable as useFluent, @@ -44,7 +43,11 @@ export const usePortalMountNode = (options: UsePortalMountNodeOptions): HTMLElem return newElement; }, [targetDocument, options.disabled]); - useIsomorphicLayoutEffect(() => { + // This useMemo call is intentional + // 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. + React.useMemo(() => { if (element) { const classesToApply = className.split(' ').filter(Boolean); diff --git a/packages/react-components/react-positioning/etc/react-positioning.api.md b/packages/react-components/react-positioning/etc/react-positioning.api.md index 417c02185fd89..202975793eea0 100644 --- a/packages/react-components/react-positioning/etc/react-positioning.api.md +++ b/packages/react-components/react-positioning/etc/react-positioning.api.md @@ -68,13 +68,13 @@ export type Position = 'above' | 'below' | 'before' | 'after'; // @public (undocumented) export type PositioningImperativeRef = { updatePosition: () => void; - setTarget: (target: HTMLElement | PositioningVirtualElement) => void; + setTarget: (target: TargetElement) => void; }; // @public (undocumented) export interface PositioningProps extends Omit { positioningRef?: React_2.Ref; - target?: HTMLElement | PositioningVirtualElement | null; + target?: TargetElement | null; } // @public (undocumented) @@ -105,11 +105,7 @@ export function resolvePositioningShorthand(shorthand: PositioningShorthand | un export type SetVirtualMouseTarget = (event: React_2.MouseEvent | MouseEvent | undefined | null) => void; // @internal (undocumented) -export function usePositioning(options: UsePositioningOptions): { - targetRef: React_2.MutableRefObject; - containerRef: React_2.MutableRefObject; - arrowRef: React_2.MutableRefObject; -}; +export function usePositioning(options: UsePositioningOptions): UsePositioningReturn; // @internal export const usePositioningMouseTarget: (initialState?: PositioningVirtualElement | (() => PositioningVirtualElement) | undefined) => readonly [PositioningVirtualElement | undefined, SetVirtualMouseTarget]; diff --git a/packages/react-components/react-positioning/src/createPositionManager.ts b/packages/react-components/react-positioning/src/createPositionManager.ts new file mode 100644 index 0000000000000..4cc1b421cec73 --- /dev/null +++ b/packages/react-components/react-positioning/src/createPositionManager.ts @@ -0,0 +1,122 @@ +import { computePosition } from '@floating-ui/dom'; +import type { Middleware, Placement, Strategy } from '@floating-ui/dom'; +import type { PositionManager, TargetElement } from './types'; +import { debounce, writeArrowUpdates, writeContainerUpdates, getScrollParent } from './utils'; + +interface PositionManagerOptions { + /** + * The positioned element + */ + container: HTMLElement; + /** + * Element that the container will be anchored to + */ + target: TargetElement; + /** + * Arrow that points from the container to the target + */ + arrow: HTMLElement | null; + /** + * The value of the css `position` property + * @default absolute + */ + strategy: Strategy; + /** + * [Floating UI middleware](https://floating-ui.com/docs/middleware) + */ + middleware: Middleware[]; + /** + * [Floating UI placement](https://floating-ui.com/docs/computePosition#placement) + */ + placement?: Placement; +} + +/** + * @internal + * @returns manager that handles positioning out of the react lifecycle + */ +export function createPositionManager(options: PositionManagerOptions): PositionManager { + const { container, target, arrow, strategy, middleware, placement } = options; + if (!target || !container) { + return { + updatePosition: () => undefined, + dispose: () => undefined, + }; + } + + let isFirstUpdate = true; + const scrollParents: Set = new Set(); + const targetWindow = container.ownerDocument.defaultView; + + // When the container is first resolved, set position `fixed` to avoid scroll jumps. + // Without this scroll jumps can occur when the element is rendered initially and receives focus + Object.assign(container.style, { position: 'fixed', left: 0, top: 0, margin: 0 }); + + const forceUpdate = () => { + if (isFirstUpdate) { + scrollParents.add(getScrollParent(container)); + if (target instanceof HTMLElement) { + scrollParents.add(getScrollParent(target)); + } + + scrollParents.forEach(scrollParent => { + scrollParent.addEventListener('scroll', updatePosition); + }); + + isFirstUpdate = false; + } + + Object.assign(container.style, { position: strategy }); + computePosition(target, container, { placement, middleware, strategy }) + .then(({ x, y, middlewareData, placement: computedPlacement }) => { + writeArrowUpdates({ arrow, middlewareData }); + writeContainerUpdates({ + container, + middlewareData, + placement: computedPlacement, + coordinates: { x, y }, + lowPPI: (targetWindow?.devicePixelRatio || 1) <= 1, + strategy, + }); + }) + .catch(err => { + // https://github.com/floating-ui/floating-ui/issues/1845 + // FIXME for node > 14 + // node 15 introduces promise rejection which means that any components + // tests need to be `it('', async () => {})` otherwise there can be race conditions with + // JSDOM being torn down before this promise is resolved so globals like `window` and `document` don't exist + // Unless all tests that ever use `usePositioning` are turned into async tests, any logging during testing + // will actually be counter productive + if (process.env.NODE_ENV === 'development') { + // eslint-disable-next-line no-console + console.error('[usePositioning]: Failed to calculate position', err); + } + }); + }; + + const updatePosition = debounce(() => forceUpdate()); + + const dispose = () => { + if (targetWindow) { + targetWindow.removeEventListener('scroll', updatePosition); + targetWindow.removeEventListener('resize', updatePosition); + } + + scrollParents.forEach(scrollParent => { + scrollParent.removeEventListener('scroll', updatePosition); + }); + }; + + if (targetWindow) { + targetWindow.addEventListener('scroll', updatePosition); + targetWindow.addEventListener('resize', updatePosition); + } + + // Update the position on initialization + updatePosition(); + + return { + updatePosition, + dispose, + }; +} diff --git a/packages/react-components/react-positioning/src/types.ts b/packages/react-components/react-positioning/src/types.ts index 7e5d84a69c5a1..44eaec449deed 100644 --- a/packages/react-components/react-positioning/src/types.ts +++ b/packages/react-components/react-positioning/src/types.ts @@ -14,6 +14,39 @@ export type OffsetFunctionParam = { alignment?: Alignment; }; +export type TargetElement = HTMLElement | PositioningVirtualElement; + +/** + * @internal + */ +export interface UsePositioningOptions extends PositioningProps { + /** + * If false, does not position anything + */ + enabled?: boolean; +} + +/** + * @internal + */ +export interface PositionManager { + updatePosition: () => void; + dispose: () => void; +} + +export interface UsePositioningReturn { + // React refs are supposed to be contravariant + // (allows a more general type to be passed rather than a more specific one) + // However, Typescript currently can't infer that fact for refs + // See https://github.com/microsoft/TypeScript/issues/30748 for more information + // eslint-disable-next-line @typescript-eslint/no-explicit-any + targetRef: React.MutableRefObject; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + containerRef: React.MutableRefObject; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + arrowRef: React.MutableRefObject; +} + export type OffsetObject = { crossAxis?: number; mainAxis: number }; export type OffsetShorthand = number; @@ -40,7 +73,7 @@ export type PositioningImperativeRef = { * Sets the target and updates positioning imperatively. * Useful for avoiding double renders with the target option. */ - setTarget: (target: HTMLElement | PositioningVirtualElement) => void; + setTarget: (target: TargetElement) => void; }; export type PositioningVirtualElement = { @@ -133,7 +166,7 @@ export interface PositioningProps /** * Manual override for the target element. Useful for scenarios where a component accepts user prop to override target */ - target?: HTMLElement | PositioningVirtualElement | null; + target?: TargetElement | null; } export type PositioningShorthandValue = diff --git a/packages/react-components/react-positioning/src/usePositioning.ts b/packages/react-components/react-positioning/src/usePositioning.ts index 5c054c6d14f92..2c29ec2b73688 100644 --- a/packages/react-components/react-positioning/src/usePositioning.ts +++ b/packages/react-components/react-positioning/src/usePositioning.ts @@ -1,18 +1,16 @@ -import { computePosition, hide as hideMiddleware, arrow as arrowMiddleware } from '@floating-ui/dom'; -import type { Middleware, Strategy, Placement, Coords, MiddlewareData } from '@floating-ui/dom'; +import { hide as hideMiddleware, arrow as arrowMiddleware } from '@floating-ui/dom'; +import type { Middleware, Strategy } from '@floating-ui/dom'; import { useFluent_unstable as useFluent } from '@fluentui/react-shared-contexts'; import { canUseDOM, useIsomorphicLayoutEffect } from '@fluentui/react-utilities'; -import { useEventCallback } from '@fluentui/react-utilities'; import * as React from 'react'; -import type { PositioningOptions, PositioningProps, PositioningVirtualElement } from './types'; -import { - useCallbackRef, - toFloatingUIPlacement, - toggleScrollListener, - hasAutofocusFilter, - debounce, - hasScrollParent, -} from './utils'; +import type { + PositioningOptions, + PositioningProps, + PositionManager, + TargetElement, + UsePositioningReturn, +} from './types'; +import { useCallbackRef, toFloatingUIPlacement, hasAutofocusFilter, hasScrollParent } from './utils'; import { shift as shiftMiddleware, flip as flipMiddleware, @@ -21,88 +19,51 @@ import { offset as offsetMiddleware, intersecting as intersectingMiddleware, } from './middleware'; -import { - DATA_POSITIONING_ESCAPED, - DATA_POSITIONING_INTERSECTING, - DATA_POSITIONING_HIDDEN, - DATA_POSITIONING_PLACEMENT, -} from './constants'; +import { createPositionManager } from './createPositionManager'; /** * @internal */ -export function usePositioning( - options: UsePositioningOptions, -): { - // React refs are supposed to be contravariant - // (allows a more general type to be passed rather than a more specific one) - // However, Typescript currently can't infer that fact for refs - // See https://github.com/microsoft/TypeScript/issues/30748 for more information - // eslint-disable-next-line @typescript-eslint/no-explicit-any - targetRef: React.MutableRefObject; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - containerRef: React.MutableRefObject; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - arrowRef: React.MutableRefObject; -} { - const { targetDocument } = useFluent(); +export function usePositioning(options: UsePositioningOptions): UsePositioningReturn { + const managerRef = React.useRef(null); + const targetRef = React.useRef(null); + const overrideTargetRef = React.useRef(null); + const containerRef = React.useRef(null); + const arrowRef = React.useRef(null); + const { enabled = true } = options; const resolvePositioningOptions = usePositioningOptions(options); - - const forceUpdate = useEventCallback(() => { - const target = overrideTargetRef.current ?? targetRef.current; - if (!canUseDOM || !enabled || !target || !containerRef.current) { - return; + const updatePositionManager = React.useCallback(() => { + if (managerRef.current) { + managerRef.current.dispose(); } + managerRef.current = null; - const { placement, middleware, strategy } = resolvePositioningOptions( - target, - containerRef.current, - arrowRef.current, - ); + const target = overrideTargetRef.current ?? targetRef.current; - // Container is always initialized with `position: fixed` to avoid scroll jumps - // Before computing the positioned coordinates, revert the container to the deisred positioning strategy - Object.assign(containerRef.current.style, { position: strategy }); - computePosition(target, containerRef.current, { placement, middleware, strategy }) - .then(({ x, y, middlewareData, placement: computedPlacement }) => { - writeArrowUpdates({ arrow: arrowRef.current, middlewareData }); - writeContainerUpdates({ - container: containerRef.current, - middlewareData, - placement: computedPlacement, - coordinates: { x, y }, - lowPPI: (targetDocument?.defaultView?.devicePixelRatio || 1) <= 1, - strategy, - }); - }) - .catch(err => { - // https://github.com/floating-ui/floating-ui/issues/1845 - // FIXME for node > 14 - // node 15 introduces promise rejection which means that any components - // tests need to be `it('', async () => {})` otherwise there can be race conditions with - // JSDOM being torn down before this promise is resolved so globals like `window` and `document` don't exist - // Unless all tests that ever use `usePositioning` are turned into async tests, any logging during testing - // will actually be counter productive - if (process.env.NODE_ENV === 'development') { - // eslint-disable-next-line no-console - console.error('[usePositioning]: Failed to calculate position', err); - } + if (enabled && canUseDOM() && target && containerRef.current) { + managerRef.current = createPositionManager({ + container: containerRef.current, + target, + arrow: arrowRef.current, + ...resolvePositioningOptions(containerRef.current, arrowRef.current), }); - }); - - const updatePosition = React.useState(() => debounce(forceUpdate))[0]; + } + }, [enabled, resolvePositioningOptions]); - const targetRef = useTargetRef(updatePosition); - const overrideTargetRef = useTargetRef(updatePosition); - const containerRef = useContainerRef(updatePosition, enabled); - const arrowRef = useArrowRef(updatePosition); + const setOverrideTarget = React.useCallback( + (target: TargetElement | null) => { + overrideTargetRef.current = target; + updatePositionManager(); + }, + [updatePositionManager], + ); React.useImperativeHandle( options.positioningRef, () => ({ - updatePosition, - setTarget: (target: HTMLElement | PositioningVirtualElement) => { + updatePosition: () => managerRef.current?.updatePosition(), + setTarget: (target: TargetElement) => { if (options.target && process.env.NODE_ENV !== 'production') { const err = new Error(); // eslint-disable-next-line no-console @@ -111,38 +72,19 @@ export function usePositioning( console.warn(err.stack); } - overrideTargetRef.current = target; + setOverrideTarget(target); }, }), - // Missing deps: - // options.target - only used for a runtime warning - // overrideTargetRef - Stable between renders - // updatePosition - Stable between renders - // eslint-disable-next-line react-hooks/exhaustive-deps - [], + [options.target, setOverrideTarget], ); useIsomorphicLayoutEffect(() => { - overrideTargetRef.current = options.target ?? null; - }, [options.target, overrideTargetRef, containerRef]); - - useIsomorphicLayoutEffect(() => { - updatePosition(); - }, [enabled, resolvePositioningOptions, updatePosition]); + setOverrideTarget(options.target ?? null); + }, [options.target, setOverrideTarget]); - // Add window resize and scroll listeners to update position useIsomorphicLayoutEffect(() => { - const win = targetDocument?.defaultView; - if (win) { - win.addEventListener('resize', updatePosition); - win.addEventListener('scroll', updatePosition); - - return () => { - win.removeEventListener('resize', updatePosition); - win.removeEventListener('scroll', updatePosition); - }; - } - }, [updatePosition, targetDocument]); + updatePositionManager(); + }, [updatePositionManager]); if (process.env.NODE_ENV !== 'production') { // This checked should run only in development mode @@ -186,7 +128,29 @@ export function usePositioning( }, []); } - return { targetRef, containerRef, arrowRef }; + const setTarget = useCallbackRef(null, target => { + if (targetRef.current !== target) { + targetRef.current = target; + updatePositionManager(); + } + }); + + const setContainer = useCallbackRef(null, container => { + if (containerRef.current !== container) { + containerRef.current = container; + updatePositionManager(); + } + }); + + const setArrow = useCallbackRef(null, arrow => { + if (arrowRef.current !== arrow) { + arrowRef.current = arrow; + updatePositionManager(); + } + }); + + // Let users use callback refs so they feel like 'normal' DOM refs + return { targetRef: setTarget, containerRef: setContainer, arrowRef: setArrow }; } interface UsePositioningOptions extends PositioningProps { @@ -216,11 +180,7 @@ function usePositioningOptions(options: PositioningOptions) { const strategy: Strategy = positionFixed ? 'fixed' : 'absolute'; return React.useCallback( - ( - target: HTMLElement | PositioningVirtualElement | null, - container: HTMLElement | null, - arrow: HTMLElement | null, - ) => { + (container: HTMLElement | null, arrow: HTMLElement | null) => { const hasScrollableElement = hasScrollParent(container); const placement = toFloatingUIPlacement(align, position, isRtl); @@ -258,95 +218,3 @@ function usePositioningOptions(options: PositioningOptions) { ], ); } - -function useContainerRef(updatePosition: () => void, enabled: boolean) { - return useCallbackRef(null, (container, prevContainer) => { - if (container && enabled) { - // When the container is first resolved, set position `fixed` to avoid scroll jumps. - // Without this scroll jumps can occur when the element is rendered initially and receives focus - Object.assign(container.style, { position: 'fixed', left: 0, top: 0, margin: 0 }); - } - - toggleScrollListener(container, prevContainer, updatePosition); - - updatePosition(); - }); -} - -function useTargetRef(updatePosition: () => void) { - return useCallbackRef(null, (target, prevTarget) => { - toggleScrollListener(target, prevTarget, updatePosition); - - updatePosition(); - }); -} - -function useArrowRef(updatePosition: () => void) { - return useCallbackRef(null, updatePosition); -} - -/** - * Writes all DOM element updates after position is computed - */ -function writeContainerUpdates(options: { - container: HTMLElement | null; - placement: Placement; - middlewareData: MiddlewareData; - /** - * Layer acceleration can disable subpixel rendering which causes slightly - * blurry text on low PPI displays, so we want to use 2D transforms - * instead - */ - lowPPI: boolean; - strategy: Strategy; - coordinates: Coords; -}) { - const { - container, - placement, - middlewareData, - strategy, - lowPPI, - coordinates: { x, y }, - } = options; - if (!container) { - return; - } - container.setAttribute(DATA_POSITIONING_PLACEMENT, placement); - container.removeAttribute(DATA_POSITIONING_INTERSECTING); - if (middlewareData.intersectionObserver.intersecting) { - container.setAttribute(DATA_POSITIONING_INTERSECTING, ''); - } - - container.removeAttribute(DATA_POSITIONING_ESCAPED); - if (middlewareData.hide?.escaped) { - container.setAttribute(DATA_POSITIONING_ESCAPED, ''); - } - - container.removeAttribute(DATA_POSITIONING_HIDDEN); - if (middlewareData.hide?.referenceHidden) { - container.setAttribute(DATA_POSITIONING_HIDDEN, ''); - } - - Object.assign(container.style, { - transform: lowPPI ? `translate(${x}px, ${y}px)` : `translate3d(${x}px, ${y}px, 0)`, - position: strategy, - }); -} - -/** - * Writes all DOM element updates after position is computed - */ -function writeArrowUpdates(options: { arrow: HTMLElement | null; middlewareData: MiddlewareData }) { - const { arrow, middlewareData } = options; - if (!middlewareData.arrow || !arrow) { - return; - } - - const { x: arrowX, y: arrowY } = middlewareData.arrow; - - Object.assign(arrow.style, { - left: `${arrowX}px`, - top: `${arrowY}px`, - }); -} diff --git a/packages/react-components/react-positioning/src/utils/index.ts b/packages/react-components/react-positioning/src/utils/index.ts index f8d4270df5536..8b4f71faaf70f 100644 --- a/packages/react-components/react-positioning/src/utils/index.ts +++ b/packages/react-components/react-positioning/src/utils/index.ts @@ -10,3 +10,5 @@ export * from './useCallbackRef'; export * from './debounce'; export * from './toggleScrollListener'; export * from './hasAutoFocusFilter'; +export * from './writeArrowUpdates'; +export * from './writeContainerupdates'; diff --git a/packages/react-components/react-positioning/src/utils/writeArrowUpdates.ts b/packages/react-components/react-positioning/src/utils/writeArrowUpdates.ts new file mode 100644 index 0000000000000..87b1ebc30c7ba --- /dev/null +++ b/packages/react-components/react-positioning/src/utils/writeArrowUpdates.ts @@ -0,0 +1,18 @@ +import { MiddlewareData } from '@floating-ui/dom'; + +/** + * Writes all DOM element updates after position is computed + */ +export function writeArrowUpdates(options: { arrow: HTMLElement | null; middlewareData: MiddlewareData }) { + const { arrow, middlewareData } = options; + if (!middlewareData.arrow || !arrow) { + return; + } + + const { x: arrowX, y: arrowY } = middlewareData.arrow; + + Object.assign(arrow.style, { + left: `${arrowX}px`, + top: `${arrowY}px`, + }); +} diff --git a/packages/react-components/react-positioning/src/utils/writeContainerupdates.ts b/packages/react-components/react-positioning/src/utils/writeContainerupdates.ts new file mode 100644 index 0000000000000..86026375f02ce --- /dev/null +++ b/packages/react-components/react-positioning/src/utils/writeContainerupdates.ts @@ -0,0 +1,56 @@ +import type { Placement, MiddlewareData, Strategy, Coords } from '@floating-ui/dom'; +import { + DATA_POSITIONING_ESCAPED, + DATA_POSITIONING_HIDDEN, + DATA_POSITIONING_INTERSECTING, + DATA_POSITIONING_PLACEMENT, +} from '../constants'; + +/** + * Writes all container element position updates after the position is computed + */ +export function writeContainerUpdates(options: { + container: HTMLElement | null; + placement: Placement; + middlewareData: MiddlewareData; + /** + * Layer acceleration can disable subpixel rendering which causes slightly + * blurry text on low PPI displays, so we want to use 2D transforms + * instead + */ + lowPPI: boolean; + strategy: Strategy; + coordinates: Coords; +}) { + const { + container, + placement, + middlewareData, + strategy, + lowPPI, + coordinates: { x, y }, + } = options; + if (!container) { + return; + } + container.setAttribute(DATA_POSITIONING_PLACEMENT, placement); + container.removeAttribute(DATA_POSITIONING_INTERSECTING); + if (middlewareData.intersectionObserver.intersecting) { + container.setAttribute(DATA_POSITIONING_INTERSECTING, ''); + } + + container.removeAttribute(DATA_POSITIONING_ESCAPED); + if (middlewareData.hide?.escaped) { + container.setAttribute(DATA_POSITIONING_ESCAPED, ''); + } + + container.removeAttribute(DATA_POSITIONING_HIDDEN); + if (middlewareData.hide?.referenceHidden) { + container.setAttribute(DATA_POSITIONING_HIDDEN, ''); + } + + Object.assign(container.style, { + transform: lowPPI ? `translate(${x}px, ${y}px)` : `translate3d(${x}px, ${y}px, 0)`, + position: strategy, + }); +}