Skip to content
Closed
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
5 changes: 5 additions & 0 deletions .changeset/fix-safari-underlinenav-overflow-race.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/react': patch
---

UnderlineNav: Fix Safari race condition that caused all tabs to collapse into the overflow menu on initial render.
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,12 @@ export function OverflowObserverProvider({

observerRef.current = new IntersectionObserver(
entries => {
// Safari can deliver the initial notification batch before the clipping root has laid out,
// resulting in a zero-size root. When that happens every child reports isIntersecting:false,
// causing all items to collapse into the overflow menu. Ignore this batch entirely; the
// ResizeObserver below re-triggers observeSubscribedElements() once the root gains a real size.
if (root instanceof HTMLElement && root.clientWidth === 0) return

for (const entry of entries) {
const callbacks = subscribersRef.current.get(entry.target)
if (!callbacks) continue
Expand Down Expand Up @@ -97,6 +103,31 @@ export function OverflowObserverProvider({
observeSubscribedElements()
})

useEffect(() => {
if (typeof ResizeObserver === 'undefined') return
// When the root element transitions from zero-width to a real size, the IntersectionObserver's
// initial batch may have already fired against a zero-size root (see guard above). A single
// ResizeObserver on the root re-triggers observation so a fresh, correct batch is delivered.
// Only act on the 0 → non-zero transition to avoid unnecessary work on every resize.
const root = rootRef.current
if (!(root instanceof HTMLElement)) return

let lastWidth = root.clientWidth
const ro = new ResizeObserver(() => {
const width = root.clientWidth
if (width > 0 && lastWidth === 0) {
observerRef.current?.disconnect()
observerRef.current = null
observerRootRef.current = null
observedElementsRef.current.clear()
observeSubscribedElements()
}
lastWidth = width
})
ro.observe(root)
return () => ro.disconnect()
}, [rootRef, observeSubscribedElements])

useEffect(() => {
const subscribers = subscribersRef.current
const observedElements = observedElementsRef.current
Expand All @@ -114,10 +145,11 @@ export function OverflowObserverProvider({
/**
* 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.
* against sub-pixel boundary cases. A small epsilon (0.99) is used instead of exactly 1 to avoid false positives
* from Safari's sub-pixel rounding (e.g. 0.999… for fully-visible elements).
*/
function getIsOverflowing(entry: Pick<IntersectionObserverEntry, 'intersectionRatio' | 'isIntersecting'>) {
return !entry.isIntersecting || entry.intersectionRatio < 1
return !entry.isIntersecting || entry.intersectionRatio < 0.99
}

function supportsIntersectionObserver() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ type Entry = Pick<IntersectionObserverEntry, 'target' | 'isIntersecting' | 'inte
class MockIntersectionObserver {
static instances: MockIntersectionObserver[] = []
readonly observed = new Set<Element>()
disconnected = false
callback: (entries: Entry[]) => void
options?: IntersectionObserverInit
constructor(callback: (entries: Entry[]) => void, options?: IntersectionObserverInit) {
Expand All @@ -24,13 +25,40 @@ class MockIntersectionObserver {
this.observed.delete(element)
}
disconnect() {
this.disconnected = true
this.observed.clear()
}
trigger(entries: Entry[]) {
act(() => this.callback(entries))
}
}

/** Minimal `ResizeObserver` stub that records instances and lets tests drive callbacks. */
class MockResizeObserver {
static instances: MockResizeObserver[] = []
readonly observed = new Set<Element>()
callback: ResizeObserverCallback
constructor(callback: ResizeObserverCallback) {
this.callback = callback
MockResizeObserver.instances.push(this)
}
observe(element: Element) {
this.observed.add(element)
}
unobserve(element: Element) {
this.observed.delete(element)
}
disconnect() {
this.observed.clear()
}
trigger(element?: Element) {
const entries = element
? ([{target: element, contentRect: element.getBoundingClientRect()}] as unknown as ResizeObserverEntry[])
: []
act(() => this.callback(entries, this as unknown as ResizeObserver))
}
}

function Item({disabled}: {disabled?: boolean}) {
const ref = useRef<HTMLLIElement>(null)
const isClipped = useIsClipped(ref, {disabled})
Expand All @@ -52,6 +80,7 @@ describe('useIsClipped', () => {
afterEach(() => {
vi.unstubAllGlobals()
MockIntersectionObserver.instances = []
MockResizeObserver.instances = []
})

it('reports false when there is no surrounding provider', () => {
Expand Down Expand Up @@ -92,4 +121,78 @@ describe('useIsClipped', () => {
observer.trigger([{target: item, isIntersecting: true, intersectionRatio: 1}])
expect(item.getAttribute('data-overflowing')).toBe('false')
})

it('does not report items as overflowing when the observer root has zero width', () => {
vi.stubGlobal('IntersectionObserver', MockIntersectionObserver)
vi.stubGlobal('ResizeObserver', MockResizeObserver)

const {getByTestId} = render(<Harness />)
const item = getByTestId('item')
const root = item.closest('ul')!

// Simulate a zero-width root (as can happen on initial Safari render).
Object.defineProperty(root, 'clientWidth', {configurable: true, get: () => 0})

const observer = MockIntersectionObserver.instances.at(-1)!
// Even though every item reports not intersecting, the zero-width root guard should suppress the update.
observer.trigger([{target: item, isIntersecting: false, intersectionRatio: 0}])
expect(item.getAttribute('data-overflowing')).toBe('false')
})

it('re-observes items when the root transitions from zero width to non-zero width', () => {
vi.stubGlobal('IntersectionObserver', MockIntersectionObserver)
vi.stubGlobal('ResizeObserver', MockResizeObserver)

// Override clientWidth on the prototype BEFORE rendering so the ResizeObserver effect
// captures lastWidth = 0, simulating a zero-width container at mount time (as on Safari).
let mockClientWidth = 0
vi.spyOn(HTMLElement.prototype, 'clientWidth', 'get').mockImplementation(() => mockClientWidth)

const {getByTestId} = render(<Harness />)
const item = getByTestId('item')
const root = item.closest('ul')!

const observerBeforeTransition = MockIntersectionObserver.instances.at(-1)!
const instanceCountBeforeTransition = MockIntersectionObserver.instances.length

// IO callback fires but is suppressed because clientWidth is 0.
observerBeforeTransition.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 must disconnect
// the existing IntersectionObserver and create a new one so a fresh batch is delivered.
mockClientWidth = 400
const ro = MockResizeObserver.instances.at(-1)!
ro.trigger(root)

vi.restoreAllMocks()

// The old observer must have been disconnected.
expect(observerBeforeTransition.disconnected).toBe(true)
// A new IntersectionObserver instance must have been created.
expect(MockIntersectionObserver.instances.length).toBeGreaterThan(instanceCountBeforeTransition)

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

// And a genuinely clipped item should still be reported as overflowing.
freshObserver.trigger([{target: item, isIntersecting: false, intersectionRatio: 0}])
expect(item.getAttribute('data-overflowing')).toBe('true')
})

it('does not treat a sub-pixel intersectionRatio (e.g. 0.999) as overflow', () => {
vi.stubGlobal('IntersectionObserver', MockIntersectionObserver)

const {getByTestId} = render(<Harness />)
const item = getByTestId('item')

const observer = MockIntersectionObserver.instances.at(-1)!
// Safari sometimes reports 0.999… for fully-visible elements due to sub-pixel rounding.
observer.trigger([{target: item, isIntersecting: true, intersectionRatio: 0.999}])
expect(item.getAttribute('data-overflowing')).toBe('false')
})
})
Loading