Underline nav shared observer coalesced rebuild#8035
Open
mattcosta7 wants to merge 1 commit into
Open
Conversation
🦋 Changeset detectedLatest commit: ccea461 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
mattcosta7
force-pushed
the
underline-nav-shared-observer-coalesced-rebuild
branch
from
13d8541 to
3d4beb8
Compare
github-actions
Bot
added
the
integration-tests: recommended
Contributor
|
mattcosta7
force-pushed
the
underline-nav-shared-observer-coalesced-rebuild
branch
from
3d4beb8 to
d436177
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR optimizes overflow detection for UnderlineNav and ActionBar by moving from per-item overflow work (multiple IntersectionObserver instances plus layout reads) to a single shared, root-scoped IntersectionObserver per component, and by coalescing registry rebuilds to reduce redundant work during resize/wrap events.
Changes:
- Extend
createDescendantRegistrywith an opt-in “overflow” mode that provides a shared observer (useRegisterOverflowObserver) and microtask-coalesced rebuild scheduling. - Update
UnderlineNavandActionBaritems to use the shared overflow observer and pass a clipping root ref to the registry provider. - Add a patch changeset describing the internal performance improvement.
Show a summary per file
| File | Description |
|---|---|
| packages/react/src/utils/descendant-registry.tsx | Adds shared overflow observer support and coalesced rebuild scheduling to the descendant registry utility. |
| packages/react/src/UnderlineNav/UnderlineNavItemsRegistry.ts | Enables the registry’s overflow mode for UnderlineNav items. |
| packages/react/src/UnderlineNav/UnderlineNavItem.tsx | Switches item overflow detection to the registry’s shared observer hook. |
| packages/react/src/UnderlineNav/UnderlineNav.tsx | Passes the clipping root ref into the UnderlineNav registry provider. |
| packages/react/src/ActionBar/ActionBar.tsx | Enables overflow mode for ActionBar registry and switches items to shared observer-based overflow detection. |
| .changeset/underline-nav-shared-overflow-observer.md | Adds a patch changeset for the internal performance optimization. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 3
- Review effort level: Low
Comment on lines
+270
to
+274
| 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 |
Comment on lines
+162
to
+170
| const scheduleRebuild = useCallback(() => { | ||
| if (pendingRebuildRef.current) return | ||
| pendingRebuildRef.current = true | ||
| queueMicrotask(() => { | ||
| pendingRebuildRef.current = false | ||
| // Guard against dispatching into an unmounted provider. | ||
| if (isMountedRef.current) rebuildRegistry() | ||
| }) | ||
| }, []) |
Comment on lines
+105
to
+116
| /** | ||
| * Subscribe an element to the registry's shared, root-scoped IntersectionObserver and derive whether it is currently | ||
| * overflowing from the observed entry. | ||
| * | ||
| * 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 | ||
| * items whose overflow is determined by an ancestor (e.g. ActionBar items inside an overflowing group). | ||
| */ | ||
| function useRegisterOverflowObserver(ref: RefObject<HTMLElement | null>, options?: {disabled?: boolean}) { |
iansan5653
reviewed
Jun 26, 2026
Comment on lines
+144
to
+145
| // Items opt into a single shared IntersectionObserver via `useRegisterOverflowObserver` instead of each item creating | ||
| // its own observer. |
Contributor
There was a problem hiding this comment.
[nit] Can probably drop this comment, it's copilot justifying changes now but doesn't really make sense out of the context of this PR
Suggested change
| // Items opt into a single shared IntersectionObserver via `useRegisterOverflowObserver` instead of each item creating | |
| // its own observer. |
| /** State setter from `useRegistryState`. */ | ||
| setRegistry: Dispatch<React.SetStateAction<ReadonlyMap<string, T> | undefined>> | ||
| /** Clipping container used as the `IntersectionObserver` root for overflow detection. */ | ||
| rootRef?: RefObject<Element | null> |
Contributor
There was a problem hiding this comment.
🤔 I like the move to a single observer, but I would like to avoid baking it into descendant-registry. While this is currently only used for the two similar overflow patterns, it's a generic pattern that can be used for registering descendants of any kind. Can we extract the intersection observer pattern to a separate hook or wrapper component instead? That would also give us smaller, more focused files
mattcosta7
force-pushed
the
underline-nav-shared-observer-coalesced-rebuild
branch
from
d436177 to
2122eac
Compare
Contributor
Author
|
Thanks for the review! Addressed all the feedback:
Copilot
Type-check, lint (incl. |
…nObserver for overflow detection Extract overflow detection into a dedicated OverflowObserverProvider + useIsOverflowing hook (kept separate from the generic descendant registry). Each component now shares one root-scoped IntersectionObserver instead of one observer per item, with microtask-coalesced registry rebuilds. Eliminates forced reflow and observer churn during resize. No public API changes.
mattcosta7
force-pushed
the
underline-nav-shared-observer-coalesced-rebuild
branch
from
2122eac to
ccea461
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #
Follow-up performance work on top of the merged
UnderlineNavrewrite (#7506) andActionBaroverflow handling. Replaces the per-item overflow detection (oneIntersectionObserverper item plus per-itemoffsetTopreads) with a single shared, root-scopedIntersectionObserverper component, and coalesces same-frame registry rebuilds into one microtask-scheduled update.This is internal-only: no public API changes.
Why
ref.current.offsetTopinsideuseSyncExternalStore's snapshot, forcing a synchronous layout per item. Overflow is now derived fromIntersectionObserverentries off the render path; snapshots read a cached boolean.queueMicrotaskso a resize that wraps several items triggers a single rebuild.Changelog
Changed
UnderlineNavandActionBarnow detect item overflow using a single shared root-scopedIntersectionObserverper component instead of one observer per item.createDescendantRegistrygains an opt-inoverflowoption exposinguseRegisterOverflowObserver, plus microtask-coalesced rebuilds.Removed
IntersectionObserver+offsetTopoverflow checks inUnderlineNav.ItemandActionBaritems.Rollout strategy
Testing & Reviewing
No behavioral changes are intended. Existing
ActionBar,UnderlineNav, anddescendant-registryunit tests pass. Verify overflow behavior by resizing the viewport: items wrap into the overflow menu and the "More" affordance appears/disappears as before, and overflowed items remain non-focusable (tabIndex={-1}/aria-hidden).Merge checklist