From dc5c459f9197d65434d69a84106b878ebc2d9496 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Fri, 29 Dec 2023 05:53:03 +0100 Subject: [PATCH 1/6] feat: Implement onPositioningEnd callback Fixes #30176 --- .../src/stories/Positioning.stories.tsx | 30 ++++- .../PositioningListenToUpdates.stories.tsx | 127 ++++++++++++++++++ .../Concepts/Positioning/index.stories.tsx | 1 + .../stories/Menu/MenuDefault.stories.tsx | 96 ++++++++++--- .../etc/react-positioning.api.md | 2 +- .../react-positioning/src/constants.ts | 1 + .../src/createPositionManager.ts | 5 +- .../react-positioning/src/types.ts | 9 ++ .../react-positioning/src/usePositioning.ts | 4 + 9 files changed, 256 insertions(+), 19 deletions(-) create mode 100644 packages/react-components/react-components/stories/Concepts/Positioning/PositioningListenToUpdates.stories.tsx diff --git a/apps/vr-tests-react-components/src/stories/Positioning.stories.tsx b/apps/vr-tests-react-components/src/stories/Positioning.stories.tsx index a6f64da5c376d5..38bd321f888d8f 100644 --- a/apps/vr-tests-react-components/src/stories/Positioning.stories.tsx +++ b/apps/vr-tests-react-components/src/stories/Positioning.stories.tsx @@ -1159,6 +1159,29 @@ const MatchTargetSize = () => { ); }; +const PositioningEndEvent = () => { + const positioningRef = React.useRef(null); + const [count, setCount] = React.useState(0); + const { targetRef, containerRef } = usePositioning({ + onPositioningEnd: () => setCount(s => s + 1), + positioningRef, + }); + + return ( + <> + +
+ positioning count: {count} +
+ + ); +}; + storiesOf('Positioning', module) .addDecorator(story => (
)) - .addStory('Match target size', () => ); + .addStory('Match target size', () => ) + .addStory('Positioning end', () => ( + + + + )); storiesOf('Positioning (no decorator)', module) .addStory('scroll jumps', () => ( diff --git a/packages/react-components/react-components/stories/Concepts/Positioning/PositioningListenToUpdates.stories.tsx b/packages/react-components/react-components/stories/Concepts/Positioning/PositioningListenToUpdates.stories.tsx new file mode 100644 index 00000000000000..9570ad31ec115a --- /dev/null +++ b/packages/react-components/react-components/stories/Concepts/Positioning/PositioningListenToUpdates.stories.tsx @@ -0,0 +1,127 @@ +import * as React from 'react'; +import { + useId, + Text, + makeStyles, + shorthands, + tokens, + Popover, + Button, + PopoverTrigger, + PopoverSurface, + PositioningImperativeRef, + PopoverProps, +} from '@fluentui/react-components'; + +const useStyles = makeStyles({ + root: { + display: 'flex', + ...shorthands.gap('20px'), + }, + + button: { + display: 'block', + minWidth: '120px', + }, + + logContainer: { + display: 'flex', + flexDirection: 'column', + }, + + logLabel: { + color: tokens.colorNeutralForegroundOnBrand, + backgroundColor: tokens.colorBrandBackground, + width: 'fit-content', + fontWeight: tokens.fontWeightBold, + ...shorthands.padding('2px', '12px'), + }, + + log: { + overflowY: 'auto', + boxShadow: tokens.shadow16, + position: 'relative', + minWidth: '200px', + height: '200px', + ...shorthands.border('2px', 'solid', tokens.colorBrandBackground), + ...shorthands.padding('12px', '12px'), + }, +}); + +export const ListenToUpdates = () => { + const styles = useStyles(); + const labelId = useId(); + const [statusLog, setStatusLog] = React.useState([]); + const positioningRef = React.useRef(null); + const [open, setOpen] = React.useState(false); + + const onOpenChange: PopoverProps['onOpenChange'] = React.useCallback((e, data) => { + setOpen(data.open); + if (!data.open) { + setStatusLog([]); + } + }, []); + + const updatePosition = React.useCallback(() => { + positioningRef.current?.updatePosition(); + }, []); + + const onPositioningEnd = React.useCallback(() => { + setStatusLog(s => [Date.now(), ...s]); + }, []); + + return ( +
+
+ + + + + + + + +
+
+
+ Status log +
+
+ {statusLog.map((time, i) => { + const date = new Date(time); + return ( +
+ {date.toLocaleTimeString()} Position updated +
+ ); + })} +
+
+
+ ); +}; + +ListenToUpdates.parameters = { + docs: { + description: { + story: [ + 'Positioning happens outside of the React render lifecycle for performance purposes so that a position update', + 'does not need to:', + '- trigger by a re-render', + '- be dependent on a re-render', + '', + 'This constraint makes it difficult to know exactly when an element has been positioned. In order to listen', + 'to position updates you can use the `onPositioningEnd` callback.', + '', + '> ⚠️ _Very few use cases would actually require listening to position updates. Please renderer that_', + '_there is a difference between **open/close state** which is normally handled in React_', + ].join('\n'), + }, + }, +}; diff --git a/packages/react-components/react-components/stories/Concepts/Positioning/index.stories.tsx b/packages/react-components/react-components/stories/Concepts/Positioning/index.stories.tsx index 2f79d0c5896d79..14f39e6c0d8b23 100644 --- a/packages/react-components/react-components/stories/Concepts/Positioning/index.stories.tsx +++ b/packages/react-components/react-components/stories/Concepts/Positioning/index.stories.tsx @@ -15,6 +15,7 @@ export { OverflowBoundaryPadding } from './OverflowBoundaryPadding.stories'; export { FlipBoundary } from './PositioningFlipBoundary.stories'; export { MatchTargetSize } from './MatchTargetSize.stories'; export { DisableTransform } from './PositioningDisableTransform.stories'; +export { ListenToUpdates } from './PositioningListenToUpdates.stories'; export default { title: 'Concepts/Developer/Positioning Components', diff --git a/packages/react-components/react-menu/stories/Menu/MenuDefault.stories.tsx b/packages/react-components/react-menu/stories/Menu/MenuDefault.stories.tsx index bb6837eade01a1..30071b0c9abd25 100644 --- a/packages/react-components/react-menu/stories/Menu/MenuDefault.stories.tsx +++ b/packages/react-components/react-menu/stories/Menu/MenuDefault.stories.tsx @@ -1,20 +1,84 @@ import * as React from 'react'; -import { Button, Menu, MenuTrigger, MenuList, MenuItem, MenuPopover } from '@fluentui/react-components'; +import { + Button, + Menu, + MenuTrigger, + MenuList, + MenuItem, + MenuPopover, + PositioningImperativeRef, + PositioningVirtualElement, + PopoverSurface, + Popover, +} from '@fluentui/react-components'; -export const Default = () => ( - - - - +export const Default = () => { + const [menuOpen, setMenuOpen] = React.useState(false); + const [popoverOpen, setPopoverOpen] = React.useState(false); + const menuPositiningRef = React.useRef(null); + const popoverPositiningRef = React.useRef(null); + const menuPopoverRef = React.useRef(null); - - - New - New Window - Open File - Open Folder - - - -); + React.useEffect(() => { + const getRect = (x = 0, y = 0) => { + return () => ({ + width: 0, + height: 0, + top: y, + right: x, + bottom: y, + left: x, + x, + y, + }); + }; + const virtualElement: PositioningVirtualElement = { + getBoundingClientRect: getRect(100, 100), + }; + menuPositiningRef.current?.setTarget(virtualElement); + setPopoverOpen(menuOpen); + }, [menuOpen]); + + React.useEffect(() => { + if (menuOpen && menuPopoverRef.current) { + popoverPositiningRef.current?.setTarget(menuPopoverRef.current); + setPopoverOpen(menuOpen); + // uncomment this to make the relative positioning work + // setTimeout(() => popoverPositiningRef.current?.updatePosition(), 3000); + } + }, [menuOpen]); + + return ( + <> + setPopoverOpen(data.open)} + positioning={{ positioningRef: popoverPositiningRef, position: 'below' }} + > + This is a popover + + setMenuOpen(data.open)} + positioning={{ + positioningRef: menuPositiningRef, + onPositioningEnd: () => popoverPositiningRef.current?.updatePosition(), + }} + > + + + + + + + New + New Window + Open File + Open Folder + + + + + ); +}; 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 28dbce76f0926f..5a1568d4f7d837 100644 --- a/packages/react-components/react-positioning/etc/react-positioning.api.md +++ b/packages/react-components/react-positioning/etc/react-positioning.api.md @@ -75,7 +75,7 @@ export type PositioningImperativeRef = { }; // @public -export interface PositioningProps extends Pick { +export interface PositioningProps extends Pick { positioningRef?: React_2.Ref; target?: TargetElement | null; } diff --git a/packages/react-components/react-positioning/src/constants.ts b/packages/react-components/react-positioning/src/constants.ts index 9766a880b866fb..e6fd421d130c84 100644 --- a/packages/react-components/react-positioning/src/constants.ts +++ b/packages/react-components/react-positioning/src/constants.ts @@ -2,3 +2,4 @@ export const DATA_POSITIONING_INTERSECTING = 'data-popper-is-intersecting'; export const DATA_POSITIONING_ESCAPED = 'data-popper-escaped'; export const DATA_POSITIONING_HIDDEN = 'data-popper-reference-hidden'; export const DATA_POSITIONING_PLACEMENT = 'data-popper-placement'; +export const POSITIONING_END_EVENT = 'fui-positioningend'; diff --git a/packages/react-components/react-positioning/src/createPositionManager.ts b/packages/react-components/react-positioning/src/createPositionManager.ts index 0b7a354162874f..ff4f10461370a9 100644 --- a/packages/react-components/react-positioning/src/createPositionManager.ts +++ b/packages/react-components/react-positioning/src/createPositionManager.ts @@ -1,9 +1,10 @@ import { computePosition } from '@floating-ui/dom'; import type { Middleware, Placement, Strategy } from '@floating-ui/dom'; +import { isHTMLElement } from '@fluentui/react-utilities'; import type { PositionManager, TargetElement } from './types'; import { debounce, writeArrowUpdates, writeContainerUpdates } from './utils'; -import { isHTMLElement } from '@fluentui/react-utilities'; import { listScrollParents } from './utils/listScrollParents'; +import { POSITIONING_END_EVENT } from './constants'; interface PositionManagerOptions { /** @@ -99,6 +100,8 @@ export function createPositionManager(options: PositionManagerOptions): Position strategy, useTransform, }); + + container.dispatchEvent(new CustomEvent(POSITIONING_END_EVENT)); }) .catch(err => { // https://github.com/floating-ui/floating-ui/issues/1845 diff --git a/packages/react-components/react-positioning/src/types.ts b/packages/react-components/react-positioning/src/types.ts index 3d29b8f7ddd9a3..5db505dda67700 100644 --- a/packages/react-components/react-positioning/src/types.ts +++ b/packages/react-components/react-positioning/src/types.ts @@ -184,6 +184,14 @@ export interface PositioningOptions { * When set, the positioned element matches the chosen dimension(s) of the target element */ matchTargetSize?: 'width'; + + /** + * Called when a position update has finished. Multiple position updates can happen in a single render, + * since positioning happens outside of the React lifecycle. + * + * It's also possible to listen to the custom DOM event `fui-positioningend` + */ + onPositioningEnd?: () => void; } /** @@ -205,6 +213,7 @@ export interface PositioningProps | 'strategy' | 'useTransform' | 'matchTargetSize' + | 'onPositioningEnd' > { /** An imperative handle to Popper methods. */ positioningRef?: React.Ref; diff --git a/packages/react-components/react-positioning/src/usePositioning.ts b/packages/react-components/react-positioning/src/usePositioning.ts index 40d8aa1a1849f2..299fb2cd44e368 100644 --- a/packages/react-components/react-positioning/src/usePositioning.ts +++ b/packages/react-components/react-positioning/src/usePositioning.ts @@ -24,6 +24,7 @@ import { import { createPositionManager } from './createPositionManager'; import { devtools } from '@floating-ui/devtools'; import { devtoolsCallback } from './utils/devtools'; +import { POSITIONING_END_EVENT } from './constants'; /** * @internal @@ -135,8 +136,11 @@ export function usePositioning(options: PositioningProps & PositioningOptions): } }); + const onPositioningEnd = useEventCallback(() => options.onPositioningEnd?.()); const setContainer = useCallbackRef(null, container => { if (containerRef.current !== container) { + containerRef.current?.removeEventListener(POSITIONING_END_EVENT, onPositioningEnd); + container?.addEventListener(POSITIONING_END_EVENT, onPositioningEnd); containerRef.current = container; updatePositionManager(); } From 57929ba101d2b1f6abc24d907b0f011cdfb67ace Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Fri, 29 Dec 2023 05:56:26 +0100 Subject: [PATCH 2/6] changefile --- ...t-positioning-d30b18b7-e743-42d7-b3bc-0b1ff22655fd.json | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 change/@fluentui-react-positioning-d30b18b7-e743-42d7-b3bc-0b1ff22655fd.json diff --git a/change/@fluentui-react-positioning-d30b18b7-e743-42d7-b3bc-0b1ff22655fd.json b/change/@fluentui-react-positioning-d30b18b7-e743-42d7-b3bc-0b1ff22655fd.json new file mode 100644 index 00000000000000..a0ac2ea6ae9e4f --- /dev/null +++ b/change/@fluentui-react-positioning-d30b18b7-e743-42d7-b3bc-0b1ff22655fd.json @@ -0,0 +1,7 @@ +{ + "type": "minor", + "comment": "feat: Implement onPositioningEnd callback", + "packageName": "@fluentui/react-positioning", + "email": "lingfangao@hotmail.com", + "dependentChangeType": "patch" +} From 133a7073a22c16edbfe3feed6b0f2a95537a1a2d Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Fri, 29 Dec 2023 06:46:13 +0100 Subject: [PATCH 3/6] fix vr test --- .../src/stories/Positioning.stories.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/vr-tests-react-components/src/stories/Positioning.stories.tsx b/apps/vr-tests-react-components/src/stories/Positioning.stories.tsx index 38bd321f888d8f..a4de2da8378295 100644 --- a/apps/vr-tests-react-components/src/stories/Positioning.stories.tsx +++ b/apps/vr-tests-react-components/src/stories/Positioning.stories.tsx @@ -1267,7 +1267,7 @@ storiesOf('Positioning', module) )) .addStory('Match target size', () => ) .addStory('Positioning end', () => ( - + )); From e2cf689088ab8800b82f3c0db22325794f53a230 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Fri, 5 Jan 2024 11:47:21 +0100 Subject: [PATCH 4/6] revert --- .../stories/Menu/MenuDefault.stories.tsx | 96 ++++--------------- 1 file changed, 16 insertions(+), 80 deletions(-) diff --git a/packages/react-components/react-menu/stories/Menu/MenuDefault.stories.tsx b/packages/react-components/react-menu/stories/Menu/MenuDefault.stories.tsx index 30071b0c9abd25..bb6837eade01a1 100644 --- a/packages/react-components/react-menu/stories/Menu/MenuDefault.stories.tsx +++ b/packages/react-components/react-menu/stories/Menu/MenuDefault.stories.tsx @@ -1,84 +1,20 @@ import * as React from 'react'; -import { - Button, - Menu, - MenuTrigger, - MenuList, - MenuItem, - MenuPopover, - PositioningImperativeRef, - PositioningVirtualElement, - PopoverSurface, - Popover, -} from '@fluentui/react-components'; +import { Button, Menu, MenuTrigger, MenuList, MenuItem, MenuPopover } from '@fluentui/react-components'; -export const Default = () => { - const [menuOpen, setMenuOpen] = React.useState(false); - const [popoverOpen, setPopoverOpen] = React.useState(false); - const menuPositiningRef = React.useRef(null); - const popoverPositiningRef = React.useRef(null); - const menuPopoverRef = React.useRef(null); +export const Default = () => ( + + + + - React.useEffect(() => { - const getRect = (x = 0, y = 0) => { - return () => ({ - width: 0, - height: 0, - top: y, - right: x, - bottom: y, - left: x, - x, - y, - }); - }; - const virtualElement: PositioningVirtualElement = { - getBoundingClientRect: getRect(100, 100), - }; - menuPositiningRef.current?.setTarget(virtualElement); - setPopoverOpen(menuOpen); - }, [menuOpen]); - - React.useEffect(() => { - if (menuOpen && menuPopoverRef.current) { - popoverPositiningRef.current?.setTarget(menuPopoverRef.current); - setPopoverOpen(menuOpen); - // uncomment this to make the relative positioning work - // setTimeout(() => popoverPositiningRef.current?.updatePosition(), 3000); - } - }, [menuOpen]); - - return ( - <> - setPopoverOpen(data.open)} - positioning={{ positioningRef: popoverPositiningRef, position: 'below' }} - > - This is a popover - - setMenuOpen(data.open)} - positioning={{ - positioningRef: menuPositiningRef, - onPositioningEnd: () => popoverPositiningRef.current?.updatePosition(), - }} - > - - - - - - - New - New Window - Open File - Open Folder - - - - - ); -}; + + + New + New Window + Open File + Open Folder + + + +); From 566bd16c4be455bd62b5fbde5885e8d9daff57f3 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Fri, 5 Jan 2024 11:48:54 +0100 Subject: [PATCH 5/6] update doc --- .../Positioning/PositioningListenToUpdates.stories.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react-components/react-components/stories/Concepts/Positioning/PositioningListenToUpdates.stories.tsx b/packages/react-components/react-components/stories/Concepts/Positioning/PositioningListenToUpdates.stories.tsx index 9570ad31ec115a..d248608a0b5a6b 100644 --- a/packages/react-components/react-components/stories/Concepts/Positioning/PositioningListenToUpdates.stories.tsx +++ b/packages/react-components/react-components/stories/Concepts/Positioning/PositioningListenToUpdates.stories.tsx @@ -119,8 +119,8 @@ ListenToUpdates.parameters = { 'This constraint makes it difficult to know exactly when an element has been positioned. In order to listen', 'to position updates you can use the `onPositioningEnd` callback.', '', - '> ⚠️ _Very few use cases would actually require listening to position updates. Please renderer that_', - '_there is a difference between **open/close state** which is normally handled in React_', + '> ⚠️ _Very few use cases would actually require listening to position updates. Please remember that_', + '_there is a difference between this and the **open/close state** which is normally handled in React_', ].join('\n'), }, }, From 8fc8bc075be6c4d537d629b315d33eb7588a7499 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Mon, 8 Jan 2024 13:11:13 +0000 Subject: [PATCH 6/6] update doc --- .../Concepts/Positioning/PositioningListenToUpdates.stories.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-components/react-components/stories/Concepts/Positioning/PositioningListenToUpdates.stories.tsx b/packages/react-components/react-components/stories/Concepts/Positioning/PositioningListenToUpdates.stories.tsx index d248608a0b5a6b..53522edcf3faed 100644 --- a/packages/react-components/react-components/stories/Concepts/Positioning/PositioningListenToUpdates.stories.tsx +++ b/packages/react-components/react-components/stories/Concepts/Positioning/PositioningListenToUpdates.stories.tsx @@ -97,7 +97,7 @@ export const ListenToUpdates = () => { const date = new Date(time); return (
- {date.toLocaleTimeString()} Position updated + {date.toLocaleTimeString()} Position updated [{i}]
); })}