perf(tab-bar): scrolling the tab strip no longer re-renders every tab (#24240)

* perf(tab-bar): move scroll thumb position out of TabBar state

The thumb moved every scroll and resize frame, re-rendering every tab with it.

Fixes #24012

* perf(tab-bar): reuse the strip's resize observers for the scroll thumb

The thumb had its own observer on every tab, on top of the tab bar's.

* test(e2e): run the tab strip scroll spec without waiting on frames

CI's hidden window draws about one frame a second, so real wheel input timed out. Also drops Reflect.get for the anti-slop audit.

* test(tab-bar): pin that a re-render leaves the scroll thumb's geometry alone

The thumb's width and transform live outside React, so the element must survive a
hover re-render with its geometry intact.

* test(e2e): set the tab strip scroll start after the strip stops retitling

Opening the tabs leaves the strip pinned to its end, and the scroll event that
releases that pin only lands with the next frame - about a second in CI's hidden
window. A tab retitling inside that window re-pinned the strip to the end, so the
wheel sweep would start at an edge and every step would flip the edge flags.

---------

Co-authored-by: Neil <neil@stably.ai>
This commit is contained in:
Kelvin Amoaba
2026-10-05 16:20:52 -07:00
committed by GitHub
co-authored by Neil
parent 3d7b306183
commit 355e35b5f2
8 changed files with 459 additions and 154 deletions
@@ -4,37 +4,20 @@ import React, { createRef } from 'react'
import { afterEach, describe, expect, it, vi } from 'vitest'
import { cleanup, fireEvent, render } from '@testing-library/react'
import { TabStripScrollIndicator } from './TabStripScrollIndicator'
import type { TabStripScrollMetrics } from './tab-strip-scroll-metrics'
afterEach(() => {
cleanup()
vi.restoreAllMocks()
})
const OVERFLOW_METRICS: TabStripScrollMetrics = {
hasOverflow: true,
canScrollStart: false,
canScrollEnd: true,
thumbSizeFraction: 0.4,
thumbOffsetFraction: 0
}
const NO_OVERFLOW_METRICS: TabStripScrollMetrics = {
hasOverflow: false,
canScrollStart: false,
canScrollEnd: false,
thumbSizeFraction: 1,
thumbOffsetFraction: 0
}
describe('TabStripScrollIndicator', () => {
it('renders null when there is no overflow', () => {
const { container } = render(<TabStripScrollIndicator metrics={NO_OVERFLOW_METRICS} />)
const { container } = render(<TabStripScrollIndicator hasOverflow={false} />)
expect(container.firstChild).toBeNull()
})
it('renders under the tabs with bottom-0 and idle 3px height', () => {
const { getByTestId } = render(<TabStripScrollIndicator metrics={OVERFLOW_METRICS} />)
const { getByTestId } = render(<TabStripScrollIndicator hasOverflow />)
const indicator = getByTestId('tab-strip-scroll-indicator')
expect(indicator).toBeTruthy()
expect(indicator.className).toContain('bottom-0')
@@ -48,7 +31,7 @@ describe('TabStripScrollIndicator', () => {
})
it('expands to 4px and becomes opaque on pointer hover, restores on leave', () => {
const { getByTestId } = render(<TabStripScrollIndicator metrics={OVERFLOW_METRICS} />)
const { getByTestId } = render(<TabStripScrollIndicator hasOverflow />)
const indicator = getByTestId('tab-strip-scroll-indicator')
expect(indicator.className).toContain('h-[3px]')
expect(indicator.className).toContain('opacity-0')
@@ -70,7 +53,7 @@ describe('TabStripScrollIndicator', () => {
;(scrollContainerRef as React.MutableRefObject<HTMLElement>).current = scrollContainer
const { getByTestId } = render(
<TabStripScrollIndicator metrics={OVERFLOW_METRICS} scrollContainerRef={scrollContainerRef} />
<TabStripScrollIndicator hasOverflow scrollContainerRef={scrollContainerRef} />
)
const indicator = getByTestId('tab-strip-scroll-indicator')
Object.defineProperty(indicator, 'clientWidth', { value: 400, configurable: true })
@@ -100,18 +83,14 @@ describe('TabStripScrollIndicator', () => {
})
it('applies pointer-events-none when disabled', () => {
const { getByTestId } = render(
<TabStripScrollIndicator metrics={OVERFLOW_METRICS} disabled={true} />
)
const { getByTestId } = render(<TabStripScrollIndicator hasOverflow disabled={true} />)
const indicator = getByTestId('tab-strip-scroll-indicator')
expect(indicator.className).toContain('pointer-events-none')
expect(indicator.className).not.toContain('group-hover/tab-strip:pointer-events-auto')
})
it('stays hidden and unexpanded on hover when disabled', () => {
const { getByTestId } = render(
<TabStripScrollIndicator metrics={OVERFLOW_METRICS} disabled={true} />
)
const { getByTestId } = render(<TabStripScrollIndicator hasOverflow disabled={true} />)
const indicator = getByTestId('tab-strip-scroll-indicator')
fireEvent.pointerEnter(indicator)
expect(indicator.className).toContain('opacity-0')
@@ -127,7 +106,7 @@ describe('TabStripScrollIndicator', () => {
const { getByTestId } = render(
<TabStripScrollIndicator
metrics={OVERFLOW_METRICS}
hasOverflow
scrollContainerRef={scrollContainerRef}
disabled={true}
/>
@@ -144,7 +123,7 @@ describe('TabStripScrollIndicator', () => {
;(scrollContainerRef as React.MutableRefObject<HTMLElement>).current = scrollContainer
const { getByTestId } = render(
<TabStripScrollIndicator metrics={OVERFLOW_METRICS} scrollContainerRef={scrollContainerRef} />
<TabStripScrollIndicator hasOverflow scrollContainerRef={scrollContainerRef} />
)
const indicator = getByTestId('tab-strip-scroll-indicator')
fireEvent.wheel(indicator, { deltaX: 40, deltaY: 0 })
@@ -164,7 +143,7 @@ describe('TabStripScrollIndicator', () => {
;(scrollContainerRef as React.MutableRefObject<HTMLElement>).current = scrollContainer
const { getByTestId } = render(
<TabStripScrollIndicator metrics={OVERFLOW_METRICS} scrollContainerRef={scrollContainerRef} />
<TabStripScrollIndicator hasOverflow scrollContainerRef={scrollContainerRef} />
)
const indicator = getByTestId('tab-strip-scroll-indicator')
Object.defineProperty(indicator, 'clientWidth', { value: 400, configurable: true })
@@ -200,13 +179,7 @@ describe('TabStripScrollIndicator', () => {
;(scrollContainerRef as React.MutableRefObject<HTMLElement>).current = scrollContainer
const { getByTestId } = render(
<TabStripScrollIndicator
metrics={{
...OVERFLOW_METRICS,
thumbSizeFraction: 0.4
}}
scrollContainerRef={scrollContainerRef}
/>
<TabStripScrollIndicator hasOverflow scrollContainerRef={scrollContainerRef} />
)
const indicator = getByTestId('tab-strip-scroll-indicator')
Object.defineProperty(indicator, 'clientWidth', { value: 400, configurable: true })
@@ -216,9 +189,9 @@ describe('TabStripScrollIndicator', () => {
fireEvent.pointerDown(thumb, { button: 0, clientX: 50 })
expect(indicator.className).toContain('h-[4px]')
// Move pointer by 60px
// 60px of a 240px thumb travel scrolls 60/240 of the 600px scroll range
fireEvent(window, new MouseEvent('pointermove', { clientX: 110 }))
expect(scrollContainer.scrollLeft).toBeGreaterThan(0)
expect(scrollContainer.scrollLeft).toBe(150)
// Release drag
fireEvent(window, new MouseEvent('pointerup'))
@@ -235,7 +208,7 @@ describe('TabStripScrollIndicator', () => {
;(scrollContainerRef as React.MutableRefObject<HTMLElement>).current = scrollContainer
const { getByTestId, rerender } = render(
<TabStripScrollIndicator metrics={OVERFLOW_METRICS} scrollContainerRef={scrollContainerRef} />
<TabStripScrollIndicator hasOverflow scrollContainerRef={scrollContainerRef} />
)
const indicator = getByTestId('tab-strip-scroll-indicator')
Object.defineProperty(indicator, 'clientWidth', { value: 400, configurable: true })
@@ -245,7 +218,7 @@ describe('TabStripScrollIndicator', () => {
rerender(
<TabStripScrollIndicator
metrics={OVERFLOW_METRICS}
hasOverflow
scrollContainerRef={scrollContainerRef}
disabled={true}
/>
@@ -257,4 +230,62 @@ describe('TabStripScrollIndicator', () => {
fireEvent(window, new MouseEvent('pointermove', { clientX: 300 }))
expect(scrollContainer.scrollLeft).toBe(0)
})
it('sizes the thumb when overflow appears and keeps it in step with scrolling and tab changes', () => {
const scrollContainer = document.createElement('div')
Object.defineProperty(scrollContainer, 'scrollWidth', { value: 1000, configurable: true })
Object.defineProperty(scrollContainer, 'clientWidth', { value: 400, configurable: true })
const scrollContainerRef = createRef<HTMLElement>()
;(scrollContainerRef as React.MutableRefObject<HTMLElement>).current = scrollContainer
const clientWidth = Object.getOwnPropertyDescriptor(HTMLElement.prototype, 'clientWidth')!
vi.spyOn(HTMLElement.prototype, 'clientWidth', 'get').mockImplementation(function (
this: HTMLElement
) {
return this.dataset.testid === 'tab-strip-scroll-indicator'
? 400
: clientWidth.get!.call(this)
})
const stripResizeListeners = new Set<() => void>()
const subscribeToStripResize = (listener: () => void): (() => void) => {
stripResizeListeners.add(listener)
return () => stripResizeListeners.delete(listener)
}
const { getByTestId, rerender, unmount } = render(
<TabStripScrollIndicator
hasOverflow={false}
scrollContainerRef={scrollContainerRef}
subscribeToStripResize={subscribeToStripResize}
/>
)
rerender(
<TabStripScrollIndicator
hasOverflow
scrollContainerRef={scrollContainerRef}
subscribeToStripResize={subscribeToStripResize}
/>
)
const thumb = getByTestId('tab-strip-scroll-thumb')
expect(thumb.style.width).toBe('160px')
expect(thumb.style.transform).toBe('translateX(0px)')
scrollContainer.scrollLeft = 300
fireEvent.scroll(scrollContainer)
expect(thumb.style.transform).toBe('translateX(120px)')
// The thumb's geometry is not a React prop, so a re-render for hover must not reset it.
fireEvent.pointerEnter(getByTestId('tab-strip-scroll-indicator'))
expect(getByTestId('tab-strip-scroll-thumb')).toBe(thumb)
expect(thumb.style.width).toBe('160px')
expect(thumb.style.transform).toBe('translateX(120px)')
// A tab opening grows the strip without a scroll event.
Object.defineProperty(scrollContainer, 'scrollWidth', { value: 2000, configurable: true })
stripResizeListeners.forEach((listener) => listener())
expect(thumb.style.width).toBe('80px')
unmount()
expect(stripResizeListeners.size).toBe(0)
})
})
@@ -1,52 +1,52 @@
import React, { useCallback, useEffect, useLayoutEffect, useRef, useState } from 'react'
import {
computeTabStripThumbLayout,
type TabStripScrollMetrics,
type TabStripThumbLayout
} from './tab-strip-scroll-metrics'
const EMPTY_THUMB_LAYOUT: TabStripThumbLayout = { widthPx: 0, leftPx: 0 }
import React, { useEffect, useLayoutEffect, useRef, useState } from 'react'
import { computeTabStripThumbLayout } from './tab-strip-scroll-metrics'
export type TabStripScrollIndicatorProps = {
metrics: TabStripScrollMetrics
hasOverflow: boolean
scrollContainerRef?: React.RefObject<HTMLElement | null>
/** Called when tabs change the strip's size without a scroll event; returns an unsubscribe. */
subscribeToStripResize?: (listener: () => void) => () => void
disabled?: boolean
}
export function TabStripScrollIndicator({
metrics,
hasOverflow,
scrollContainerRef,
subscribeToStripResize,
disabled = false
}: TabStripScrollIndicatorProps): React.JSX.Element | null {
const trackRef = useRef<HTMLDivElement>(null)
const thumbRef = useRef<HTMLDivElement>(null)
const cleanupDragRef = useRef<(() => void) | null>(null)
const [thumbLayout, setThumbLayout] = useState<TabStripThumbLayout>(EMPTY_THUMB_LAYOUT)
const [isHovered, setIsHovered] = useState(false)
const [isDragging, setIsDragging] = useState(false)
const [isScrolling, setIsScrolling] = useState(false)
const scrollTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null)
const remeasureThumb = useCallback((): void => {
const track = trackRef.current
if (!track) {
return
}
setThumbLayout(computeTabStripThumbLayout(track.clientWidth, metrics))
}, [metrics])
useLayoutEffect(() => {
remeasureThumb()
}, [remeasureThumb])
// Why write the thumb's style directly: it moves every scroll frame, and React state would re-render per frame.
useLayoutEffect(() => {
const track = trackRef.current
if (!track) {
const thumb = thumbRef.current
const scrollContainer = scrollContainerRef?.current
if (!track || !thumb || !scrollContainer) {
return
}
const resizeObserver = new ResizeObserver(remeasureThumb)
resizeObserver.observe(track)
return () => resizeObserver.disconnect()
}, [remeasureThumb])
const syncThumb = (): void => {
const layout = computeTabStripThumbLayout(track.clientWidth, scrollContainer)
thumb.style.width = `${layout.widthPx}px`
thumb.style.transform = `translateX(${layout.leftPx}px)`
}
syncThumb()
scrollContainer.addEventListener('scroll', syncThumb, { passive: true })
const trackResizeObserver = new ResizeObserver(syncThumb)
trackResizeObserver.observe(track)
const unsubscribeStripResize = subscribeToStripResize?.(syncThumb)
return () => {
scrollContainer.removeEventListener('scroll', syncThumb)
trackResizeObserver.disconnect()
unsubscribeStripResize?.()
}
}, [hasOverflow, scrollContainerRef, subscribeToStripResize])
useEffect(() => {
const scrollContainer = scrollContainerRef?.current
@@ -79,10 +79,10 @@ export function TabStripScrollIndicator({
// Why: hiding the indicator mid-drag would otherwise leave window listeners and body cursor/user-select stuck.
useEffect(() => {
if (disabled || !metrics.hasOverflow) {
if (disabled || !hasOverflow) {
cleanupDragRef.current?.()
}
}, [disabled, metrics.hasOverflow])
}, [disabled, hasOverflow])
const handleThumbPointerDown = (e: React.PointerEvent<HTMLDivElement>): void => {
if (e.button !== 0 || disabled) {
@@ -100,7 +100,8 @@ export function TabStripScrollIndicator({
const startX = e.clientX
const startScrollLeft = scrollContainer.scrollLeft
const trackWidth = track.clientWidth
const maxLeft = Math.max(1, trackWidth - thumbLayout.widthPx)
const thumbWidth = computeTabStripThumbLayout(trackWidth, scrollContainer).widthPx
const maxLeft = Math.max(1, trackWidth - thumbWidth)
const maxScrollLeft = Math.max(0, scrollContainer.scrollWidth - scrollContainer.clientWidth)
if (maxScrollLeft <= 0 || maxLeft <= 0) {
@@ -157,7 +158,7 @@ export function TabStripScrollIndicator({
const trackRect = track.getBoundingClientRect()
const clickX = e.clientX - trackRect.left
const trackWidth = track.clientWidth
const thumbWidth = thumbLayout.widthPx
const thumbWidth = computeTabStripThumbLayout(trackWidth, scrollContainer).widthPx
const maxLeft = Math.max(1, trackWidth - thumbWidth)
const maxScrollLeft = Math.max(0, scrollContainer.scrollWidth - scrollContainer.clientWidth)
@@ -184,7 +185,7 @@ export function TabStripScrollIndicator({
scrollContainer.scrollLeft += delta
}
if (!metrics.hasOverflow) {
if (!hasOverflow) {
return null
}
@@ -214,18 +215,15 @@ export function TabStripScrollIndicator({
aria-hidden
>
<div
ref={thumbRef}
data-testid="tab-strip-scroll-thumb"
className={`absolute bottom-0 h-full rounded-full transition-colors duration-150 ease-out ${
className={`absolute bottom-0 left-0 h-full rounded-full will-change-transform transition-colors duration-150 ease-out ${
isDragging
? 'bg-foreground/70 cursor-grabbing'
: isHovered
? 'bg-muted-foreground/80 cursor-grab'
: 'bg-muted-foreground/60 cursor-default'
}`}
style={{
width: `${thumbLayout.widthPx}px`,
left: `${thumbLayout.leftPx}px`
}}
onPointerDown={handleThumbPointerDown}
/>
</div>
@@ -88,8 +88,13 @@ export function renderTabBarSurface({
} = createMenu
const { orderedItems, sortableIds, dropIndicatorByVisibleId } = itemProjection
const clientHostedBrowserRows = props.clientHostedBrowserRows ?? EMPTY_CLIENT_HOSTED_ROWS
const { tabStripRef, tabStripOverflowState, activeTabDockSide, scrollTabStrip } =
tabStripNavigation
const {
tabStripRef,
tabStripOverflowState,
activeTabDockSide,
scrollTabStrip,
subscribeToStripResize
} = tabStripNavigation
const includeTopTabBorder = tabStripChrome !== 'floating-panel'
const renderedItems = renderTabBarItems({
items: orderedItems,
@@ -166,8 +171,9 @@ export function renderTabBarSurface({
) : null}
</div>
<TabStripScrollIndicator
metrics={tabStripOverflowState}
hasOverflow={tabStripOverflowState.hasOverflow}
scrollContainerRef={tabStripRef}
subscribeToStripResize={subscribeToStripResize}
disabled={tabStripDragScroll.isTabDragActive}
/>
</div>
@@ -1,6 +1,6 @@
// @vitest-environment happy-dom
import { act, cleanup, render } from '@testing-library/react'
import { afterEach, beforeEach, describe, expect, it } from 'vitest'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { useTabStripOverflowNavigation } from './tab-strip-overflow-navigation'
const TAB_WIDTH = 100
@@ -80,6 +80,8 @@ function restoreStripLayout(): void {
}
const NO_HOSTED_ROWS: string[] = []
let stripRenderCount = 0
let subscribeToStripResize: ((listener: () => void) => () => void) | null = null
/** `hostedRows` render like client-hosted browser rows: a strip slot with no `data-tab-id`. */
function Strip({
@@ -93,12 +95,14 @@ function Strip({
hostedRows?: string[]
activeHostedRow?: string | null
}): React.JSX.Element {
stripRenderCount++
const navigation = useTabStripOverflowNavigation({
activeVisibleTabId: active,
activeDockSlotId: activeHostedRow ?? active,
layoutKey: [...tabs, ...hostedRows].join(','),
worktreeId: 'wt-1'
})
subscribeToStripResize = navigation.subscribeToStripResize
return (
<div
data-strip=""
@@ -301,3 +305,30 @@ describe('tab strip with a docked active tab', () => {
expect(tabX(strip, 'N')).toBe(100)
})
})
describe('tab strip while scrolling', () => {
beforeEach(installStripLayout)
afterEach(() => {
cleanup()
restoreStripLayout()
})
it('does not re-render the strip for a scroll that stays between the edges', () => {
const { strip } = mountScrolled('E', 300)
const rendersBefore = stripRenderCount
act(() => {
strip.scrollLeft = 400
strip.dispatchEvent(new Event('scroll'))
})
expect(stripRenderCount).toBe(rendersBefore)
})
it('tells resize subscribers when tabs change the strip without a scroll', async () => {
const { rerender } = mountScrolled('J', 700)
const listener = vi.fn()
subscribeToStripResize!(listener)
rerender(<Strip tabs={[...TABS, 'N']} active="J" />)
await Promise.resolve()
expect(listener).toHaveBeenCalled()
})
})
@@ -1,9 +1,9 @@
import { useCallback, useEffect, useLayoutEffect, useRef, useState, type RefObject } from 'react'
import { bindTabStripContentResizeObservers } from './tab-strip-content-resize-observers'
import {
computeTabStripScrollMetrics,
sameTabStripScrollMetrics,
type TabStripScrollMetrics
computeTabStripOverflowState,
sameTabStripOverflowState,
type TabStripOverflowState
} from './tab-strip-scroll-metrics'
import { isTabStripPointerGestureActive } from './tab-strip-pointer-gesture'
import {
@@ -44,12 +44,10 @@ function isTabStripScrolledToEnd(el: HTMLElement): boolean {
return el.scrollLeft >= max - 2
}
const EMPTY_TAB_STRIP_OVERFLOW_STATE: TabStripScrollMetrics = {
const EMPTY_TAB_STRIP_OVERFLOW_STATE: TabStripOverflowState = {
hasOverflow: false,
canScrollStart: false,
canScrollEnd: false,
thumbSizeFraction: 1,
thumbOffsetFraction: 0
canScrollEnd: false
}
export function useTabStripOverflowNavigation({
@@ -65,11 +63,13 @@ export function useTabStripOverflowNavigation({
worktreeId: string
}): {
tabStripRef: RefObject<HTMLDivElement | null>
tabStripOverflowState: TabStripScrollMetrics
tabStripOverflowState: TabStripOverflowState
activeTabDockSide: ActiveTabDockSide | null
scrollTabStrip: (direction: 'start' | 'end', behavior?: ScrollBehavior) => void
subscribeToStripResize: (listener: () => void) => () => void
} {
const tabStripRef = useRef<HTMLDivElement>(null)
const stripResizeListenersRef = useRef<Set<() => void>>(new Set())
const prevStripRef = useRef<{ worktreeId: string; tabIds: ReadonlySet<string> } | null>(null)
const stickToEndRef = useRef(false)
const tabClosedThisCommitRef = useRef(false)
@@ -79,7 +79,8 @@ export function useTabStripOverflowNavigation({
activeTabId: string | null
anchor: TabStripScrollAnchor | null
} | null>(null)
const [tabStripOverflowState, setTabStripOverflowState] = useState<TabStripScrollMetrics>(
// Why no thumb position here: it changes every scroll frame, and every tab would re-render with it.
const [tabStripOverflowState, setTabStripOverflowState] = useState<TabStripOverflowState>(
EMPTY_TAB_STRIP_OVERFLOW_STATE
)
const [activeTabDockSide, setActiveTabDockSide] = useState<ActiveTabDockSide | null>(null)
@@ -88,9 +89,9 @@ export function useTabStripOverflowNavigation({
if (!el) {
return
}
const next = computeTabStripScrollMetrics(el)
const next = computeTabStripOverflowState(el)
setTabStripOverflowState((previous) =>
sameTabStripScrollMetrics(previous, next) ? previous : next
sameTabStripOverflowState(previous, next) ? previous : next
)
setActiveTabDockSide(getActiveTabDockSide(el))
}, [])
@@ -151,6 +152,9 @@ export function useTabStripOverflowNavigation({
el.scrollLeft = Math.max(0, el.scrollWidth - el.clientWidth)
}
recordScrollAnchor()
for (const listener of stripResizeListenersRef.current) {
listener()
}
}
const disconnectResizeObservers = bindTabStripContentResizeObservers(el, handleStripResize)
@@ -300,5 +304,19 @@ export function useTabStripOverflowNavigation({
tabClosedThisCommitRef.current = false
})
return { tabStripRef, tabStripOverflowState, activeTabDockSide, scrollTabStrip }
// Why share these observers: each one watches every tab, so a second set doubles that work.
const subscribeToStripResize = useCallback((listener: () => void): (() => void) => {
stripResizeListenersRef.current.add(listener)
return () => {
stripResizeListenersRef.current.delete(listener)
}
}, [])
return {
tabStripRef,
tabStripOverflowState,
activeTabDockSide,
scrollTabStrip,
subscribeToStripResize
}
}
@@ -1,14 +1,14 @@
import { describe, expect, it } from 'vitest'
import {
computeTabStripScrollMetrics,
computeTabStripOverflowState,
computeTabStripThumbLayout,
getTabStripScrollMaskClassName
} from './tab-strip-scroll-metrics'
describe('computeTabStripScrollMetrics', () => {
describe('computeTabStripOverflowState', () => {
it('reports no overflow when all tabs fit', () => {
expect(
computeTabStripScrollMetrics({
computeTabStripOverflowState({
scrollWidth: 400,
clientWidth: 400,
scrollLeft: 0
@@ -16,38 +16,20 @@ describe('computeTabStripScrollMetrics', () => {
).toEqual({
hasOverflow: false,
canScrollStart: false,
canScrollEnd: false,
thumbSizeFraction: 1,
thumbOffsetFraction: 0
})
})
it('tracks thumb size and offset while scrolled', () => {
expect(
computeTabStripScrollMetrics({
scrollWidth: 800,
clientWidth: 400,
scrollLeft: 200
})
).toEqual({
hasOverflow: true,
canScrollStart: true,
canScrollEnd: true,
thumbSizeFraction: 0.5,
thumbOffsetFraction: 0.5
canScrollEnd: false
})
})
it('marks the start and end scroll edges', () => {
expect(
computeTabStripScrollMetrics({
computeTabStripOverflowState({
scrollWidth: 800,
clientWidth: 400,
scrollLeft: 0
}).canScrollStart
).toBe(false)
expect(
computeTabStripScrollMetrics({
computeTabStripOverflowState({
scrollWidth: 800,
clientWidth: 400,
scrollLeft: 0
@@ -55,14 +37,14 @@ describe('computeTabStripScrollMetrics', () => {
).toBe(true)
expect(
computeTabStripScrollMetrics({
computeTabStripOverflowState({
scrollWidth: 800,
clientWidth: 400,
scrollLeft: 400
}).canScrollStart
).toBe(true)
expect(
computeTabStripScrollMetrics({
computeTabStripOverflowState({
scrollWidth: 800,
clientWidth: 400,
scrollLeft: 400
@@ -74,22 +56,16 @@ describe('computeTabStripScrollMetrics', () => {
describe('computeTabStripThumbLayout', () => {
it('clamps thumb width and keeps the thumb inside the track', () => {
expect(
computeTabStripThumbLayout(200, {
thumbSizeFraction: 0.04,
thumbOffsetFraction: 1
})
computeTabStripThumbLayout(200, { scrollWidth: 10_000, clientWidth: 400, scrollLeft: 9_600 })
).toEqual({
widthPx: 18,
leftPx: 182
})
})
it('uses the raw width when it is already above the minimum', () => {
it('sizes and offsets the thumb from the strip scroll position', () => {
expect(
computeTabStripThumbLayout(400, {
thumbSizeFraction: 0.5,
thumbOffsetFraction: 0.25
})
computeTabStripThumbLayout(400, { scrollWidth: 800, clientWidth: 400, scrollLeft: 100 })
).toEqual({
widthPx: 200,
leftPx: 50
@@ -1,13 +1,9 @@
import type { ActiveTabDockSide } from './tab-strip-slot-geometry'
export type TabStripScrollMetrics = {
export type TabStripOverflowState = {
hasOverflow: boolean
canScrollStart: boolean
canScrollEnd: boolean
/** Portion of total tab width currently visible in the strip viewport. */
thumbSizeFraction: number
/** 0 = scrolled to start, 1 = scrolled to end. */
thumbOffsetFraction: number
}
export type TabStripThumbLayout = {
@@ -15,48 +11,49 @@ export type TabStripThumbLayout = {
leftPx: number
}
type TabStripScrollBox = Pick<HTMLElement, 'scrollWidth' | 'clientWidth' | 'scrollLeft'>
export const TAB_STRIP_THUMB_MIN_WIDTH_PX = 18
const OVERFLOW_EPSILON_PX = 1
export function computeTabStripScrollMetrics(
el: Pick<HTMLElement, 'scrollWidth' | 'clientWidth' | 'scrollLeft'>
): TabStripScrollMetrics {
export function computeTabStripOverflowState(el: TabStripScrollBox): TabStripOverflowState {
const maxScrollLeft = Math.max(0, el.scrollWidth - el.clientWidth)
const hasOverflow = maxScrollLeft > OVERFLOW_EPSILON_PX
const thumbSizeFraction = el.scrollWidth > 0 ? Math.min(1, el.clientWidth / el.scrollWidth) : 1
const thumbOffsetFraction = hasOverflow && maxScrollLeft > 0 ? el.scrollLeft / maxScrollLeft : 0
return {
hasOverflow,
canScrollStart: hasOverflow && el.scrollLeft > OVERFLOW_EPSILON_PX,
canScrollEnd: hasOverflow && el.scrollLeft < maxScrollLeft - OVERFLOW_EPSILON_PX,
thumbSizeFraction,
thumbOffsetFraction
canScrollEnd: hasOverflow && el.scrollLeft < maxScrollLeft - OVERFLOW_EPSILON_PX
}
}
export function computeTabStripThumbLayout(
trackWidthPx: number,
metrics: Pick<TabStripScrollMetrics, 'thumbSizeFraction' | 'thumbOffsetFraction'>
el: TabStripScrollBox
): TabStripThumbLayout {
if (trackWidthPx <= 0) {
return { widthPx: 0, leftPx: 0 }
}
const rawWidthPx = metrics.thumbSizeFraction * trackWidthPx
const widthPx = Math.min(trackWidthPx, Math.max(TAB_STRIP_THUMB_MIN_WIDTH_PX, rawWidthPx))
const maxScrollLeft = Math.max(0, el.scrollWidth - el.clientWidth)
const sizeFraction = el.scrollWidth > 0 ? Math.min(1, el.clientWidth / el.scrollWidth) : 1
const offsetFraction = maxScrollLeft > OVERFLOW_EPSILON_PX ? el.scrollLeft / maxScrollLeft : 0
const widthPx = Math.min(
trackWidthPx,
Math.max(TAB_STRIP_THUMB_MIN_WIDTH_PX, sizeFraction * trackWidthPx)
)
const maxLeftPx = Math.max(0, trackWidthPx - widthPx)
return {
widthPx,
leftPx: metrics.thumbOffsetFraction * maxLeftPx
leftPx: offsetFraction * maxLeftPx
}
}
/** `dockedSide` skips that edge's fade, which would otherwise wash out the active tab docked there. */
export function getTabStripScrollMaskClassName(
metrics: Pick<TabStripScrollMetrics, 'canScrollStart' | 'canScrollEnd' | 'hasOverflow'>,
metrics: TabStripOverflowState,
dockedSide: ActiveTabDockSide | null = null
): string {
if (!metrics.hasOverflow) {
@@ -73,15 +70,13 @@ export function getTabStripScrollMaskClassName(
return classes.join(' ')
}
export function sameTabStripScrollMetrics(
left: TabStripScrollMetrics,
right: TabStripScrollMetrics
export function sameTabStripOverflowState(
left: TabStripOverflowState,
right: TabStripOverflowState
): boolean {
return (
left.hasOverflow === right.hasOverflow &&
left.canScrollStart === right.canScrollStart &&
left.canScrollEnd === right.canScrollEnd &&
Math.abs(left.thumbSizeFraction - right.thumbSizeFraction) < 0.002 &&
Math.abs(left.thumbOffsetFraction - right.thumbOffsetFraction) < 0.002
left.canScrollEnd === right.canScrollEnd
)
}
@@ -0,0 +1,250 @@
/**
* E2E test for scrolling an overflowing tab strip: the scroll must not re-render the tabs, and the
* scroll thumb must still track the strip.
*
* Why E2E: only real Chromium lays out the strip and delivers the scroll and resize callbacks the
* thumb follows, and only the whole app shows every React commit a scroll step causes.
*/
import type { Locator, Page } from '@stablyai/playwright-test'
import { test, expect } from './helpers/orca-app'
import { waitForSessionReady, waitForActiveWorktree, ensureTerminalVisible } from './helpers/store'
const STRIP = '.terminal-tab-strip'
const START_SCROLL_LEFT_PX = 400
const WHEEL_STEPS = 20
const WHEEL_DELTA_PX = 50
const MIN_SCROLL_RANGE_PX = 2_000
type CommitFiber = {
tag: number
flags: number
child: CommitFiber | null
sibling: CommitFiber | null
stateNode: unknown
}
declare global {
// oxlint-disable-next-line typescript-eslint/consistent-type-definitions -- declaration merging requires interface
interface Window {
__REACT_DEVTOOLS_GLOBAL_HOOK__?: {
onCommitFiberRoot?: (
rendererId: unknown,
root: { current: CommitFiber },
...rest: unknown[]
) => unknown
}
__tabsRenderedPerCommit?: number[]
}
}
async function addBackgroundTerminalTabs(
page: Page,
worktreeId: string,
count: number
): Promise<string[]> {
return page.evaluate(
({ wId, tabCount }) =>
Array.from(
{ length: tabCount },
() =>
window.__store!.getState().createTab(wId, undefined, undefined, { activate: false }).id
),
{ wId: worktreeId, tabCount: count }
)
}
/**
* Records how many tabs each React commit re-renders, the way React DevTools highlights updates:
* through the commit hook the renderer always installs. Commits that render no tab (the sidebar,
* terminals starting up) are left out.
*/
async function startRecordingTabRenders(page: Page): Promise<void> {
await page.evaluate(() => {
// Function, class, forwardRef and memo components; bit 1 is React's PerformedWork flag.
const componentTags = new Set([0, 1, 11, 14, 15])
const performedWork = 1
const hook = window.__REACT_DEVTOOLS_GLOBAL_HOOK__!
const original = hook.onCommitFiberRoot
// A subtree React skipped keeps last commit's fiber objects, whose flags are stale.
let previousFibers = new Set<CommitFiber>()
window.__tabsRenderedPerCommit = []
hook.onCommitFiberRoot = function (rendererId, root, ...rest) {
const fibers = new Set<CommitFiber>()
const renderedTabIds = new Set<string>()
const stack: CommitFiber[] = [root.current]
while (stack.length > 0) {
const fiber = stack.pop()!
fibers.add(fiber)
if (
componentTags.has(fiber.tag) &&
(fiber.flags & performedWork) === performedWork &&
!previousFibers.has(fiber)
) {
let host: CommitFiber | null = fiber
while (host && host.tag !== 5) {
host = host.child
}
const element = host?.stateNode
const tabId =
element instanceof Element
? element.closest('[data-tab-strip-slot]')?.getAttribute('data-tab-strip-slot')
: undefined
if (tabId) {
renderedTabIds.add(tabId)
}
}
if (fiber.sibling) {
stack.push(fiber.sibling)
}
if (fiber.child) {
stack.push(fiber.child)
}
}
previousFibers = fibers
if (renderedTabIds.size > 0) {
window.__tabsRenderedPerCommit?.push(renderedTabIds.size)
}
return original?.call(this, rendererId, root, ...rest)
}
})
}
async function takeTabRenders(page: Page): Promise<number[]> {
return page.evaluate(() => {
const recorded = window.__tabsRenderedPerCommit ?? []
window.__tabsRenderedPerCommit = []
return recorded
})
}
/** Thumb geometry next to where the strip's scroll position and size say it should be. */
async function readThumb(strip: Locator) {
return strip.evaluate((el) => {
const track = el.parentElement!.querySelector<HTMLElement>(
'[data-testid="tab-strip-scroll-indicator"]'
)!
const trackRect = track.getBoundingClientRect()
const thumbRect = track
.querySelector<HTMLElement>('[data-testid="tab-strip-scroll-thumb"]')!
.getBoundingClientRect()
const maxScrollLeft = el.scrollWidth - el.clientWidth
const expectedWidth = Math.max(18, (el.clientWidth / el.scrollWidth) * trackRect.width)
return {
width: thumbRect.width,
expectedWidth,
left: thumbRect.left - trackRect.left,
expectedLeft: (el.scrollLeft / maxScrollLeft) * (trackRect.width - expectedWidth)
}
})
}
/** Polls rather than waiting frames: CI's hidden window can draw as little as one frame a second. */
async function expectThumbToTrackStrip(strip: Locator): Promise<void> {
await expect
.poll(async () => {
const thumb = await readThumb(strip)
return Math.max(
Math.abs(thumb.width - thumb.expectedWidth),
Math.abs(thumb.left - thumb.expectedLeft)
)
})
.toBeLessThanOrEqual(1)
}
test.describe('Tab strip scroll render isolation', () => {
test.beforeEach(async ({ orcaPage }) => {
await waitForSessionReady(orcaPage)
await waitForActiveWorktree(orcaPage)
await ensureTerminalVisible(orcaPage)
})
test('scrolls a long strip without re-rendering its tabs, and the thumb keeps up', async ({
orcaPage
}) => {
const worktreeId = await waitForActiveWorktree(orcaPage)
const strip = orcaPage.locator(STRIP).first()
await expect(strip).toBeVisible()
const tabIds: string[] = []
for (let i = 0; i < 12; i++) {
const range = await strip.evaluate((el) => el.scrollWidth - el.clientWidth)
if (range >= MIN_SCROLL_RANGE_PX) {
break
}
tabIds.push(...(await addBackgroundTerminalTabs(orcaPage, worktreeId, 10)))
await expect
.poll(() => strip.evaluate((el) => el.querySelectorAll('[data-tab-strip-slot]').length))
.toBeGreaterThan(tabIds.length)
}
await startRecordingTabRenders(orcaPage)
// New terminals retitle their tabs after opening, longer on a loaded machine; wait for quiet.
await expect
.poll(
async () => {
await takeTabRenders(orcaPage)
await orcaPage.waitForTimeout(1_500)
return (await takeTabRenders(orcaPage)).length
},
{ timeout: 45_000 }
)
.toBe(0)
expect(await strip.evaluate((el) => el.scrollWidth - el.clientWidth)).toBeGreaterThanOrEqual(
MIN_SCROLL_RANGE_PX
)
// Start and end mid-strip so neither scroll edge flips the arrows, fades or dock. Why set it
// here and hold until it sticks: opening the tabs left the strip pinned to its end, and the
// scroll event that releases that pin only lands with the next frame — about a second in CI's
// hidden window. A tab retitling inside that window re-pins the strip to the end.
await expect
.poll(
async () => {
await strip.evaluate((el, left) => {
el.scrollLeft = left
}, START_SCROLL_LEFT_PX)
await orcaPage.waitForTimeout(1_500)
return strip.evaluate((el) => el.scrollLeft)
},
{ timeout: 30_000 }
)
.toBe(START_SCROLL_LEFT_PX)
// That scroll flipped the end-edge flags; the commit it caused is not the scroll under test.
await takeTabRenders(orcaPage)
// Why wheel events dispatched in the page: real input waits for a frame per step, and CI's hidden
// window draws about one a second. The strip's own wheel handler still does the scrolling.
await strip.evaluate(
async (el, { steps, delta }) => {
for (let i = 0; i < steps; i++) {
el.dispatchEvent(new WheelEvent('wheel', { deltaY: delta, cancelable: true }))
await new Promise((resolve) => setTimeout(resolve, 20))
}
// The scroll events arrive with the next frame; let React commit what they cause.
await new Promise(requestAnimationFrame)
await new Promise((resolve) => setTimeout(resolve, 20))
},
{ steps: WHEEL_STEPS, delta: WHEEL_DELTA_PX }
)
expect(await strip.evaluate((el) => el.scrollLeft)).toBe(
START_SCROLL_LEFT_PX + WHEEL_STEPS * WHEEL_DELTA_PX
)
// Why "many tabs": before the fix every scroll step re-rendered all of them in one commit;
// a single tab can still update itself (a late retitle).
expect((await takeTabRenders(orcaPage)).filter((tabs) => tabs >= 3)).toEqual([])
await expectThumbToTrackStrip(strip)
// Control: switching tabs re-renders at least the tab it activates, so the recorder sees tabs.
await orcaPage.evaluate((tabId) => window.__store!.getState().setActiveTab(tabId), tabIds[0])
await expect.poll(async () => (await takeTabRenders(orcaPage)).length).toBeGreaterThan(0)
// Tabs opening grow the strip without a scroll event.
await addBackgroundTerminalTabs(orcaPage, worktreeId, 10)
await expectThumbToTrackStrip(strip)
// A narrower pane shrinks the strip and the track without a scroll event.
await strip.evaluate((el) => {
el.closest<HTMLElement>('[data-native-file-drop-target]')!.style.maxWidth = '700px'
})
await expectThumbToTrackStrip(strip)
})
})