Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .changeset/underline-nav-shared-overflow-observer.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
21 changes: 9 additions & 12 deletions packages/react/src/ActionBar/ActionBar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -139,11 +139,9 @@ export type ActionBarMenuProps = {
returnFocusRef?: React.RefObject<HTMLElement>
} & 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<ChildProps | null>({overflow: {threshold: 0.75}})
// Items opt into a single shared IntersectionObserver via `useRegisterOverflowObserver` instead of each item creating
// its own observer.
const ActionBarItemsRegistry = createDescendantRegistry<ChildProps | null>({overflow: {}})

const FOCUSABLE_ITEM_SELECTOR =
':is(button, a, input, [tabindex]):not(:disabled):not([data-overflowing]):not([data-more-button-inactive])'
Expand Down Expand Up @@ -207,6 +205,7 @@ export const ActionBar: React.FC<React.PropsWithChildren<ActionBarProps>> = ({
gap = 'condensed',
}) => {
const [childRegistry, setChildRegistry] = ActionBarItemsRegistry.useRegistryState()
const overflowContainerRef = useRef<HTMLDivElement>(null)

const overflowItems = useMemo(
() =>
Expand Down Expand Up @@ -237,10 +236,12 @@ export const ActionBar: React.FC<React.PropsWithChildren<ActionBarProps>> = ({
data-size={size}
data-has-overflow={overflowItems ? overflowItems.length > 0 : undefined}
>
<div className={styles.OverflowContainer}>
<div ref={overflowContainerRef} className={styles.OverflowContainer}>
{/* An empty first element allows the real first item to wrap to the next line and get clipped. */}
<div className={styles.OverflowSpacer} />
<ActionBarItemsRegistry.Provider setRegistry={setChildRegistry}>{children}</ActionBarItemsRegistry.Provider>
<ActionBarItemsRegistry.Provider setRegistry={setChildRegistry} rootRef={overflowContainerRef}>
{children}
</ActionBarItemsRegistry.Provider>
</div>
<ActionMenu>
<ActionMenu.Anchor>
Expand Down Expand Up @@ -314,12 +315,8 @@ function useActionBarItem(ref: React.RefObject<HTMLElement | null>, 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
Expand Down
2 changes: 1 addition & 1 deletion packages/react/src/UnderlineNav/UnderlineNav.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ export const UnderlineNav = forwardRef(
data-has-overflow={isOverflowing ? 'true' : undefined}
>
<UnderlineItemList ref={listRef} role="list" className={classes.ItemsList}>
<UnderlineNavItemsRegistry.Provider setRegistry={setRegisteredItems}>
<UnderlineNavItemsRegistry.Provider setRegistry={setRegisteredItems} rootRef={listRef}>
{children}
</UnderlineNavItemsRegistry.Provider>
</UnderlineItemList>
Expand Down
5 changes: 2 additions & 3 deletions packages/react/src/UnderlineNav/UnderlineNavItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<li>` 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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<UnderlineNavItemProps | null>({
overflow: {threshold: 1},
overflow: {},
})
81 changes: 63 additions & 18 deletions packages/react/src/utils/__tests__/descendant-registry.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ describe('createDescendantRegistry coalesced rebuilds', () => {
<button type="button" onClick={() => setExtraItems(5)}>
Add five
</button>
</RegistryParent>,
</RegistryParent>
)
}

Expand Down Expand Up @@ -282,7 +282,7 @@ describe('createDescendantRegistry coalesced rebuilds', () => {
<button type="button" onClick={() => setShow(true)}>
Show middle
</button>
</RegistryParent>,
</RegistryParent>
)
}

Expand All @@ -302,20 +302,24 @@ 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<IntersectionObserverEntry, 'target' | 'isIntersecting' | 'intersectionRatio'>
type FakeObserver = {
callback: IntersectionObserverCallback
options?: IntersectionObserverInit
observed: Set<Element>
observe: ReturnType<typeof vi.fn>
unobserve: ReturnType<typeof vi.fn>
disconnect: ReturnType<typeof vi.fn>
trigger: () => void
emit: (entries: MockEntry[]) => void
}
let observers: FakeObserver[] = []

beforeEach(() => {
observers = []
class MockIntersectionObserver {
callback: IntersectionObserverCallback
options?: IntersectionObserverInit
observed = new Set<Element>()
observe = vi.fn((el: Element) => {
this.observed.add(el)
Expand All @@ -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() {
Expand All @@ -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<string | null>({overflow: {threshold: 1}})
const {Provider, useRegistryState, useRegisterDescendant, useRegisterOverflowObserver} = createDescendantRegistry<
string | null
>({overflow: {}})

function RegistryParent({children}: {children: React.ReactNode}) {
const [, setRegistry] = useRegistryState()
return <Provider setRegistry={setRegistry}>{children}</Provider>
const rootRef = useRef<HTMLDivElement>(null)
return (
<div ref={rootRef} data-testid="observer-root">
<Provider setRegistry={setRegistry} rootRef={rootRef}>
{children}
</Provider>
</div>
)
}

function Item({value}: {value: string}) {
Expand All @@ -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(
<RegistryParent>
<Item value="a" />
<Item value="b" />
Expand All @@ -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)
})

Expand All @@ -397,22 +411,52 @@ describe('createDescendantRegistry shared IntersectionObserver', () => {
</RegistryParent>,
)

// 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) {
expect(el).toHaveAttribute('data-overflowing', 'true')
}
})

it('updates an item from the observed entry without reading layout', () => {
const {RegistryParent, Item} = createOverflowRegistry()

const {getByTestId} = render(
<RegistryParent>
<Item value="a" />
</RegistryParent>,
)

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

Expand Down Expand Up @@ -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 () => {
Expand Down
Loading
Loading