mirror of
https://github.com/NanmiCoder/claude-code-haha.git
synced 2026-10-10 11:53:10 +08:00
fix(desktop): keep the active tab whole when the strip resizes under it
The rightmost tab lost its right edge whenever it was the active one, close button included. Not merely hidden either: the button's centre sat past the strip, so elementFromPoint there returned the toolbar's terminal button and that tab could not be closed at all. The activation scroll is the cause, but only together with the chevrons. They are w-7 siblings of the scroll region, so the moment updateScrollState decides the strip overflows they take 28px each out of a flex-1 region — after scrollIntoView has already landed on a scrollLeft computed without them, and scrollLeft does not follow a layout change. Measured on a 1280px window with seven tabs: the scroll stopped at 108 when the reachable end had moved to 164, and the tab lost exactly those 56px. So keeping the active tab whole cannot be a one-shot on activation; it is an invariant the strip has to re-establish whenever its own width changes. It now runs off the ResizeObserver that was already there, which covers window resizes, sidebar drags and the toolbar's conditional buttons for free. Guarded on whether the user has driven the strip themselves, not on the strip getting narrower. Width was tried first and is wrong: a chevron retires when its end is reached and rejoins when it is left, so a plain chevron press narrows the strip mid-flight and is indistinguishable from the layout event being guarded against. Measured, that snapped the view straight back and made the left end unreachable. Switching tabs hands the position back — the user has just named a tab they want to see. Four tests, two of them pinning the guard rather than the fix. Sentinel runs confirm both halves: removing the realign reddens two, removing the guard reddens the one written for it. The ResizeObserver mock now records its callback, since jsdom lays nothing out and the geometry has to be stubbed alongside it. Walked through against a live strip: rightmost tab clip 56px -> 0, close button reachable again, left end reachable, sidebar expand re-aligns by 136px.
This commit is contained in:
@@ -23,6 +23,36 @@ const sessionsApiMock = vi.hoisted(() => ({
|
||||
delete: vi.fn(() => Promise.resolve()),
|
||||
}))
|
||||
|
||||
// The strip re-reveals a clipped active tab from its ResizeObserver, so the
|
||||
// tests have to be able to fire one. jsdom lays nothing out, so the geometry
|
||||
// the guard reads has to be stubbed alongside it.
|
||||
const resizeObserverCallbacks = new Set<ResizeObserverCallback>()
|
||||
|
||||
function stubRect(element: Element, left: number, right: number) {
|
||||
Object.defineProperty(element, 'getBoundingClientRect', {
|
||||
configurable: true,
|
||||
value: () => ({
|
||||
left,
|
||||
right,
|
||||
width: right - left,
|
||||
top: 0,
|
||||
bottom: 46,
|
||||
height: 46,
|
||||
x: left,
|
||||
y: 0,
|
||||
toJSON: () => ({}),
|
||||
}),
|
||||
})
|
||||
}
|
||||
|
||||
function fireStripResize() {
|
||||
act(() => {
|
||||
for (const callback of [...resizeObserverCallbacks]) {
|
||||
callback([], {} as ResizeObserver)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
function makeChatSession(chatState: ChatState): PerSessionState {
|
||||
return {
|
||||
messages: [],
|
||||
@@ -158,12 +188,19 @@ describe('TabBar', () => {
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
resizeObserverCallbacks.clear()
|
||||
|
||||
class ResizeObserverMock {
|
||||
constructor(_callback: ResizeObserverCallback) {}
|
||||
constructor(private readonly callback: ResizeObserverCallback) {
|
||||
resizeObserverCallbacks.add(callback)
|
||||
}
|
||||
|
||||
observe(_target: Element) {}
|
||||
|
||||
disconnect() {}
|
||||
disconnect() {
|
||||
resizeObserverCallbacks.delete(this.callback)
|
||||
}
|
||||
|
||||
unobserve() {}
|
||||
}
|
||||
|
||||
@@ -880,6 +917,131 @@ describe('TabBar', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('keeping the active tab whole while the strip resizes', () => {
|
||||
// The chevrons are `w-7` siblings of the scroll region, so the moment the
|
||||
// strip is found to overflow they take 28px each out of it — after the
|
||||
// activation scroll has already landed on a scrollLeft computed without
|
||||
// them. Measured on a 1280px window with seven tabs: the scroll stopped at
|
||||
// 108 when the reachable end had moved to 164, and the last tab lost
|
||||
// exactly those 56px, taking the close button past the strip edge, where
|
||||
// elementFromPoint returned the toolbar's terminal button instead. So the
|
||||
// tab could not be closed at all, not merely not seen.
|
||||
async function renderOverflowingStrip() {
|
||||
const { TabBar } = await import('./TabBar')
|
||||
const { useTabStore } = await import('../../stores/tabStore')
|
||||
const { useChatStore } = await import('../../stores/chatStore')
|
||||
|
||||
useTabStore.setState({
|
||||
tabs: [
|
||||
{ sessionId: 'tab-1', title: 'First Session', type: 'session', status: 'idle' },
|
||||
{ sessionId: 'tab-2', title: 'Second Session', type: 'session', status: 'idle' },
|
||||
{ sessionId: 'tab-3', title: 'Third Session', type: 'session', status: 'idle' },
|
||||
{ sessionId: 'tab-4', title: 'Fourth Session', type: 'session', status: 'idle' },
|
||||
{ sessionId: 'tab-5', title: 'Last Session', type: 'session', status: 'idle' },
|
||||
],
|
||||
activeTabId: 'tab-5',
|
||||
})
|
||||
useChatStore.setState({
|
||||
sessions: {},
|
||||
disconnectSession: vi.fn(),
|
||||
} as Partial<ReturnType<typeof useChatStore.getState>>)
|
||||
|
||||
await act(async () => {
|
||||
render(<TabBar />)
|
||||
})
|
||||
|
||||
const strip = screen.getByTestId('tab-bar-scroll-region')
|
||||
const activeTab = strip.querySelector('[data-active="true"]') as HTMLElement
|
||||
expect(activeTab).toBeInTheDocument()
|
||||
|
||||
// The strip runs 0–840 once both chevrons are in. Everything below moves
|
||||
// only the active tab's own rect against that.
|
||||
stubRect(strip, 0, 840)
|
||||
Object.defineProperty(strip, 'clientWidth', { configurable: true, get: () => 840 })
|
||||
Object.defineProperty(strip, 'scrollWidth', { configurable: true, get: () => 1004 })
|
||||
Object.defineProperty(strip, 'scrollLeft', { configurable: true, get: () => 108 })
|
||||
Object.defineProperty(strip, 'scrollBy', { configurable: true, value: vi.fn() })
|
||||
|
||||
return { strip, activeTab, useTabStore }
|
||||
}
|
||||
|
||||
it('re-reveals the active tab when a resize clips it', async () => {
|
||||
const { activeTab } = await renderOverflowingStrip()
|
||||
// 56px past the strip's right edge — the close button's slot.
|
||||
stubRect(activeTab, 640, 896)
|
||||
scrollIntoViewMock.mockClear()
|
||||
|
||||
fireStripResize()
|
||||
|
||||
expect(scrollIntoViewMock).toHaveBeenCalledWith({
|
||||
block: 'nearest',
|
||||
inline: 'nearest',
|
||||
behavior: 'smooth',
|
||||
})
|
||||
})
|
||||
|
||||
it('leaves an already whole active tab where it is', async () => {
|
||||
const { activeTab } = await renderOverflowingStrip()
|
||||
stubRect(activeTab, 640, 840)
|
||||
scrollIntoViewMock.mockClear()
|
||||
|
||||
fireStripResize()
|
||||
|
||||
// Nothing is clipped, so a resize must not scroll. Without the tolerance
|
||||
// in the visibility test, subpixel edges would land here and re-scroll on
|
||||
// every single resize.
|
||||
expect(scrollIntoViewMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('leaves the strip alone once the user has driven it with a chevron', async () => {
|
||||
const { activeTab } = await renderOverflowingStrip()
|
||||
stubRect(activeTab, 640, 896)
|
||||
|
||||
const leftChevron = await waitFor(() => {
|
||||
const button = screen.getByText('chevron_left').closest('button')
|
||||
expect(button).toBeInTheDocument()
|
||||
return button as HTMLButtonElement
|
||||
})
|
||||
fireEvent.click(leftChevron)
|
||||
scrollIntoViewMock.mockClear()
|
||||
|
||||
// A chevron press is itself a resize source: reaching an end retires one
|
||||
// chevron and leaving an end brings the other back, so a plain user
|
||||
// scroll fires this observer mid-flight. Realigning there snapped the
|
||||
// view straight back and made the left end unreachable.
|
||||
fireStripResize()
|
||||
|
||||
expect(scrollIntoViewMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('takes the strip back over when the user switches tabs', async () => {
|
||||
const { activeTab, strip, useTabStore } = await renderOverflowingStrip()
|
||||
stubRect(activeTab, 640, 896)
|
||||
|
||||
const leftChevron = await waitFor(() => {
|
||||
const button = screen.getByText('chevron_left').closest('button')
|
||||
expect(button).toBeInTheDocument()
|
||||
return button as HTMLButtonElement
|
||||
})
|
||||
fireEvent.click(leftChevron)
|
||||
|
||||
await act(async () => {
|
||||
useTabStore.getState().setActiveTab('tab-4')
|
||||
})
|
||||
const nextActiveTab = strip.querySelector('[data-active="true"]') as HTMLElement
|
||||
stubRect(nextActiveTab, 640, 896)
|
||||
scrollIntoViewMock.mockClear()
|
||||
|
||||
fireStripResize()
|
||||
|
||||
expect(scrollIntoViewMock).toHaveBeenCalledWith({
|
||||
block: 'nearest',
|
||||
inline: 'nearest',
|
||||
behavior: 'smooth',
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
it('keeps the overflow button flush against window controls on Windows', async () => {
|
||||
const { TabBar } = await import('./TabBar')
|
||||
const { useTabStore } = await import('../../stores/tabStore')
|
||||
|
||||
@@ -41,6 +41,17 @@ const DRAG_START_THRESHOLD = 4
|
||||
// undershoot a row of long ones; leaving a quarter behind keeps the tab you
|
||||
// were looking at on screen as an anchor.
|
||||
const SCROLL_STEP_RATIO = 0.75
|
||||
// `nearest` on both axes: bring the tab fully on screen with the smallest
|
||||
// possible move, and leave a tab that is already whole exactly where it is.
|
||||
const REVEAL_ACTIVE_TAB: ScrollIntoViewOptions = {
|
||||
block: 'nearest',
|
||||
inline: 'nearest',
|
||||
behavior: 'smooth',
|
||||
}
|
||||
// Subpixel slack for the "is the active tab whole?" test. Without it a strip
|
||||
// whose edges land on fractional pixels reports the tab as clipped on every
|
||||
// single resize and re-scrolls forever.
|
||||
const TAB_VISIBILITY_TOLERANCE = 1
|
||||
// One glyph per *non-chat* tab kind: the glyph says "this tab is not a
|
||||
// conversation". Chat tabs deliberately have none — a bubble on every tab in a
|
||||
// strip that is mostly chats is pure noise, and the slot it occupied is worth
|
||||
@@ -170,6 +181,9 @@ export function TabBar() {
|
||||
|
||||
const moveTab = useTabStore((s) => s.moveTab)
|
||||
const scrollRef = useRef<HTMLDivElement>(null)
|
||||
// Set the moment the user drives the strip themselves, cleared when they
|
||||
// switch tabs. See `realignActiveTab`.
|
||||
const userScrolledRef = useRef(false)
|
||||
const [canScrollLeft, setCanScrollLeft] = useState(false)
|
||||
const [canScrollRight, setCanScrollRight] = useState(false)
|
||||
const [contextMenu, setContextMenu] = useState<{ sessionId: string; x: number; y: number } | null>(null)
|
||||
@@ -201,29 +215,76 @@ export function TabBar() {
|
||||
setCanScrollRight(el.scrollLeft + el.clientWidth < el.scrollWidth - 1)
|
||||
}, [])
|
||||
|
||||
// Keeping the active tab whole is an invariant the strip has to re-establish
|
||||
// after its own width changes, not something a single scroll on activation
|
||||
// can settle. The chevrons are why: they are `w-7` siblings of this region,
|
||||
// so the moment `updateScrollState` decides the strip overflows they take
|
||||
// 28px each out of it — *after* the activation scroll has already landed on
|
||||
// a scrollLeft computed without them. Measured on a 1280px window with seven
|
||||
// tabs: the scroll stopped at 108 when the reachable end had moved to 164,
|
||||
// and the last tab lost exactly those 56px off its right edge, taking the
|
||||
// close button with it. Not merely hidden — the button's centre sat past the
|
||||
// strip, so `elementFromPoint` there returned the toolbar's terminal button
|
||||
// and the tab could not be closed at all. Window resizes, sidebar drags and
|
||||
// the toolbar's own conditional buttons narrow the region the same way.
|
||||
const realignActiveTab = useCallback(() => {
|
||||
// Once the user has driven the strip with a chevron, where it sits is what
|
||||
// they asked for, and the active tab being half off the edge is an
|
||||
// ordinary consequence of scrolling a row. Only reinstate the invariant
|
||||
// when the position is still ours to choose. Width alone cannot stand in
|
||||
// for this: a chevron retires when its end is reached and rejoins when it
|
||||
// is left, so a plain user scroll narrows the strip mid-flight and looks
|
||||
// exactly like the layout event this guards against — measured, the view
|
||||
// snapped straight back and the left end became unreachable.
|
||||
if (userScrolledRef.current) return
|
||||
|
||||
const el = scrollRef.current
|
||||
if (!el) return
|
||||
const currentActiveTabId = useTabStore.getState().activeTabId
|
||||
if (!currentActiveTabId) return
|
||||
const activeTabEl = tabRefs.current.get(currentActiveTabId)
|
||||
if (!activeTabEl) return
|
||||
|
||||
const strip = el.getBoundingClientRect()
|
||||
const tab = activeTabEl.getBoundingClientRect()
|
||||
|
||||
// Already whole. The tolerance is for subpixel layout, which would
|
||||
// otherwise report a clip on every resize and scroll forever.
|
||||
if (
|
||||
tab.left >= strip.left - TAB_VISIBILITY_TOLERANCE &&
|
||||
tab.right <= strip.right + TAB_VISIBILITY_TOLERANCE
|
||||
) return
|
||||
|
||||
activeTabEl.scrollIntoView(REVEAL_ACTIVE_TAB)
|
||||
}, [])
|
||||
|
||||
useEffect(() => {
|
||||
updateScrollState()
|
||||
const el = scrollRef.current
|
||||
if (!el) return
|
||||
el.addEventListener('scroll', updateScrollState)
|
||||
const ro = new ResizeObserver(updateScrollState)
|
||||
const ro = new ResizeObserver(() => {
|
||||
updateScrollState()
|
||||
realignActiveTab()
|
||||
})
|
||||
ro.observe(el)
|
||||
return () => {
|
||||
el.removeEventListener('scroll', updateScrollState)
|
||||
ro.disconnect()
|
||||
}
|
||||
}, [updateScrollState, tabs.length])
|
||||
}, [realignActiveTab, updateScrollState, tabs.length])
|
||||
|
||||
useEffect(() => {
|
||||
if (!activeTabId) return
|
||||
const activeTabEl = tabRefs.current.get(activeTabId)
|
||||
if (!activeTabEl) return
|
||||
|
||||
activeTabEl.scrollIntoView({
|
||||
block: 'nearest',
|
||||
inline: 'nearest',
|
||||
behavior: 'smooth',
|
||||
})
|
||||
// Switching tabs hands the position back to the strip: wherever the user
|
||||
// had scrolled to, they have now named a tab they want to see.
|
||||
userScrolledRef.current = false
|
||||
// Unconditional, unlike the resize path: a tab that has just been activated
|
||||
// has to come on screen even from completely outside the strip.
|
||||
activeTabEl.scrollIntoView(REVEAL_ACTIVE_TAB)
|
||||
|
||||
const frame = window.requestAnimationFrame(updateScrollState)
|
||||
return () => window.cancelAnimationFrame(frame)
|
||||
@@ -244,6 +305,10 @@ export function TabBar() {
|
||||
const el = scrollRef.current
|
||||
if (!el) return
|
||||
const step = el.clientWidth * SCROLL_STEP_RATIO
|
||||
// The chevrons are the only way to drive the strip by hand — it is
|
||||
// `overflow-x-hidden`, so wheel and trackpad do not reach it — which makes
|
||||
// this the one place that has to hand the position over to the user.
|
||||
userScrolledRef.current = true
|
||||
el.scrollBy({ left: direction === 'left' ? -step : step, behavior: 'smooth' })
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user