{
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(
)
+ await Promise.resolve()
+ expect(listener).toHaveBeenCalled()
+ })
+})
diff --git a/src/renderer/src/components/tab-bar/tab-strip-overflow-navigation.ts b/src/renderer/src/components/tab-bar/tab-strip-overflow-navigation.ts
index 29508efb3e6..df1f292bfb4 100644
--- a/src/renderer/src/components/tab-bar/tab-strip-overflow-navigation.ts
+++ b/src/renderer/src/components/tab-bar/tab-strip-overflow-navigation.ts
@@ -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
- tabStripOverflowState: TabStripScrollMetrics
+ tabStripOverflowState: TabStripOverflowState
activeTabDockSide: ActiveTabDockSide | null
scrollTabStrip: (direction: 'start' | 'end', behavior?: ScrollBehavior) => void
+ subscribeToStripResize: (listener: () => void) => () => void
} {
const tabStripRef = useRef(null)
+ const stripResizeListenersRef = useRef void>>(new Set())
const prevStripRef = useRef<{ worktreeId: string; tabIds: ReadonlySet } | 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(
+ // Why no thumb position here: it changes every scroll frame, and every tab would re-render with it.
+ const [tabStripOverflowState, setTabStripOverflowState] = useState(
EMPTY_TAB_STRIP_OVERFLOW_STATE
)
const [activeTabDockSide, setActiveTabDockSide] = useState(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
+ }
}
diff --git a/src/renderer/src/components/tab-bar/tab-strip-scroll-metrics.test.ts b/src/renderer/src/components/tab-bar/tab-strip-scroll-metrics.test.ts
index cbaa69603a5..8ede8fdd39e 100644
--- a/src/renderer/src/components/tab-bar/tab-strip-scroll-metrics.test.ts
+++ b/src/renderer/src/components/tab-bar/tab-strip-scroll-metrics.test.ts
@@ -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
diff --git a/src/renderer/src/components/tab-bar/tab-strip-scroll-metrics.ts b/src/renderer/src/components/tab-bar/tab-strip-scroll-metrics.ts
index 39489e21a74..9f467c06d3c 100644
--- a/src/renderer/src/components/tab-bar/tab-strip-scroll-metrics.ts
+++ b/src/renderer/src/components/tab-bar/tab-strip-scroll-metrics.ts
@@ -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
+
export const TAB_STRIP_THUMB_MIN_WIDTH_PX = 18
const OVERFLOW_EPSILON_PX = 1
-export function computeTabStripScrollMetrics(
- el: Pick
-): 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
+ 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,
+ 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
)
}
diff --git a/tests/e2e/tab-strip-scroll-render-isolation.spec.ts b/tests/e2e/tab-strip-scroll-render-isolation.spec.ts
new file mode 100644
index 00000000000..ca1336203d5
--- /dev/null
+++ b/tests/e2e/tab-strip-scroll-render-isolation.spec.ts
@@ -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 {
+ 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 {
+ 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()
+ window.__tabsRenderedPerCommit = []
+ hook.onCommitFiberRoot = function (rendererId, root, ...rest) {
+ const fibers = new Set()
+ const renderedTabIds = new Set()
+ 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 {
+ 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(
+ '[data-testid="tab-strip-scroll-indicator"]'
+ )!
+ const trackRect = track.getBoundingClientRect()
+ const thumbRect = track
+ .querySelector('[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 {
+ 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('[data-native-file-drop-target]')!.style.maxWidth = '700px'
+ })
+ await expectThumbToTrackStrip(strip)
+ })
+})