diff --git a/.changeset/anchored-position-scroll-recalculation.md b/.changeset/anchored-position-scroll-recalculation.md new file mode 100644 index 00000000000..e29a6819a1b --- /dev/null +++ b/.changeset/anchored-position-scroll-recalculation.md @@ -0,0 +1,5 @@ +--- +'@primer/react': patch +--- + +useAnchoredPosition: recalculate overlay position when any scrollable ancestor (or the window) is scrolled. diff --git a/packages/react/src/AnchoredOverlay/AnchoredOverlay.dev.stories.tsx b/packages/react/src/AnchoredOverlay/AnchoredOverlay.dev.stories.tsx index 1d79a4569c6..3a9a149b282 100644 --- a/packages/react/src/AnchoredOverlay/AnchoredOverlay.dev.stories.tsx +++ b/packages/react/src/AnchoredOverlay/AnchoredOverlay.dev.stories.tsx @@ -4,7 +4,7 @@ import React, {useState} from 'react' import {Button} from '../Button' import {AnchoredOverlay} from '.' import {Stack} from '../Stack' -import {Dialog, Spinner} from '..' +import {Dialog, Spinner, Text} from '..' const meta = { title: 'Components/AnchoredOverlay/Dev', @@ -59,6 +59,128 @@ export const RepositionAfterContentGrows = () => { ) } +export const ScrollRecalculation = () => { + const [open, setOpen] = useState(false) + + return ( +
+ + + How to test (scrollable container): + + + 1. Scroll down inside the bordered box until you see the "Open overlay" button +
+ 2. Click the button to open the overlay +
+ 3. Scroll inside the box again +
+ 4. Expected: the overlay stays visually attached to the button +
+ 5. Bug (without fix): the overlay stays at its initial absolute position and detaches from + the button +
+
+ +
+ {/* Spacer to push button below the fold */} +
+ + ↓ Scroll down to find the trigger button + +
+ +
+ setOpen(true)} + onClose={() => setOpen(false)} + renderAnchor={props => } + overlayProps={{ + role: 'dialog', + 'aria-modal': true, + 'aria-label': 'Scroll recalculation demo', + }} + focusZoneSettings={{disabled: true}} + preventOverflow={false} + > +
+ This overlay should stay attached to the button as you scroll. +
+
+
+ + {/* Spacer below the button so there's room to scroll further */} +
+
+
+ ) +} + +export const WindowScrollRecalculation = () => { + const [open, setOpen] = useState(false) + + return ( +
+ + + How to test (window scroll): + + + 1. Scroll down the page until you see the "Open overlay" button +
+ 2. Click the button to open the overlay +
+ 3. Scroll the page again +
+ 4. Expected: the overlay stays visually attached to the button +
+ 5. Bug (without fix): the overlay stays at its initial absolute position and detaches from + the button +
+
+ + {/* Spacer to push button below the fold */} +
+ + ↓ Scroll down to find the trigger button + +
+ +
+ setOpen(true)} + onClose={() => setOpen(false)} + renderAnchor={props => } + overlayProps={{ + role: 'dialog', + 'aria-modal': true, + 'aria-label': 'Window scroll recalculation demo', + }} + focusZoneSettings={{disabled: true}} + preventOverflow={false} + > +
+ This overlay should stay attached to the button as you scroll the page. +
+
+
+ + {/* Spacer below the button so there's room to scroll further */} +
+
+ ) +} + export const RepositionAfterContentGrowsWithinDialog = () => { const [open, setOpen] = useState(false) const [loading, setLoading] = useState(true) diff --git a/packages/react/src/hooks/__tests__/useAnchoredPosition.test.tsx b/packages/react/src/hooks/__tests__/useAnchoredPosition.test.tsx index d894305aa18..258db5ec4bb 100644 --- a/packages/react/src/hooks/__tests__/useAnchoredPosition.test.tsx +++ b/packages/react/src/hooks/__tests__/useAnchoredPosition.test.tsx @@ -1,5 +1,5 @@ -import {render, waitFor} from '@testing-library/react' -import {it, expect, vi} from 'vitest' +import {render, waitFor, act} from '@testing-library/react' +import {it, expect, vi, describe} from 'vitest' import React from 'react' import {useAnchoredPosition} from '../../hooks/useAnchoredPosition' @@ -34,3 +34,74 @@ it('should should return a position', async () => { `) }) }) + +describe('scroll recalculation', () => { + it('should recalculate position when window scrolls', async () => { + const cb = vi.fn() + render() + + // Wait for initial position calculation to stabilise + await waitFor(() => { + expect(cb).toHaveBeenCalledTimes(2) + }) + + // Wait a tick to let any pending effects settle + await new Promise(resolve => setTimeout(resolve, 50)) + const callCountBefore = cb.mock.calls.length + + // Simulate a window scroll event + act(() => { + window.dispatchEvent(new Event('scroll')) + }) + + // Wait for rAF-throttled handler to fire and trigger re-render + await new Promise(resolve => setTimeout(resolve, 50)) + + expect(cb.mock.calls.length).toBeGreaterThan(callCountBefore) + }) + + it('should recalculate position when a scrollable ancestor scrolls', async () => { + const ScrollableComponent = ({ + callback, + }: { + callback: (hookReturnValue: ReturnType) => void + }) => { + const floatingElementRef = React.useRef(null) + const anchorElementRef = React.useRef(null) + callback(useAnchoredPosition({floatingElementRef, anchorElementRef})) + return ( +
+
+
+
+
+
+ ) + } + + const cb = vi.fn() + const {container} = render() + + await waitFor(() => { + expect(cb).toHaveBeenCalledTimes(2) + }) + + // Wait a tick to let any pending effects settle + await new Promise(resolve => setTimeout(resolve, 50)) + const callCountBefore = cb.mock.calls.length + const scrollContainer = container.firstElementChild! + + // Simulate scroll on the scrollable ancestor + act(() => { + scrollContainer.dispatchEvent(new Event('scroll')) + }) + + // Wait for rAF-throttled handler to fire and trigger re-render + await new Promise(resolve => setTimeout(resolve, 50)) + + expect(cb.mock.calls.length).toBeGreaterThan(callCountBefore) + }) +}) diff --git a/packages/react/src/hooks/useAnchoredPosition.ts b/packages/react/src/hooks/useAnchoredPosition.ts index 32777aad1d7..7d89bdfd595 100644 --- a/packages/react/src/hooks/useAnchoredPosition.ts +++ b/packages/react/src/hooks/useAnchoredPosition.ts @@ -5,6 +5,27 @@ import {useProvidedRefOrCreate} from './useProvidedRefOrCreate' import {useResizeObserver} from './useResizeObserver' import useLayoutEffect from '../utils/useIsomorphicLayoutEffect' +/** + * Returns all scrollable ancestor elements of the given element, plus the window. + * An element is scrollable if its computed overflow/overflow-x/overflow-y is + * 'auto', 'scroll', or 'overlay'. + */ +function getScrollableAncestors(element: Element): Array { + const scrollables: Array = [] + let current = element.parentElement + while (current) { + const style = getComputedStyle(current) + const overflowY = style.overflowY + const overflowX = style.overflowX + if (/auto|scroll|overlay/.test(overflowY) || /auto|scroll|overlay/.test(overflowX)) { + scrollables.push(current) + } + current = current.parentElement + } + scrollables.push(window) + return scrollables +} + export interface AnchoredPositionHookSettings extends Partial { floatingElementRef?: React.RefObject anchorElementRef?: React.RefObject @@ -100,6 +121,37 @@ export function useAnchoredPosition( useResizeObserver(updatePosition) // watches for changes in window size useResizeObserver(updatePosition, floatingElementRef as React.RefObject) // watches for changes in floating element size + // Recalculate position when any scrollable ancestor of the anchor scrolls. + // Uses requestAnimationFrame to avoid layout thrashing during scroll. + React.useEffect(() => { + const anchorEl = anchorElementRef.current + if (!anchorEl) return + + let rafId: number | null = null + const handleScroll = () => { + if (rafId !== null) return + rafId = requestAnimationFrame(() => { + rafId = null + updatePosition() + }) + } + + const scrollables = getScrollableAncestors(anchorEl) + for (const scrollable of scrollables) { + // eslint-disable-next-line github/prefer-observers -- IntersectionObserver cannot detect continuous scroll position changes needed for repositioning + scrollable.addEventListener('scroll', handleScroll) + } + + return () => { + for (const scrollable of scrollables) { + scrollable.removeEventListener('scroll', handleScroll) + } + if (rafId !== null) { + cancelAnimationFrame(rafId) + } + } + }, [anchorElementRef, updatePosition]) + return { floatingElementRef, anchorElementRef,