diff --git a/.changeset/underline-nav-shared-overflow-observer.md b/.changeset/underline-nav-shared-overflow-observer.md new file mode 100644 index 00000000000..86927be358c --- /dev/null +++ b/.changeset/underline-nav-shared-overflow-observer.md @@ -0,0 +1,5 @@ +--- +'@primer/react': patch +--- + +UnderlineNav, ActionBar: Detect item overflow with a single shared IntersectionObserver per component instead of one observer per item, reducing observer churn during resize. No public API changes. diff --git a/packages/react/src/ActionBar/ActionBar.tsx b/packages/react/src/ActionBar/ActionBar.tsx index 299ff7ffb6c..037922e3aed 100644 --- a/packages/react/src/ActionBar/ActionBar.tsx +++ b/packages/react/src/ActionBar/ActionBar.tsx @@ -1,5 +1,5 @@ import {type RefObject, type MouseEventHandler, useContext} from 'react' -import React, {useState, useCallback, useRef, forwardRef, useMemo, useSyncExternalStore} from 'react' +import React, {useState, useCallback, useRef, forwardRef, useMemo} from 'react' import {KebabHorizontalIcon} from '@primer/octicons-react' import {ActionList, type ActionListItemProps} from '../ActionList' @@ -11,6 +11,8 @@ import styles from './ActionBar.module.css' import {clsx} from 'clsx' import {useMergedRefs} from '../hooks' import {createDescendantRegistry} from '../utils/descendant-registry' +import {OverflowObserverProvider} from '../internal/components/OverflowObserverProvider' +import {useIsClipped} from '../internal/hooks/useOverflowObserver' type ChildProps = | { @@ -238,7 +240,11 @@ export const ActionBar: React.FC> = ({
{/* An empty first element allows the real first item to wrap to the next line and get clipped. */}
- {children} + + + {children} + +
@@ -314,33 +320,9 @@ function useActionBarItem(ref: React.RefObject, registryProp const isGroupOverflowing = useContext(ActionBarGroupContext)?.isOverflowing const isInGroup = isGroupOverflowing !== undefined - const subscribeIntersectionObserver = useCallback( - (onChange: () => void) => { - // There's no need to register observers on items inside of a group - // since the entire group overflows at once - if (isInGroup) return () => {} - - // Technically 1 should work as the threshold, but in some scenarios that - // doesn't seem to trigger correctly - probably because the browser still - // thinks a tiny bit of the button is not visible, since the container - // height is exactly the button height. So 75% should be more reliable. - const observer = new IntersectionObserver(() => onChange(), {threshold: 0.75}) - - if (ref.current) observer.observe(ref.current) - return () => observer.disconnect() - }, - [ref, isInGroup], - ) - - const isItemOverflowing = useSyncExternalStore( - subscribeIntersectionObserver, - // Note: the IntersectionObserver 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 will move to the next row which - // increases its `offsetTop` - () => (ref.current ? ref.current.offsetTop > 0 : false), - () => false, - ) + // 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 and always reports `false` for the child item itself. + const isItemOverflowing = useIsClipped(ref, {disabled: isInGroup}) const isOverflowing = isGroupOverflowing || isItemOverflowing diff --git a/packages/react/src/UnderlineNav/UnderlineNav.tsx b/packages/react/src/UnderlineNav/UnderlineNav.tsx index 5a9507cd890..0559f4f71a2 100644 --- a/packages/react/src/UnderlineNav/UnderlineNav.tsx +++ b/packages/react/src/UnderlineNav/UnderlineNav.tsx @@ -6,6 +6,7 @@ import {ActionList} from '../ActionList' import {ActionMenu} from '../ActionMenu' import CounterLabel from '../CounterLabel' import {LoadingCounter, UnderlineItemList, UnderlineWrapper} from '../internal/components/UnderlineTabbedInterface' +import {OverflowObserverProvider} from '../internal/components/OverflowObserverProvider' import {invariant} from '../utils/invariant' import classes from './UnderlineNav.module.css' import {UnderlineNavContext} from './UnderlineNavContext' @@ -101,9 +102,11 @@ export const UnderlineNav = forwardRef( data-has-overflow={isOverflowing ? 'true' : undefined} > - - {children} - + + + {children} + +
diff --git a/packages/react/src/UnderlineNav/UnderlineNavItem.tsx b/packages/react/src/UnderlineNav/UnderlineNavItem.tsx index e25083c9304..9dc65b11028 100644 --- a/packages/react/src/UnderlineNav/UnderlineNavItem.tsx +++ b/packages/react/src/UnderlineNav/UnderlineNavItem.tsx @@ -1,9 +1,10 @@ -import React, {forwardRef, useRef, useContext, useCallback, useSyncExternalStore} from 'react' +import React, {forwardRef, useRef, useContext} from 'react' import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic' import {UnderlineNavContext} from './UnderlineNavContext' import {UnderlineItem} from '../internal/components/UnderlineTabbedInterface' import classes from './UnderlineNavItem.module.css' import {type UnderlineNavItemProps, UnderlineNavItemsRegistry} from './UnderlineNavItemsRegistry' +import {useIsClipped} from '../internal/hooks/useOverflowObserver' export const UnderlineNavItem = forwardRef((allProps, forwardedRef) => { const { @@ -22,24 +23,9 @@ export const UnderlineNavItem = forwardRef((allProps, forwardedRef) => { const {loadingCounters} = useContext(UnderlineNavContext) - const isOverflowing = useSyncExternalStore( - useCallback( - onChange => { - const observer = new IntersectionObserver(() => onChange(), { - threshold: 1, - }) - if (ref.current) observer.observe(ref.current) - return () => observer.disconnect() - }, - [ref], - ), - // Note: the IntersectionObserver 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 will move to the next row which - // increases its `offsetTop` - () => (ref.current ? ref.current.offsetTop > 0 : false), - () => false, - ) + // Observe the wrapping `
  • ` directly so a root-scoped IntersectionObserver can detect when the item is clipped + // onto the hidden next row. + const isOverflowing = useIsClipped(ref) UnderlineNavItemsRegistry.useRegisterDescendant(isOverflowing ? allProps : null) diff --git a/packages/react/src/internal/components/OverflowObserverProvider.tsx b/packages/react/src/internal/components/OverflowObserverProvider.tsx new file mode 100644 index 00000000000..60f4095e72a --- /dev/null +++ b/packages/react/src/internal/components/OverflowObserverProvider.tsx @@ -0,0 +1,125 @@ +import {useCallback, useEffect, useRef, type ReactNode, type RefObject} from 'react' +import useIsomorphicLayoutEffect from '../../utils/useIsomorphicLayoutEffect' +import {OverflowObserverContext, type ObserveFn} from '../hooks/useOverflowObserver' + +/** + * Owns a single `IntersectionObserver` shared by every descendant that calls `useIsClipped`, instead of each item + * creating its own observer. Each observed element maps to a set of change callbacks; one observer notification fans + * out to all of them. + * + * `rootRef` must point to the element that clips overflowing children (typically a single-row, `overflow: hidden` + * container). The observer is root-scoped to that element so children that wrap onto a clipped row are reported as + * overflowing. Until the root is attached the provider stays inert (it never falls back to the viewport, which would + * not reflect the container's clipping) and re-checks on later renders. + */ +export function OverflowObserverProvider({ + children, + rootRef, +}: { + children: ReactNode + /** Clipping container used as the `IntersectionObserver` root for overflow detection. */ + rootRef: RefObject +}) { + // Map of observed element -> set of subscriber callbacks. + const subscribersRef = useRef void>>>(new Map()) + const observedElementsRef = useRef>(new Set()) + const observerRef = useRef(null) + const observerRootRef = useRef(null) + + // Lazily create the observer once the root is available so SSR / zero-item renders allocate nothing. + const getObserver = useCallback(() => { + if (!supportsIntersectionObserver()) return null + + const root = rootRef.current + // Root-scoped overflow detection requires the clipping container. Stay inert until it's attached rather than + // falling back to a viewport-rooted observer, which wouldn't reflect the container's clipping. + if (root === null) return 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 + const isOverflowing = getIsOverflowing(entry) + for (const cb of callbacks) cb(isOverflowing) + } + }, + {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, onOverflowChange) => { + let callbacks = subscribersRef.current.get(element) + if (!callbacks) { + callbacks = new Set() + subscribersRef.current.set(element, callbacks) + } + callbacks.add(onOverflowChange) + observeSubscribedElements() + + return () => { + const set = subscribersRef.current.get(element) + if (!set) return + set.delete(onOverflowChange) + if (set.size === 0) { + subscribersRef.current.delete(element) + observedElementsRef.current.delete(element) + observerRef.current?.unobserve(element) + } + } + }, + [observeSubscribedElements], + ) + + useIsomorphicLayoutEffect(() => { + observeSubscribedElements() + }) + + useEffect(() => { + const subscribers = subscribersRef.current + const observedElements = observedElementsRef.current + return () => { + observerRef.current?.disconnect() + observerRef.current = null + observedElements.clear() + subscribers.clear() + } + }, []) + + return {children} +} + +/** + * 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' +} diff --git a/packages/react/src/internal/hooks/__tests__/useOverflowObserver.test.tsx b/packages/react/src/internal/hooks/__tests__/useOverflowObserver.test.tsx new file mode 100644 index 00000000000..359e10386a6 --- /dev/null +++ b/packages/react/src/internal/hooks/__tests__/useOverflowObserver.test.tsx @@ -0,0 +1,95 @@ +import {afterEach, describe, expect, it, vi} from 'vitest' +import {useRef} from 'react' +import {act, render} from '@testing-library/react' +import {OverflowObserverProvider} from '../../components/OverflowObserverProvider' +import {useIsClipped} from '../useOverflowObserver' + +type Entry = Pick + +/** Minimal `IntersectionObserver` stub that records instances and lets tests drive callbacks. */ +class MockIntersectionObserver { + static instances: MockIntersectionObserver[] = [] + readonly observed = new Set() + callback: (entries: Entry[]) => void + options?: IntersectionObserverInit + constructor(callback: (entries: Entry[]) => void, options?: IntersectionObserverInit) { + this.callback = callback + this.options = options + MockIntersectionObserver.instances.push(this) + } + observe(element: Element) { + this.observed.add(element) + } + unobserve(element: Element) { + this.observed.delete(element) + } + disconnect() { + this.observed.clear() + } + trigger(entries: Entry[]) { + act(() => this.callback(entries)) + } +} + +function Item({disabled}: {disabled?: boolean}) { + const ref = useRef(null) + const isClipped = useIsClipped(ref, {disabled}) + return
  • +} + +function Harness({disabled}: {disabled?: boolean}) { + const rootRef = useRef(null) + return ( +
      + + + +
    + ) +} + +describe('useIsClipped', () => { + afterEach(() => { + vi.unstubAllGlobals() + MockIntersectionObserver.instances = [] + }) + + it('reports false when there is no surrounding provider', () => { + const {getByTestId} = render() + expect(getByTestId('item').getAttribute('data-overflowing')).toBe('false') + }) + + it('is inert and reports false when IntersectionObserver is unavailable', () => { + vi.stubGlobal('IntersectionObserver', undefined) + const {getByTestId} = render() + expect(getByTestId('item').getAttribute('data-overflowing')).toBe('false') + }) + + it('does not subscribe and reports false when disabled', () => { + vi.stubGlobal('IntersectionObserver', MockIntersectionObserver) + const {getByTestId} = render() + expect(getByTestId('item').getAttribute('data-overflowing')).toBe('false') + // A disabled item should never be observed by the shared observer. + const item = getByTestId('item') + expect(MockIntersectionObserver.instances.every(o => !o.observed.has(item))).toBe(true) + }) + + it('reflects overflow state from the shared root-scoped observer', () => { + vi.stubGlobal('IntersectionObserver', MockIntersectionObserver) + const {getByTestId} = render() + const item = getByTestId('item') + + const observer = MockIntersectionObserver.instances.at(-1)! + // The observer is created against the provided clipping root, not the viewport. + expect(observer.options?.root).not.toBeNull() + expect(observer.observed.has(item)).toBe(true) + + // A clipped (wrapped) item is reported as overflowing. + observer.trigger([{target: item, isIntersecting: false, intersectionRatio: 0}]) + expect(item.getAttribute('data-overflowing')).toBe('true') + + // A fully visible item is not overflowing. + observer.trigger([{target: item, isIntersecting: true, intersectionRatio: 1}]) + expect(item.getAttribute('data-overflowing')).toBe('false') + }) +}) diff --git a/packages/react/src/internal/hooks/useOverflowObserver.ts b/packages/react/src/internal/hooks/useOverflowObserver.ts new file mode 100644 index 00000000000..26cd32c3317 --- /dev/null +++ b/packages/react/src/internal/hooks/useOverflowObserver.ts @@ -0,0 +1,49 @@ +import {createContext, useCallback, useContext, useRef, useSyncExternalStore, type RefObject} from 'react' + +/** Subscribe a single observed element to the shared `IntersectionObserver`. Returns an unsubscribe function. */ +export type ObserveFn = (element: Element, onOverflowChange: (isOverflowing: boolean) => void) => () => void + +/** Provided by `OverflowObserverProvider`; `null` when no provider is present. Consumed via `useIsClipped`. */ +export const OverflowObserverContext = createContext(null) + +/** + * Track whether `ref`'s element is currently clipped (overflowing) by the nearest `OverflowObserverProvider`'s + * root-scoped `IntersectionObserver`. + * + * Returns `false` when there is no surrounding provider, during SSR, when `IntersectionObserver` is unavailable, or when + * `disabled` is set. + * + * @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 items + * whose overflow is determined by an ancestor (e.g. ActionBar items inside an overflowing group). + */ +export function useIsClipped(ref: RefObject, options?: {disabled?: boolean}) { + const disabled = options?.disabled ?? false + const observe = useContext(OverflowObserverContext) + const isOverflowingRef = useRef(false) + + const subscribe = useCallback( + (onChange: () => void) => { + if (disabled) return () => {} + const element = ref.current + // The hook only tracks overflow through a surrounding provider's shared observer. When no provider is present (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() + } + } + + return observe(element, updateOverflowState) + }, + [ref, observe, disabled], + ) + + return useSyncExternalStore(subscribe, () => (disabled ? false : isOverflowingRef.current), getOverflowServerSnapshot) +} + +/** Stable server snapshot for `useIsClipped`: overflow is never measured during SSR. */ +const getOverflowServerSnapshot = () => false diff --git a/packages/react/src/utils/__tests__/descendant-registry.test.tsx b/packages/react/src/utils/__tests__/descendant-registry.test.tsx index f357b8d2b6e..3ba46c8dace 100644 --- a/packages/react/src/utils/__tests__/descendant-registry.test.tsx +++ b/packages/react/src/utils/__tests__/descendant-registry.test.tsx @@ -121,6 +121,38 @@ describe('createDescendantRegistry', () => { expect(getByTestId('registry-values').textContent).toBe('a,middle,b') }) + it('coalesces multiple descendants added in a single update into one correct rebuild', async () => { + const {RegistryParent, Item} = createTestRegistry() + + function Test() { + const [expanded, setExpanded] = useState(false) + return ( + + + {expanded && ( + <> + + + + + )} + + + + ) + } + + const {getByTestId, getByRole} = render() + expect(getByTestId('registry-values').textContent).toBe('a,e') + + // Several descendants register in the same tick; the coalesced rebuild must still capture all of them in order. + await userEvent.click(getByRole('button')) + + expect(getByTestId('registry-values').textContent).toBe('a,b,c,d,e') + }) + it('drops items from the registry after they unmount', async () => { const {RegistryParent, Item} = createTestRegistry() diff --git a/packages/react/src/utils/descendant-registry.tsx b/packages/react/src/utils/descendant-registry.tsx index d2e6359a490..2397db27315 100644 --- a/packages/react/src/utils/descendant-registry.tsx +++ b/packages/react/src/utils/descendant-registry.tsx @@ -83,6 +83,28 @@ export function createDescendantRegistry() { /** State value to trigger a re-render and force all descendants to re-register. This ensures everything remains ordered. */ const [key, rebuildRegistry] = useReducer(prev => prev + 1, 0) + // Coalesce rebuilds: when several descendants register in the same tick (e.g. multiple items crossing the wrap + // boundary during one resize frame), schedule a single rebuild via microtask instead of one rebuild per + // registration. The microtask drains before paint, so the rebuild still lands within the same frame. + const pendingRebuildRef = useRef(false) + const isMountedRef = useRef(true) + const scheduleRebuild = useCallback(() => { + if (pendingRebuildRef.current) return + pendingRebuildRef.current = true + scheduleMicrotask(() => { + pendingRebuildRef.current = false + // Guard against dispatching into an unmounted provider. + if (isMountedRef.current) rebuildRegistry() + }) + }, []) + + useEffect(() => { + isMountedRef.current = true + return () => { + isMountedRef.current = false + } + }, []) + // If a rebuild is queued, instantiate a new map. Must be in a layout effect to run before all descendants' effects run to populate it useIsomorphicLayoutEffect(function instantiateNewRegistry() { if (workingRegistryRef.current === 'queued') { @@ -101,9 +123,10 @@ export function createDescendantRegistry() { workingRegistryRef.current.set(id, unsetValue) } else if (workingRegistryRef.current === 'idle') { // When idle, registering a new component causes the whole registry to be rebuilt (because that item could - // be inserted anywhere in the tree, changing the order of items) + // be inserted anywhere in the tree, changing the order of items). Coalesce concurrent registrations so the + // rebuild only happens once per tick. workingRegistryRef.current = 'queued' - rebuildRegistry() + scheduleRebuild() } // Noop if status is `queued` since we will restart the map in the next cycle @@ -121,7 +144,7 @@ export function createDescendantRegistry() { } } }, - [setRegistry], + [setRegistry, scheduleRebuild], ) /** Update a descendant's value in the registry. */ @@ -158,3 +181,13 @@ export function createDescendantRegistry() { return {Provider, useRegistryState, useRegisterDescendant} } + +/** Schedule a callback on a microtask, with a fallback for environments that lack `queueMicrotask`. */ +function scheduleMicrotask(callback: () => void) { + if (typeof queueMicrotask === 'function') { + queueMicrotask(callback) + } else { + // Fallback for environments without `queueMicrotask`. A macrotask still coalesces all synchronous registrations. + setTimeout(callback) + } +}