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),