From 001fae8451bdea40aec13546be3d11d5653572ad Mon Sep 17 00:00:00 2001 From: Jakub Miskech Date: Mon, 7 Sep 2026 16:41:23 +0200 Subject: [PATCH] fix(react-positioning): improve safe zone hit testing Reduce the safe zone to the container edges facing the pointer and update SVG geometry imperatively to avoid rerenders during pointer movement. Add focused geometry coverage, update the debug story, and include a patch change file. --- ...-982975b8-77a6-4364-bb65-696fed1f0904.json | 7 + .../useSafeZoneArea/SafeZoneArea.styles.ts | 4 +- .../hooks/useSafeZoneArea/SafeZoneArea.tsx | 198 ++++++------------ .../__snapshots__/SafeZoneArea.test.tsx.snap | 56 +---- .../computeOutsideClipPath.test.ts | 71 ++----- .../useSafeZoneArea/computeOutsideClipPath.ts | 8 +- .../useSafeZoneArea/getSafeZonePoints.test.ts | 31 +++ .../useSafeZoneArea/getSafeZonePoints.ts | 50 +++++ .../UseSafeZoneAreaDefault.stories.tsx | 4 +- 9 files changed, 179 insertions(+), 250 deletions(-) create mode 100644 change/@fluentui-react-positioning-982975b8-77a6-4364-bb65-696fed1f0904.json create mode 100644 packages/react-components/react-positioning/library/src/hooks/useSafeZoneArea/getSafeZonePoints.test.ts create mode 100644 packages/react-components/react-positioning/library/src/hooks/useSafeZoneArea/getSafeZonePoints.ts diff --git a/change/@fluentui-react-positioning-982975b8-77a6-4364-bb65-696fed1f0904.json b/change/@fluentui-react-positioning-982975b8-77a6-4364-bb65-696fed1f0904.json new file mode 100644 index 00000000000000..ddfdbd0a88b0a7 --- /dev/null +++ b/change/@fluentui-react-positioning-982975b8-77a6-4364-bb65-696fed1f0904.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "fix: improve safe zone hit testing and avoid rerenders during pointer movement", + "packageName": "@fluentui/react-positioning", + "email": "jakubmiskech@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/packages/react-components/react-positioning/library/src/hooks/useSafeZoneArea/SafeZoneArea.styles.ts b/packages/react-components/react-positioning/library/src/hooks/useSafeZoneArea/SafeZoneArea.styles.ts index 29a83e54bcd180..e0694c929368c0 100644 --- a/packages/react-components/react-positioning/library/src/hooks/useSafeZoneArea/SafeZoneArea.styles.ts +++ b/packages/react-components/react-positioning/library/src/hooks/useSafeZoneArea/SafeZoneArea.styles.ts @@ -20,10 +20,10 @@ export const useStyles = makeStyles({ top: 0, left: 0, }, - triangle: { + safeZone: { pointerEvents: 'auto', }, - triangleDebug: { + safeZoneDebug: { cursor: 'crosshair', fill: `color-mix(in srgb, ${tokens.colorPaletteGreenBackground3} 20%, transparent)`, }, diff --git a/packages/react-components/react-positioning/library/src/hooks/useSafeZoneArea/SafeZoneArea.tsx b/packages/react-components/react-positioning/library/src/hooks/useSafeZoneArea/SafeZoneArea.tsx index 749616c145ea08..63bb5c4fdf3272 100644 --- a/packages/react-components/react-positioning/library/src/hooks/useSafeZoneArea/SafeZoneArea.tsx +++ b/packages/react-components/react-positioning/library/src/hooks/useSafeZoneArea/SafeZoneArea.tsx @@ -13,6 +13,7 @@ import { pointsToSvgPath } from './pointsToSvgPath'; import { useStyles } from './SafeZoneArea.styles'; import type { Point } from './types'; import { computeOutsideClipPath } from './computeOutsideClipPath'; +import { getSafeZonePoints } from './getSafeZonePoints'; export type SafeZoneAreaImperativeHandle = { updateSVG: (options: { containerRect: DOMRect; targetRect: DOMRect; mouseCoordinates: Point }) => void; @@ -35,55 +36,13 @@ export type SafeZoneAreaProps = { stateStore: ReturnType; }; -/** - * @internal - */ -type SafeZoneAreaState = { - containerRect: DOMRect; - targetRect: DOMRect; - mouseCoordinates: Point; -}; - -// --- - -const EMPTY_RECT: DOMRect = { - top: 0, - right: 0, - bottom: 0, - left: 0, - width: 0, - height: 0, - x: 0, - y: 0, - toJSON() { - return ''; - }, -}; - -export function isSameRect(a: DOMRect, b: DOMRect): boolean { - return ( - a.top === b.top && - a.right === b.right && - a.bottom === b.bottom && - a.left === b.left && - a.width === b.width && - a.height === b.height - ); -} - -export function isSameCoordinates(a: Point, b: Point): boolean { - return a[0] === b[0] && a[1] === b[1]; -} - // --- /** * A component that renders a safe zone area with SVG shapes. Uses `useSyncExternalStore` to manage its active state * to avoid causing re-renders in `useSafeZoneArea()` as the hook might be used in host components like `Menu`. * - * Draws two shapes: - * - a triangle that points to the target element which is an actual safe zone - * - a rectangle for a clip path that clips out the target element + * Draws a polygon from the mouse to the facing edges of the container and clips out the target element. * * @internal */ @@ -95,112 +54,79 @@ export const SafeZoneArea = React.memo((props: SafeZoneAreaProps): JSXElement => const active = useSyncExternalStore(stateStore.subscribe, stateStore.isActive); const svgRef = React.useRef(null); - - const [state, setState] = React.useState(() => ({ - containerRect: EMPTY_RECT, - targetRect: EMPTY_RECT, - mouseCoordinates: [0, 0], - })); + const safeZoneRef = React.useRef(null); + const clipPathRef = React.useRef(null); + const rectDebugRef = React.useRef(null); React.useImperativeHandle( props.imperativeRef, () => ({ - updateSVG(newState) { - setState(prevState => { - // Heads up! - // A small optimization to avoid unnecessary re-renders - if ( - isSameRect(prevState.containerRect, newState.containerRect) && - isSameRect(prevState.targetRect, newState.targetRect) && - isSameCoordinates(prevState.mouseCoordinates, newState.mouseCoordinates) - ) { - return prevState; - } - - return newState; + updateSVG({ containerRect, targetRect, mouseCoordinates }) { + const topOffset = Math.min(targetRect.top, containerRect.top); + const leftOffset = Math.min(targetRect.left, containerRect.left); + const bottomOffset = Math.max(targetRect.bottom, containerRect.bottom); + const rightOffset = Math.max(targetRect.right, containerRect.right); + + const containerCorners = getRectCorners(containerRect, [leftOffset, topOffset]); + const targetCorners = getRectCorners(targetRect, [leftOffset, topOffset]); + + // SVG coordinates are relative to its top-left corner. + const relativeMouseCoordinates: Point = [mouseCoordinates[0] - leftOffset, mouseCoordinates[1] - topOffset]; + const mouseAnchor = getMouseAnchor( + containerCorners.topLeft, + containerCorners.bottomRight, + relativeMouseCoordinates, + ); + + const svgWidth = rightOffset - leftOffset; + const svgHeight = bottomOffset - topOffset; + const clipPath = computeOutsideClipPath(svgWidth, svgHeight, { + x: targetCorners.topLeft[0], + y: targetCorners.topLeft[1], + width: targetRect.width, + height: targetRect.height, }); + + if (svgRef.current) { + svgRef.current.style.width = `${svgWidth}px`; + svgRef.current.style.height = `${svgHeight}px`; + svgRef.current.style.transform = `translate(${leftOffset}px, ${topOffset}px)`; + } + + const safeZonePoints = getSafeZonePoints(mouseAnchor, containerCorners); + safeZoneRef.current?.setAttribute('d', safeZonePoints.length > 0 ? pointsToSvgPath(safeZonePoints) : ''); + clipPathRef.current?.setAttribute('d', clipPath); + rectDebugRef.current?.setAttribute('d', clipPath); }, }), [], ); - const { containerRect, targetRect, mouseCoordinates } = state; - - const topOffset = Math.min(targetRect.top, containerRect.top); - const leftOffset = Math.min(targetRect.left, containerRect.left); - const bottomOffset = Math.max(targetRect.bottom, containerRect.bottom); - const rightOffset = Math.max(targetRect.right, containerRect.right); - - // --- - - const containerCorners = getRectCorners(containerRect, [leftOffset, topOffset]); - const targetCorners = getRectCorners(targetRect, [leftOffset, topOffset]); - - // Heads up! - // The SVG coordinate system starts at the top-left corner of the SVG element, - // so we need to adjust the mouse coordinates relative to the SVG's top-left corner. - const relativeMouseCoordinates: Point = [mouseCoordinates[0] - leftOffset, mouseCoordinates[1] - topOffset]; - const mouseAnchor = getMouseAnchor(containerCorners.topLeft, containerCorners.bottomRight, relativeMouseCoordinates); - - const triangleA = [mouseAnchor, containerCorners.topLeft, containerCorners.topRight]; - const triangleB = [mouseAnchor, containerCorners.topRight, containerCorners.bottomRight]; - const triangleC = [mouseAnchor, containerCorners.bottomRight, containerCorners.bottomLeft]; - const triangleD = [mouseAnchor, containerCorners.bottomLeft, containerCorners.topLeft]; - - const svgWidth = rightOffset - leftOffset; - const svgHeight = bottomOffset - topOffset; - - const clipPath = computeOutsideClipPath( - svgWidth, - svgHeight, - { - x: targetCorners.topLeft[0], - y: targetCorners.topLeft[1], - width: targetRect.width, - height: targetRect.height, - }, - { - x: containerCorners.topLeft[0], - y: containerCorners.topLeft[1], - width: containerRect.width, - height: containerRect.height, - }, - ); - return (
- {active ? ( - + - - - - - - - - - - - - {debug && } - - ) : null} + + + + + + + + {debug && } +
); }); diff --git a/packages/react-components/react-positioning/library/src/hooks/useSafeZoneArea/__snapshots__/SafeZoneArea.test.tsx.snap b/packages/react-components/react-positioning/library/src/hooks/useSafeZoneArea/__snapshots__/SafeZoneArea.test.tsx.snap index 947867ddab5f49..cb2945e43456f8 100644 --- a/packages/react-components/react-positioning/library/src/hooks/useSafeZoneArea/__snapshots__/SafeZoneArea.test.tsx.snap +++ b/packages/react-components/react-positioning/library/src/hooks/useSafeZoneArea/__snapshots__/SafeZoneArea.test.tsx.snap @@ -12,28 +12,19 @@ exports[`SafeZoneArea updateSVGs updates SVGs 1`] = ` clip-path="url(#fui-_r_0_)" > - - - `; @@ -49,29 +40,20 @@ exports[`SafeZoneArea updateSVGs updates SVGs 2`] = ` class="" clip-path="url(#fui-_r_1_)" > - - - `; @@ -87,15 +69,6 @@ exports[`SafeZoneArea updateSVGs updates SVGs 3`] = ` class="" clip-path="url(#fui-_r_2_)" > - - - @@ -104,12 +77,12 @@ exports[`SafeZoneArea updateSVGs updates SVGs 3`] = ` id="fui-_r_2_" > `; @@ -126,28 +99,19 @@ exports[`SafeZoneArea updateSVGs updates SVGs 4`] = ` clip-path="url(#fui-_r_3_)" > - - - `; diff --git a/packages/react-components/react-positioning/library/src/hooks/useSafeZoneArea/computeOutsideClipPath.test.ts b/packages/react-components/react-positioning/library/src/hooks/useSafeZoneArea/computeOutsideClipPath.test.ts index a20c00fa079448..f27706b1c81fe8 100644 --- a/packages/react-components/react-positioning/library/src/hooks/useSafeZoneArea/computeOutsideClipPath.test.ts +++ b/packages/react-components/react-positioning/library/src/hooks/useSafeZoneArea/computeOutsideClipPath.test.ts @@ -5,61 +5,43 @@ describe('computeOutsideClipPath', () => { const svgWidth = 1000; const svgHeight = 800; const targetRect = { x: 100, y: 100, width: 200, height: 150 }; - const containerRect = { x: 400, y: 400, width: 300, height: 200 }; + const pathData = computeOutsideClipPath(svgWidth, svgHeight, targetRect); - const pathData = computeOutsideClipPath(svgWidth, svgHeight, targetRect, containerRect); - - expect(pathData).toBe( - [ - 'M 0,0 H 1000 V 800 H 0 Z ', - 'M 100,100 V 250 H 300 V 100 H 100 Z ', - 'M 400,400 V 600 H 700 V 400 H 400 Z ', - ].join(''), - ); + expect(pathData).toBe(['M 0,0 H 1000 V 800 H 0 Z ', 'M 100,100 V 250 H 300 V 100 H 100 Z '].join('')); }); it('should handle zero-sized SVG dimensions', () => { const svgWidth = 0; const svgHeight = 0; const targetRect = { x: 10, y: 10, width: 50, height: 50 }; - const containerRect = { x: 100, y: 100, width: 50, height: 50 }; - - const pathData = computeOutsideClipPath(svgWidth, svgHeight, targetRect, containerRect); + const pathData = computeOutsideClipPath(svgWidth, svgHeight, targetRect); - expect(pathData).toBe( - ['M 0,0 H 0 V 0 H 0 Z ', 'M 10,10 V 60 H 60 V 10 H 10 Z ', 'M 100,100 V 150 H 150 V 100 H 100 Z '].join(''), - ); + expect(pathData).toBe(['M 0,0 H 0 V 0 H 0 Z ', 'M 10,10 V 60 H 60 V 10 H 10 Z '].join('')); }); it('should skip rectangles with zero width', () => { const svgWidth = 1000; const svgHeight = 800; const targetRect = { x: 100, y: 100, width: 0, height: 150 }; - const containerRect = { x: 400, y: 400, width: 300, height: 200 }; - - const pathData = computeOutsideClipPath(svgWidth, svgHeight, targetRect, containerRect); + const pathData = computeOutsideClipPath(svgWidth, svgHeight, targetRect); - expect(pathData).toBe(['M 0,0 H 1000 V 800 H 0 Z ', 'M 400,400 V 600 H 700 V 400 H 400 Z '].join('')); + expect(pathData).toBe('M 0,0 H 1000 V 800 H 0 Z '); }); it('should skip rectangles with zero height', () => { const svgWidth = 1000; const svgHeight = 800; const targetRect = { x: 100, y: 100, width: 200, height: 0 }; - const containerRect = { x: 400, y: 400, width: 300, height: 200 }; - - const pathData = computeOutsideClipPath(svgWidth, svgHeight, targetRect, containerRect); + const pathData = computeOutsideClipPath(svgWidth, svgHeight, targetRect); - expect(pathData).toBe(['M 0,0 H 1000 V 800 H 0 Z ', 'M 400,400 V 600 H 700 V 400 H 400 Z '].join('')); + expect(pathData).toBe('M 0,0 H 1000 V 800 H 0 Z '); }); it('should skip rectangles with negative dimensions', () => { const svgWidth = 1000; const svgHeight = 800; const targetRect = { x: 100, y: 100, width: 200, height: 150 }; - const containerRect = { x: 400, y: 400, width: -10, height: -20 }; - - const pathData = computeOutsideClipPath(svgWidth, svgHeight, targetRect, containerRect); + const pathData = computeOutsideClipPath(svgWidth, svgHeight, targetRect); expect(pathData).toBe(['M 0,0 H 1000 V 800 H 0 Z ', 'M 100,100 V 250 H 300 V 100 H 100 Z '].join('')); }); @@ -68,33 +50,10 @@ describe('computeOutsideClipPath', () => { const svgWidth = 1000.5; const svgHeight = 800.25; const targetRect = { x: 100.75, y: 100.5, width: 200.25, height: 150.5 }; - const containerRect = { x: 400.25, y: 400.75, width: 300.5, height: 200.25 }; - - const pathData = computeOutsideClipPath(svgWidth, svgHeight, targetRect, containerRect); + const pathData = computeOutsideClipPath(svgWidth, svgHeight, targetRect); expect(pathData).toBe( - [ - 'M 0,0 H 1000.5 V 800.25 H 0 Z ', - 'M 100.75,100.5 V 251 H 301 V 100.5 H 100.75 Z ', - 'M 400.25,400.75 V 601 H 700.75 V 400.75 H 400.25 Z ', - ].join(''), - ); - }); - - it('should handle overlapping rectangles correctly', () => { - const svgWidth = 1000; - const svgHeight = 800; - const targetRect = { x: 100, y: 100, width: 400, height: 400 }; - const containerRect = { x: 200, y: 200, width: 400, height: 400 }; - - const pathData = computeOutsideClipPath(svgWidth, svgHeight, targetRect, containerRect); - - expect(pathData).toBe( - [ - 'M 0,0 H 1000 V 800 H 0 Z ', - 'M 100,100 V 500 H 500 V 100 H 100 Z ', - 'M 200,200 V 600 H 600 V 200 H 200 Z ', - ].join(''), + ['M 0,0 H 1000.5 V 800.25 H 0 Z ', 'M 100.75,100.5 V 251 H 301 V 100.5 H 100.75 Z '].join(''), ); }); @@ -102,12 +61,8 @@ describe('computeOutsideClipPath', () => { const svgWidth = 1000; const svgHeight = 800; const targetRect = { x: 0, y: 0, width: 200, height: 150 }; - const containerRect = { x: 800, y: 650, width: 200, height: 150 }; - - const pathData = computeOutsideClipPath(svgWidth, svgHeight, targetRect, containerRect); + const pathData = computeOutsideClipPath(svgWidth, svgHeight, targetRect); - expect(pathData).toBe( - ['M 0,0 H 1000 V 800 H 0 Z ', 'M 0,0 V 150 H 200 V 0 H 0 Z ', 'M 800,650 V 800 H 1000 V 650 H 800 Z '].join(''), - ); + expect(pathData).toBe(['M 0,0 H 1000 V 800 H 0 Z ', 'M 0,0 V 150 H 200 V 0 H 0 Z '].join('')); }); }); diff --git a/packages/react-components/react-positioning/library/src/hooks/useSafeZoneArea/computeOutsideClipPath.ts b/packages/react-components/react-positioning/library/src/hooks/useSafeZoneArea/computeOutsideClipPath.ts index 4bfe74bc2c961a..7b406faef37082 100644 --- a/packages/react-components/react-positioning/library/src/hooks/useSafeZoneArea/computeOutsideClipPath.ts +++ b/packages/react-components/react-positioning/library/src/hooks/useSafeZoneArea/computeOutsideClipPath.ts @@ -18,7 +18,7 @@ function drawRectangle(rect: { x: number; y: number; width: number; height: numb } /** - * Computes a clip path that covers the area outside multiple rectangles. + * Computes a clip path that covers the area outside a rectangle. * * @internal */ @@ -26,16 +26,12 @@ export function computeOutsideClipPath( svgWidth: number, svgHeight: number, targetRect: { x: number; y: number; width: number; height: number }, - containerRect: { x: number; y: number; width: number; height: number }, ): string { let pathData = `M 0,0 H ${svgWidth} V ${svgHeight} H 0 Z `; - // For each rectangle, add a subpath that "cuts out" the rectangle - // The trick is to draw each rectangle in the counterclockwise direction - // which creates a "hole" in the main path + // Draw the rectangle counterclockwise to create a hole in the main path. pathData += drawRectangle(targetRect); - pathData += drawRectangle(containerRect); return pathData; } diff --git a/packages/react-components/react-positioning/library/src/hooks/useSafeZoneArea/getSafeZonePoints.test.ts b/packages/react-components/react-positioning/library/src/hooks/useSafeZoneArea/getSafeZonePoints.test.ts new file mode 100644 index 00000000000000..59e36cdf1f9f10 --- /dev/null +++ b/packages/react-components/react-positioning/library/src/hooks/useSafeZoneArea/getSafeZonePoints.test.ts @@ -0,0 +1,31 @@ +import { getSafeZonePoints } from './getSafeZonePoints'; +import type { Point } from './types'; + +const containerCorners = { + topLeft: [0, 0] satisfies Point, + topRight: [100, 0] satisfies Point, + bottomRight: [100, 100] satisfies Point, + bottomLeft: [0, 100] satisfies Point, +}; + +describe('getSafeZonePoints', () => { + it.each([ + { anchor: [-50, 50] satisfies Point, corners: ['bottomLeft', 'topLeft'] }, + { anchor: [150, 50] satisfies Point, corners: ['topRight', 'bottomRight'] }, + { anchor: [50, -50] satisfies Point, corners: ['topLeft', 'topRight'] }, + { anchor: [50, 150] satisfies Point, corners: ['bottomRight', 'bottomLeft'] }, + { anchor: [-50, -50] satisfies Point, corners: ['bottomLeft', 'topLeft', 'topRight'] }, + { anchor: [150, -50] satisfies Point, corners: ['topLeft', 'topRight', 'bottomRight'] }, + { anchor: [-50, 150] satisfies Point, corners: ['topLeft', 'bottomLeft', 'bottomRight'] }, + { anchor: [150, 150] satisfies Point, corners: ['topRight', 'bottomRight', 'bottomLeft'] }, + ])('returns the facing corners for anchor $anchor', ({ anchor, corners }) => { + expect(getSafeZonePoints(anchor, containerCorners)).toEqual([ + anchor, + ...corners.map(corner => containerCorners[corner as keyof typeof containerCorners]), + ]); + }); + + it('returns no safe zone when the anchor is inside the container', () => { + expect(getSafeZonePoints([50, 50], containerCorners)).toEqual([]); + }); +}); diff --git a/packages/react-components/react-positioning/library/src/hooks/useSafeZoneArea/getSafeZonePoints.ts b/packages/react-components/react-positioning/library/src/hooks/useSafeZoneArea/getSafeZonePoints.ts new file mode 100644 index 00000000000000..d1beacd4cdc360 --- /dev/null +++ b/packages/react-components/react-positioning/library/src/hooks/useSafeZoneArea/getSafeZonePoints.ts @@ -0,0 +1,50 @@ +import type { Point } from './types'; + +type RectCorners = Record<'topLeft' | 'topRight' | 'bottomRight' | 'bottomLeft', Point>; + +/** + * Returns the mouse anchor and the container corners facing it. + * + * @internal + */ +export function getSafeZonePoints(mouseAnchor: Point, containerCorners: RectCorners): Point[] { + const { topLeft, topRight, bottomRight, bottomLeft } = containerCorners; + const isAbove = mouseAnchor[1] < topLeft[1]; + const isBelow = mouseAnchor[1] > bottomLeft[1]; + const isLeft = mouseAnchor[0] < topLeft[0]; + const isRight = mouseAnchor[0] > topRight[0]; + + if (isAbove) { + if (isLeft) { + return [mouseAnchor, bottomLeft, topLeft, topRight]; + } + + if (isRight) { + return [mouseAnchor, topLeft, topRight, bottomRight]; + } + + return [mouseAnchor, topLeft, topRight]; + } + + if (isBelow) { + if (isLeft) { + return [mouseAnchor, topLeft, bottomLeft, bottomRight]; + } + + if (isRight) { + return [mouseAnchor, topRight, bottomRight, bottomLeft]; + } + + return [mouseAnchor, bottomRight, bottomLeft]; + } + + if (isLeft) { + return [mouseAnchor, bottomLeft, topLeft]; + } + + if (isRight) { + return [mouseAnchor, topRight, bottomRight]; + } + + return []; +} diff --git a/packages/react-components/react-positioning/stories/src/UseSafeZoneArea/UseSafeZoneAreaDefault.stories.tsx b/packages/react-components/react-positioning/stories/src/UseSafeZoneArea/UseSafeZoneAreaDefault.stories.tsx index b64b0f4dbd14e4..e3fc0549631571 100644 --- a/packages/react-components/react-positioning/stories/src/UseSafeZoneArea/UseSafeZoneAreaDefault.stories.tsx +++ b/packages/react-components/react-positioning/stories/src/UseSafeZoneArea/UseSafeZoneAreaDefault.stories.tsx @@ -94,8 +94,8 @@ export const UseSafeZoneAreaDefault = (props: UseSafeZoneOptions): JSXElement => const [targetWidth, setTargetWidth] = React.useState<'small' | 'medium' | 'large'>('large'); const safeZoneArea = useSafeZoneArea({ - debug: true, - timeout: 100000, + debug, + timeout: 5000, }); const positioning = usePositioning({ ...resolvePositioningShorthand(position),