diff --git a/.changeset/underline-nav-shared-overflow-observer.md b/.changeset/underline-nav-shared-overflow-observer.md index 087b9fc0a1c..da950989c4e 100644 --- a/.changeset/underline-nav-shared-overflow-observer.md +++ b/.changeset/underline-nav-shared-overflow-observer.md @@ -2,4 +2,4 @@ "@primer/react": patch --- -Internal: `UnderlineNav` and `ActionBar` overflow detection now shares a single `IntersectionObserver` per component (owned by the descendant registry) instead of creating one observer per item, and the registry coalesces rebuilds so multiple items crossing the overflow boundary in the same frame trigger a single rebuild. This reduces re-render churn and observer allocations during resize. No public API changes. +Internal: `UnderlineNav` and `ActionBar` overflow detection now uses a shared root-scoped `IntersectionObserver` per component, and the registry coalesces same-frame rebuilds. This reduces observer churn during resize with no public API changes. diff --git a/packages/react/src/ActionBar/ActionBar.tsx b/packages/react/src/ActionBar/ActionBar.tsx index f61dfdd5b03..d0827364ca5 100644 --- a/packages/react/src/ActionBar/ActionBar.tsx +++ b/packages/react/src/ActionBar/ActionBar.tsx @@ -139,11 +139,9 @@ export type ActionBarMenuProps = { returnFocusRef?: React.RefObject } & IconButtonProps -// Items opt into a single shared IntersectionObserver (threshold 0.75) via `useRegisterOverflowObserver` instead of -// each item creating its own observer. 0.75 is used (rather than 1) because in some scenarios a threshold of 1 doesn't -// trigger correctly - the browser still thinks a tiny bit of the button is not visible since the container height is -// exactly the button height. See `useActionBarItem`. -const ActionBarItemsRegistry = createDescendantRegistry({overflow: {threshold: 0.75}}) +// Items opt into a single shared IntersectionObserver via `useRegisterOverflowObserver` instead of each item creating +// its own observer. +const ActionBarItemsRegistry = createDescendantRegistry({overflow: {}}) const FOCUSABLE_ITEM_SELECTOR = ':is(button, a, input, [tabindex]):not(:disabled):not([data-overflowing]):not([data-more-button-inactive])' @@ -207,6 +205,7 @@ export const ActionBar: React.FC> = ({ gap = 'condensed', }) => { const [childRegistry, setChildRegistry] = ActionBarItemsRegistry.useRegistryState() + const overflowContainerRef = useRef(null) const overflowItems = useMemo( () => @@ -237,10 +236,12 @@ export const ActionBar: React.FC> = ({ data-size={size} data-has-overflow={overflowItems ? overflowItems.length > 0 : undefined} > -
+
{/* An empty first element allows the real first item to wrap to the next line and get clipped. */}
- {children} + + {children} +
@@ -314,12 +315,8 @@ function useActionBarItem(ref: React.RefObject, registryProp const isGroupOverflowing = useContext(ActionBarGroupContext)?.isOverflowing const isInGroup = isGroupOverflowing !== undefined - // Subscribe to the registry's shared IntersectionObserver to detect overflow. The observer is just being used as a - // trigger to re-check `offsetTop > 0`; this is fast and simpler than checking visibility from the observed entry. - // When an item wraps, it moves to the next row which increases its `offsetTop`. - // // There's no need to observe items inside of a group since the entire group overflows at once, so `disabled` skips - // subscription for grouped items (the snapshot returns `false`, matching the previous early-return behavior). + // subscription for grouped items and always reports `false` for the child item itself. const isItemOverflowing = ActionBarItemsRegistry.useRegisterOverflowObserver(ref, {disabled: isInGroup}) const isOverflowing = isGroupOverflowing || isItemOverflowing diff --git a/packages/react/src/UnderlineNav/UnderlineNav.tsx b/packages/react/src/UnderlineNav/UnderlineNav.tsx index ca085e71689..bf95c79284e 100644 --- a/packages/react/src/UnderlineNav/UnderlineNav.tsx +++ b/packages/react/src/UnderlineNav/UnderlineNav.tsx @@ -101,7 +101,7 @@ export const UnderlineNav = forwardRef( data-has-overflow={isOverflowing ? 'true' : undefined} > - + {children} diff --git a/packages/react/src/UnderlineNav/UnderlineNavItem.tsx b/packages/react/src/UnderlineNav/UnderlineNavItem.tsx index e2104a0233e..3acf653a25e 100644 --- a/packages/react/src/UnderlineNav/UnderlineNavItem.tsx +++ b/packages/react/src/UnderlineNav/UnderlineNavItem.tsx @@ -22,9 +22,8 @@ export const UnderlineNavItem = forwardRef((allProps, forwardedRef) => { const {loadingCounters} = useContext(UnderlineNavContext) - // Subscribe to the registry's shared IntersectionObserver. The observer is just being used as a trigger to - // re-check `offsetTop > 0`; this is fast and simpler than checking visibility from the observed entry. When an - // item wraps, it moves to the next (clipped) row, which increases its `offsetTop`. + // Observe the wrapping `
  • ` directly so a root-scoped IntersectionObserver can detect when the item is clipped + // onto the hidden next row. const isOverflowing = UnderlineNavItemsRegistry.useRegisterOverflowObserver(ref) UnderlineNavItemsRegistry.useRegisterDescendant(isOverflowing ? allProps : null) diff --git a/packages/react/src/UnderlineNav/UnderlineNavItemsRegistry.ts b/packages/react/src/UnderlineNav/UnderlineNavItemsRegistry.ts index 75a3b525863..f85bcd608c6 100644 --- a/packages/react/src/UnderlineNav/UnderlineNavItemsRegistry.ts +++ b/packages/react/src/UnderlineNav/UnderlineNavItemsRegistry.ts @@ -56,9 +56,9 @@ export type UnderlineNavItemProps = { /** * Registry of currently-overflowing underline items. If an item is not overflowing, its value will be `null`. * - * Items opt into a single shared IntersectionObserver (threshold 1) via `useRegisterOverflowObserver` instead of - * each item creating its own observer. + * Items opt into a single shared IntersectionObserver via `useRegisterOverflowObserver` instead of each item creating + * its own observer. */ export const UnderlineNavItemsRegistry = createDescendantRegistry({ - overflow: {threshold: 1}, + overflow: {}, }) diff --git a/packages/react/src/utils/__tests__/descendant-registry.test.tsx b/packages/react/src/utils/__tests__/descendant-registry.test.tsx index 39fa167a191..227336e6dd8 100644 --- a/packages/react/src/utils/__tests__/descendant-registry.test.tsx +++ b/packages/react/src/utils/__tests__/descendant-registry.test.tsx @@ -233,7 +233,7 @@ describe('createDescendantRegistry coalesced rebuilds', () => { - , + ) } @@ -282,7 +282,7 @@ describe('createDescendantRegistry coalesced rebuilds', () => { - , + ) } @@ -302,13 +302,16 @@ describe('createDescendantRegistry coalesced rebuilds', () => { describe('createDescendantRegistry shared IntersectionObserver', () => { // Capture every IntersectionObserver instance and its observed elements so we can assert a single shared observer // is used and drive its callback manually. + /** Minimal `IntersectionObserverEntry` shape needed to drive overflow updates in tests. */ + type MockEntry = Pick type FakeObserver = { callback: IntersectionObserverCallback + options?: IntersectionObserverInit observed: Set observe: ReturnType unobserve: ReturnType disconnect: ReturnType - trigger: () => void + emit: (entries: MockEntry[]) => void } let observers: FakeObserver[] = [] @@ -316,6 +319,7 @@ describe('createDescendantRegistry shared IntersectionObserver', () => { observers = [] class MockIntersectionObserver { callback: IntersectionObserverCallback + options?: IntersectionObserverInit observed = new Set() observe = vi.fn((el: Element) => { this.observed.add(el) @@ -326,14 +330,15 @@ describe('createDescendantRegistry shared IntersectionObserver', () => { disconnect = vi.fn(() => { this.observed.clear() }) - constructor(callback: IntersectionObserverCallback) { + constructor(callback: IntersectionObserverCallback, options?: IntersectionObserverInit) { this.callback = callback + this.options = options observers.push(this as unknown as FakeObserver) - ;(this as unknown as FakeObserver).trigger = () => { - const entries = Array.from(this.observed).map( - target => ({target}) as IntersectionObserverEntry, + ;(this as unknown as FakeObserver).emit = entries => { + this.callback( + entries.map(entry => entry as IntersectionObserverEntry), + this as unknown as IntersectionObserver, ) - this.callback(entries, this as unknown as IntersectionObserver) } } takeRecords() { @@ -352,12 +357,20 @@ describe('createDescendantRegistry shared IntersectionObserver', () => { * assert how many were updated by a single observer callback (the fan-out). */ function createOverflowRegistry() { - const {Provider, useRegistryState, useRegisterDescendant, useRegisterOverflowObserver} = - createDescendantRegistry({overflow: {threshold: 1}}) + const {Provider, useRegistryState, useRegisterDescendant, useRegisterOverflowObserver} = createDescendantRegistry< + string | null + >({overflow: {}}) function RegistryParent({children}: {children: React.ReactNode}) { const [, setRegistry] = useRegistryState() - return {children} + const rootRef = useRef(null) + return ( +
    + + {children} + +
    + ) } function Item({value}: {value: string}) { @@ -370,10 +383,10 @@ describe('createDescendantRegistry shared IntersectionObserver', () => { return {RegistryParent, Item} } - it('creates a single shared observer for all items rather than one per item', () => { + it('creates a single shared observer for all items with the provided root element', () => { const {RegistryParent, Item} = createOverflowRegistry() - render( + const {getByTestId} = render( @@ -383,6 +396,7 @@ describe('createDescendantRegistry shared IntersectionObserver', () => { // Exactly one observer instance, observing all three elements. expect(observers).toHaveLength(1) + expect(observers[0].options?.root).toBe(getByTestId('observer-root')) expect(observers[0].observed.size).toBe(3) }) @@ -397,15 +411,17 @@ describe('createDescendantRegistry shared IntersectionObserver', () => { , ) - // Simulate all items wrapping to a clipped row (offsetTop > 0). jsdom reports 0 for offsetTop, so stub it. const items = ['a', 'b', 'c'].map(v => getByTestId(`item-${v}`)) - for (const el of items) { - Object.defineProperty(el, 'offsetTop', {configurable: true, value: 10}) - } // One observer notification should update all three items via fan-out. act(() => { - observers[0].trigger() + observers[0].emit( + items.map(target => ({ + target, + isIntersecting: false, + intersectionRatio: 0, + })), + ) }) for (const el of items) { @@ -413,6 +429,34 @@ describe('createDescendantRegistry shared IntersectionObserver', () => { } }) + it('updates an item from the observed entry without reading layout', () => { + const {RegistryParent, Item} = createOverflowRegistry() + + const {getByTestId} = render( + + + , + ) + + const item = getByTestId('item-a') + Object.defineProperty(item, 'offsetTop', { + configurable: true, + get() { + throw new Error('offsetTop should not be read') + }, + }) + + act(() => { + observers[0].emit([{target: item, isIntersecting: false, intersectionRatio: 0}]) + }) + expect(item).toHaveAttribute('data-overflowing', 'true') + + act(() => { + observers[0].emit([{target: item, isIntersecting: true, intersectionRatio: 1}]) + }) + expect(item).toHaveAttribute('data-overflowing', 'false') + }) + it('unobserves an element from the shared observer when its item unmounts', async () => { const {RegistryParent, Item} = createOverflowRegistry() @@ -441,6 +485,7 @@ describe('createDescendantRegistry shared IntersectionObserver', () => { // Still the same single observer, now observing only the two remaining items. expect(observers).toHaveLength(1) expect(observers[0].observed.size).toBe(2) + expect(observers[0].unobserve).toHaveBeenCalled() }) it('disconnects the shared observer when the provider unmounts', async () => { diff --git a/packages/react/src/utils/descendant-registry.tsx b/packages/react/src/utils/descendant-registry.tsx index 9aa862a17e4..2b2b86ab81d 100644 --- a/packages/react/src/utils/descendant-registry.tsx +++ b/packages/react/src/utils/descendant-registry.tsx @@ -19,6 +19,8 @@ export interface ProviderProps { children: ReactNode /** State setter from `useRegistryState`. */ setRegistry: Dispatch | undefined>> + /** Clipping container used as the `IntersectionObserver` root for overflow detection. */ + rootRef?: RefObject } interface DescendantRegistryContext { @@ -28,7 +30,7 @@ interface DescendantRegistryContext { } /** Subscribe a single observed element to the shared IntersectionObserver. Returns an unsubscribe function. */ -type ObserveFn = (element: Element, onChange: () => void) => () => void +type ObserveFn = (element: Element, onOverflowChange: (isOverflowing: boolean) => void) => () => void interface OverflowObserverContext { /** Subscribe an element. `null` when no shared observer is configured for this registry. */ @@ -66,14 +68,7 @@ export function createDescendantRegistry(options?: { * Configure a shared IntersectionObserver owned by the `Provider`. When set, descendants can call * `useRegisterOverflowObserver` to subscribe to a single observer rather than each creating their own. */ - overflow?: { - /** - * IntersectionObserver threshold. The observer is only used as a *trigger* to re-check `offsetTop > 0`, - * so the exact value mainly affects reliability at the wrap boundary. - * @default 1 - */ - threshold?: number - } + overflow?: object }) { const Context = createContext>({ register: () => () => {}, @@ -83,7 +78,6 @@ export function createDescendantRegistry(options?: { const ObserverContext = createContext(noopObserve) - const overflowThreshold = options?.overflow?.threshold ?? 1 const overflowEnabled = options?.overflow !== undefined /** @@ -109,13 +103,11 @@ export function createDescendantRegistry(options?: { } /** - * Subscribe an element to the registry's shared IntersectionObserver and derive whether it is currently overflowing - * (i.e. has wrapped to a clipped row, so `offsetTop > 0`). Falls back to a per-item observer if no shared observer is - * configured for this registry, so the hook is always safe to call. + * Subscribe an element to the registry's shared, root-scoped IntersectionObserver and derive whether it is currently + * overflowing from the observed entry. * - * The IntersectionObserver is only used as a cheap *trigger* to re-check `offsetTop`; the actual overflow detection - * relies on `flex-wrap` + `overflow: hidden` pushing items to a clipped second row, not on viewport intersection. - * That's why there is no `root` option on the observer. + * This requires the registry to be created with the `overflow` option so a shared observer is configured. When no + * shared observer is configured the hook is inert and always reports `false`. * * @param ref Ref to the element whose overflow state should be tracked. * @param options.disabled When true, skips observer subscription entirely and always reports `false`. Useful for @@ -124,26 +116,31 @@ export function createDescendantRegistry(options?: { function useRegisterOverflowObserver(ref: RefObject, options?: {disabled?: boolean}) { const disabled = options?.disabled ?? false const {observe} = useContext(ObserverContext) + const isOverflowingRef = useRef(false) const subscribe = useCallback( (onChange: () => void) => { if (disabled) return () => {} const element = ref.current - if (!element) return () => {} - - // Prefer the provider's shared observer; fall back to a local observer when none is configured. - if (observe) return observe(element, onChange) + // The hook only tracks overflow through the provider's shared observer. When no shared observer is configured + // (or the element isn't attached yet) the hook is inert and reports `false`. + if (!element || observe === null) return () => {} + + const updateOverflowState = (isOverflowing: boolean) => { + if (isOverflowing !== isOverflowingRef.current) { + isOverflowingRef.current = isOverflowing + onChange() + } + } - const observer = new IntersectionObserver(() => onChange(), {threshold: overflowThreshold}) - observer.observe(element) - return () => observer.disconnect() + return observe(element, updateOverflowState) }, [ref, observe, disabled], ) return useSyncExternalStore( subscribe, - () => (!disabled && ref.current ? ref.current.offsetTop > 0 : false), + () => (disabled ? false : isOverflowingRef.current), () => false, ) } @@ -151,7 +148,7 @@ export function createDescendantRegistry(options?: { const unsetValue = Symbol('unset') /** Provide context for registering descendant components. This only needs to wrap `children`. */ - function Provider({children, setRegistry}: ProviderProps) { + function Provider({children, setRegistry, rootRef}: ProviderProps) { const workingRegistryRef = useRef | 'queued' | 'idle'>('queued') /** State value to trigger a re-render and force all descendants to re-register. This ensures everything remains ordered. */ @@ -252,7 +249,7 @@ export function createDescendantRegistry(options?: { return ( - {overflowEnabled ? {children} : children} + {overflowEnabled ? {children} : children} ) } @@ -261,57 +258,88 @@ export function createDescendantRegistry(options?: { * Owns a single IntersectionObserver shared by every descendant that calls `useRegisterOverflowObserver`. * Each observed element maps to a set of change callbacks; one observer notification fans out to all of them. */ - function OverflowObserverProvider({children}: {children: ReactNode}) { + function OverflowObserverProvider({children, rootRef}: {children: ReactNode; rootRef?: RefObject}) { // Map of observed element -> set of subscriber callbacks. - const subscribersRef = useRef void>>>(new Map()) + const subscribersRef = useRef void>>>(new Map()) + const observedElementsRef = useRef>(new Set()) const observerRef = useRef(null) + const observerRootRef = useRef(null) // Lazily create the observer on first subscribe so SSR / zero-item renders allocate nothing. const getObserver = useCallback(() => { - if (observerRef.current) return observerRef.current - if (typeof IntersectionObserver === 'undefined') return null + if (!supportsIntersectionObserver()) return null + if (rootRef && rootRef.current === null) return null + + const root = rootRef?.current ?? null + if (observerRef.current && observerRootRef.current === root) return observerRef.current + + observerRef.current?.disconnect() + observedElementsRef.current.clear() + observerRef.current = new IntersectionObserver( entries => { for (const entry of entries) { const callbacks = subscribersRef.current.get(entry.target) if (!callbacks) continue - for (const cb of callbacks) cb() + const isOverflowing = getIsOverflowing(entry) + for (const cb of callbacks) cb(isOverflowing) } }, - {threshold: overflowThreshold}, + {root, threshold: [0, 1]}, ) + observerRootRef.current = root return observerRef.current - }, []) + }, [rootRef]) + + const observeSubscribedElements = useCallback(() => { + const observer = getObserver() + if (!observer) return + + // When the root ref becomes available or changes, re-check every subscribed element so they are all attached to + // the latest shared observer instance. + for (const element of subscribersRef.current.keys()) { + if (!observedElementsRef.current.has(element)) { + observer.observe(element) + observedElementsRef.current.add(element) + } + } + }, [getObserver]) const observe = useCallback( - (element, onChange) => { - const observer = getObserver() + (element, onOverflowChange) => { let callbacks = subscribersRef.current.get(element) if (!callbacks) { callbacks = new Set() subscribersRef.current.set(element, callbacks) - observer?.observe(element) } - callbacks.add(onChange) + callbacks.add(onOverflowChange) + observeSubscribedElements() return () => { const set = subscribersRef.current.get(element) if (!set) return - set.delete(onChange) + set.delete(onOverflowChange) if (set.size === 0) { subscribersRef.current.delete(element) - observer?.unobserve(element) + observedElementsRef.current.delete(element) + observerRef.current?.unobserve(element) } } }, - [getObserver], + [observeSubscribedElements], ) + useIsomorphicLayoutEffect(() => { + observeSubscribedElements() + }) + useEffect(() => { const subscribers = subscribersRef.current + const observedElements = observedElementsRef.current return () => { observerRef.current?.disconnect() observerRef.current = null + observedElements.clear() subscribers.clear() } }, []) @@ -323,3 +351,16 @@ export function createDescendantRegistry(options?: { return {Provider, useRegistryState, useRegisterDescendant, useRegisterOverflowObserver} } + +/** + * Treat any target that is not fully visible within the observer root as overflowing. Wrapped items should be fully + * clipped (`isIntersecting: false`, `intersectionRatio: 0`), but partial ratios also count as overflowing to guard + * against sub-pixel boundary cases. + */ +function getIsOverflowing(entry: Pick) { + return !entry.isIntersecting || entry.intersectionRatio < 1 +} + +function supportsIntersectionObserver() { + return typeof IntersectionObserver !== 'undefined' +}