Skip to content

fix: Safari UnderlineNav zero-size root race in OverflowObserverProvider#8183

Closed
mattcosta7 with Copilot wants to merge 4 commits into
mainfrom
copilot/fix-zero-size-root-race
Closed

fix: Safari UnderlineNav zero-size root race in OverflowObserverProvider#8183
mattcosta7 with Copilot wants to merge 4 commits into
mainfrom
copilot/fix-zero-size-root-race

Conversation

Copilot AI commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Safari intermittently delivers the IntersectionObserver's initial callback batch before the root element has a real layout box (clientWidth === 0). With a zero-size root, every observed <li> reports isIntersecting: false, collapsing all tabs into the "More" menu. Because collapsing removes the overflow from the DOM, the observer never re-fires — the bad state is sticky until an unrelated resize occurs.

Changes in OverflowObserverProvider.tsx

  • Zero-size root guard — IO callback returns early when root.clientWidth === 0, suppressing false-overflow reports from a not-yet-laid-out root.
  • ResizeObserver on root — A single ResizeObserver watches the root element exclusively. On the 0 → non-zero width transition it clears observed state and re-calls observeSubscribedElements(), delivering a fresh correct IO batch. No-ops on all other resizes. SSR-safe.
  • Sub-pixel thresholdgetIsOverflowing now uses intersectionRatio < 0.99 instead of < 1, eliminating Safari's 0.999… false positives for fully-visible elements.

All three changes are scoped to OverflowObserverProvider.tsx, so every consumer of useIsClipped (UnderlineNav, ActionBar) benefits.

Changelog

New

  • none

Changed

  • OverflowObserverProvider: ignore IntersectionObserver batches delivered against a zero-width root; re-observe when root width transitions from 0 to non-zero; treat intersectionRatio ≥ 0.99 as fully visible

Removed

  • none

Rollout strategy

  • Patch release
  • Minor release
  • Major release; if selected, include a written rollout or migration plan
  • None; if selected, include a brief description as to why

Testing & Reviewing

Three new unit tests cover the fix:

  • IO callback with zero-width root does not flip items to overflowing
  • Root 0 → non-zero transition re-observes and produces correct state
  • intersectionRatio: 0.999 is not treated as overflow

@changeset-bot

changeset-bot Bot commented Jul 17, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 4589f21

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@primer/react Patch

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

Copilot AI and others added 2 commits July 17, 2026 13:30
Co-authored-by: mattcosta7 <8616962+mattcosta7@users.noreply.github.com>
…ove ResizeObserver mock

Co-authored-by: mattcosta7 <8616962+mattcosta7@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix zero-size root race in UnderlineNav overflow detection fix: Safari UnderlineNav zero-size root race in OverflowObserverProvider Jul 17, 2026
Copilot AI requested a review from mattcosta7 July 17, 2026 13:34
@github-actions github-actions Bot added the integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm label Jul 17, 2026
@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Action required

👋 Hi, this pull request contains changes to the source code that github/github-ui depends on. If you are GitHub staff, test these changes with github/github-ui using the integration workflow. If this doesn't work, you can also use the original workflow here. Check the integration testing docs for step-by-step instructions. Or, apply the integration-tests: skipped manually label to skip these checks.

To publish a canary release for integration testing, apply the Canary Release label to this PR.

@mattcosta7 mattcosta7 added the Canary Release Apply this label when you want CI to create a canary release of the current PR label Jul 17, 2026
@github-actions
github-actions Bot temporarily deployed to storybook-preview-8183 July 17, 2026 13:45 Inactive
@mattcosta7
mattcosta7 marked this pull request as ready for review July 17, 2026 14:01
@mattcosta7
mattcosta7 requested a review from a team as a code owner July 17, 2026 14:01

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses a Safari-specific IntersectionObserver race where the initial callback can run before the clipping root has a real layout box (clientWidth === 0), causing UnderlineNav (and other useIsClipped consumers) to incorrectly mark all items as overflowing and get stuck in the “More” menu state.

Changes:

  • Add a guard to ignore IntersectionObserver batches delivered against a zero-width root.
  • Add a root-only ResizeObserver to re-run observation when the root transitions from 0 → non-zero width.
  • Loosen the “fully visible” threshold to intersectionRatio >= 0.99 to avoid Safari sub-pixel rounding false positives, and add unit tests + a patch changeset.
Show a summary per file
File Description
packages/react/src/internal/hooks/tests/useOverflowObserver.test.tsx Adds unit tests covering the zero-width root guard, the resize-driven re-observe path, and the 0.999 ratio case.
packages/react/src/internal/components/OverflowObserverProvider.tsx Implements the Safari guard, resize-driven re-observation, and the intersectionRatio < 0.99 overflow heuristic.
.changeset/fix-safari-underlinenav-overflow-race.md Patch changeset describing the Safari UnderlineNav overflow race fix.

Review details

  • Files reviewed: 3/3 changed files
  • Comments generated: 2
  • Review effort level: Low

Comment on lines +118 to +123
if (width > 0 && lastWidth === 0) {
observedElementsRef.current.clear()
observeSubscribedElements()
}
lastWidth = width
})
Comment on lines +152 to +165
const observer = MockIntersectionObserver.instances.at(-1)!
observer.trigger([{target: item, isIntersecting: false, intersectionRatio: 0}])
expect(item.getAttribute('data-overflowing')).toBe('false')

// Root gains size — ResizeObserver fires the 0→non-zero transition, which clears
// observedElementsRef and calls observeSubscribedElements() to re-attach the IO.
clientWidth = 400
const ro = MockResizeObserver.instances.at(-1)!
ro.trigger(root)

// Now that items are re-observed, a fresh IO batch with correct intersections is delivered.
const freshObserver = MockIntersectionObserver.instances.at(-1)!
freshObserver.trigger([{target: item, isIntersecting: true, intersectionRatio: 1}])
expect(item.getAttribute('data-overflowing')).toBe('false')
…8184)

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: mattcosta7 <8616962+mattcosta7@users.noreply.github.com>
@primer-integration

primer-integration Bot commented Jul 17, 2026

Copy link
Copy Markdown

Integration test results from github/github-ui PR:

Passed  CI   Passed
Waiting  VRT   Waiting
Waiting  Projects   Waiting

@mattcosta7 mattcosta7 closed this Jul 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Canary Release Apply this label when you want CI to create a canary release of the current PR integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants