From 148f8dcea5c1d70c78d49b907028f35d6df7fc78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=A8=8B=E5=BA=8F=E5=91=98=E9=98=BF=E6=B1=9F=28Relakkes?= =?UTF-8?q?=29?= Date: Tue, 28 Jul 2026 22:28:03 +0800 Subject: [PATCH] fix(desktop): restore depth in the session tab strip #1123 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The strip, the active tab and the content below it were all `--color-surface`: on the白 theme that is three planes of #FFFFFF with a 3px terracotta rule as the only separator, which is what #1123 reported as "粗犷". The flattening came from c712f5285, which lifted the strip's ground from `--cc-s2` to `--cc-bg` and dropped the active tab's fill. The strip is frame, not paper. It now sits on the sidebar's ground (`--color-surface-sidebar`, the same `--cc-s0` the sidebar already uses) and the active tab is filled with `--color-surface`, so it reads as a sheet lifted off the desk and continuous with the view it opens onto. No new tokens, and the top band no longer changes colour halfway across. The issue also proposed pill/segmented shapes. Those are segmented controls — fixed item counts, short labels, no close/drag/overflow — so they are not adopted here; a document tab is what this strip is. Its icon request is, and it turned up a real bug alongside it. Also in this change: - Hover no longer uses `--color-surface-hover`. That token is tuned for hovering *on* paper, so on both ink themes it lands brighter than paper itself (dark #2B271F vs #201D17, measured luminance 0.0206 vs 0.0125) and a hovered tab outshone the selected one. Hover now shares paper with the active tab and selection is carried by the rule plus the label weight, which is strictly stronger in all six themes. - Tabs size to their titles (`min-w-[140px] max-w-[200px]`) instead of a dead 180px, matching the workspace preview tabs. Chevron scrolling moves by a fraction of the visible strip; pointer reordering already measured with `getBoundingClientRect` and is unaffected. - One fixed icon slot per tab, filling in the four kinds that had no glyph (session, market, traces, subagent). The running dot used to be *inserted* ahead of the label, so a title jumped sideways the moment its session started and jumped back when it finished; the dot now swaps into the slot and every title starts at the same x. The `rather than a filled pill` guard is rewritten rather than deleted: the ban is on the pill *shape* (radius, drop shadow, gaps), not on the fill, and the comment says so — asserting `bg-transparent` is what would flatten the strip again. Four tests added for the layering contract, the fluid width, the icon slot's no-shift property, and the scroll step. Verified: check:desktop green (275 files / 3369 passed / 1 skipped), and a six-theme walkthrough over the built dist confirming visible separation in every theme and the ink-theme hover inversion gone. --- desktop/src/components/layout/TabBar.test.tsx | 182 +++++++++++++++++- desktop/src/components/layout/TabBar.tsx | 98 ++++++---- 2 files changed, 242 insertions(+), 38 deletions(-) diff --git a/desktop/src/components/layout/TabBar.test.tsx b/desktop/src/components/layout/TabBar.test.tsx index 5631244b..d8065f18 100644 --- a/desktop/src/components/layout/TabBar.test.tsx +++ b/desktop/src/components/layout/TabBar.test.tsx @@ -710,6 +710,98 @@ describe('TabBar', () => { expect(screen.queryByTestId('session-activity-badge')).not.toBeInTheDocument() }) + it('gives session tabs an icon and keeps the title still when one starts running', async () => { + const { TabBar } = await import('./TabBar') + const { useTabStore } = await import('../../stores/tabStore') + const { useChatStore } = await import('../../stores/chatStore') + + useTabStore.setState({ + tabs: [ + { sessionId: 'tab-1', title: 'Idle Session', type: 'session', status: 'idle' }, + ], + activeTabId: 'tab-1', + }) + useChatStore.setState({ + sessions: {}, + disconnectSession: vi.fn(), + } as Partial>) + + await act(async () => { + render() + }) + + // #1123 asked for icons on the tabs. Five special kinds already had one; + // the session kind — the overwhelming majority — did not. + const idleLabel = screen.getByText('Idle Session') + expect(idleLabel.previousElementSibling?.textContent).toBe('chat_bubble') + const siblingsBeforeLabel = Array.from(idleLabel.parentElement?.children ?? []) + .indexOf(idleLabel) + + await act(async () => { + useTabStore.setState({ + tabs: [ + { sessionId: 'tab-1', title: 'Idle Session', type: 'session', status: 'running' }, + ], + activeTabId: 'tab-1', + }) + }) + + // The real bug behind the icon request: the running dot used to be + // *inserted* ahead of the label, so a title jumped sideways the moment its + // session started and jumped back when it finished. The dot now swaps into + // the icon slot, so the label keeps its position in the row. + const runningLabel = screen.getByText('Idle Session') + expect(screen.getByRole('status')).toBeInTheDocument() + expect(Array.from(runningLabel.parentElement?.children ?? []).indexOf(runningLabel)) + .toBe(siblingsBeforeLabel) + }) + + it('scrolls by a fraction of the visible strip rather than a fixed tab width', async () => { + 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' }, + ], + activeTabId: 'tab-1', + }) + useChatStore.setState({ + sessions: {}, + disconnectSession: vi.fn(), + } as Partial>) + + await act(async () => { + render() + }) + + const scrollRegion = screen.getByTestId('tab-bar-scroll-region') + const scrollByMock = vi.fn() + Object.defineProperty(scrollRegion, 'clientWidth', { configurable: true, get: () => 400 }) + Object.defineProperty(scrollRegion, 'scrollWidth', { configurable: true, get: () => 1200 }) + Object.defineProperty(scrollRegion, 'scrollLeft', { configurable: true, get: () => 0 }) + Object.defineProperty(scrollRegion, 'scrollBy', { configurable: true, value: scrollByMock }) + + act(() => { + fireEvent.scroll(scrollRegion) + }) + + const rightButton = await waitFor(() => { + const button = screen.getByText('chevron_right').closest('button') + expect(button).toBeInTheDocument() + return button as HTMLButtonElement + }) + + fireEvent.click(rightButton) + + // Tabs size to their titles now, so the old fixed 180px step would + // overshoot a row of short ones and undershoot a row of long ones. + expect(scrollByMock).toHaveBeenCalledWith({ left: 300, behavior: 'smooth' }) + }) + it('scrolls the active tab into view when the active tab changes', async () => { const { TabBar } = await import('./TabBar') const { useTabStore } = await import('../../stores/tabStore') @@ -880,7 +972,7 @@ describe('TabBar', () => { expect(screen.getByTestId('tab-bar-drag-gutter')).toHaveClass('min-h-[52px]') }) - it('marks the active tab with a rule rather than a filled pill', async () => { + it('lifts the active tab onto the paper ground without turning it into a pill', async () => { const { TabBar } = await import('./TabBar') const { useTabStore } = await import('../../stores/tabStore') const { useChatStore } = await import('../../stores/chatStore') @@ -905,10 +997,92 @@ describe('TabBar', () => { const inactive = screen.getByText('Inactive One').closest('.tab-bar-interactive') expect(active?.className).toContain('shadow-[inset_0_-3px_0_var(--color-brand)]') - // The active tab shares the strip's ground; giving it its own fill is what - // turned the underline treatment back into a pill. - expect(active?.className).toContain('bg-transparent') + + // The fill is the point, and it is not a pill. #1123 landed because the + // strip, the active tab and the content below it were all + // `--color-surface`: three planes, one colour, nothing but a 3px rule to + // separate them. The strip now sits on the sidebar's ground and the active + // tab is filled with paper, so it reads as a sheet lifted off the desk and + // continuous with the view it opens onto. + expect(active?.className).toContain('bg-[var(--color-surface)]') + expect(active?.className).not.toContain('bg-transparent') + expect(inactive?.className).toContain('bg-transparent') expect(inactive?.className).not.toContain('inset_0_-3px') + + // What stays banned is the *shape*, not the fill. A pill is a rounded + // block floating clear of both the strip and the content; a document tab + // is square and flush. Guard the three properties that would turn one into + // the other. + expect(active?.className).not.toMatch(/\brounded/) + expect(active?.className).not.toMatch(/\bshadow-\[0/) + expect(active?.className).not.toMatch(/\bm[xlr]?-/) + + // Hover shares paper with the active tab instead of using + // `--color-surface-hover`. That token is tuned for hovering *on* paper, so + // on the ink themes it sits brighter than paper (dark #2B271F vs #201D17) + // and a hovered tab would outshine the selected one. Selection stays + // strictly stronger because only it adds the rule and the label weight. + expect(inactive?.className).toContain('hover:bg-[var(--color-surface)]') + expect(inactive?.className).not.toContain('hover:bg-[var(--color-surface-hover)]') + }) + + it('keeps the strip on the frame ground so the active tab can lift off it', async () => { + const { TabBar } = await import('./TabBar') + const { useTabStore } = await import('../../stores/tabStore') + const { useChatStore } = await import('../../stores/chatStore') + + useTabStore.setState({ + tabs: [ + { sessionId: 'tab-1', title: 'Active One', type: 'session', status: 'idle' }, + ], + activeTabId: 'tab-1', + }) + useChatStore.setState({ + sessions: {}, + disconnectSession: vi.fn(), + } as Partial>) + + await act(async () => { + render() + }) + + // The two halves of the contract. If the strip ever goes back to + // `--color-surface` the active tab's fill stops reading as a lift and the + // whole bar flattens again, which is the regression #1123 reported. + expect(screen.getByTestId('tab-bar')).toHaveClass('bg-[var(--color-surface-sidebar)]') + expect(screen.getByText('Active One').closest('.tab-bar-interactive')?.className) + .toContain('bg-[var(--color-surface)]') + }) + + it('sizes tabs to their titles instead of a fixed width', async () => { + const { TabBar } = await import('./TabBar') + const { useTabStore } = await import('../../stores/tabStore') + const { useChatStore } = await import('../../stores/chatStore') + + useTabStore.setState({ + tabs: [ + { sessionId: 'tab-1', title: 'Short', type: 'session', status: 'idle' }, + ], + activeTabId: 'tab-1', + }) + useChatStore.setState({ + sessions: {}, + disconnectSession: vi.fn(), + } as Partial>) + + await act(async () => { + render() + }) + + const tab = screen.getByText('Short').closest('.tab-bar-interactive') as HTMLElement + + expect(tab.className).toContain('min-w-[140px]') + expect(tab.className).toContain('max-w-[200px]') + // The inline `width`/`maxWidth` pair is what pinned every tab to 180px and + // left short titles trailing dead space. Only `transform` belongs inline + // now — it carries the drag offset. + expect(tab.style.width).toBe('') + expect(tab.style.maxWidth).toBe('') }) it('passes the active session workdir into the open-project control', async () => { diff --git a/desktop/src/components/layout/TabBar.tsx b/desktop/src/components/layout/TabBar.tsx index 3f63fb66..a38cffe2 100644 --- a/desktop/src/components/layout/TabBar.tsx +++ b/desktop/src/components/layout/TabBar.tsx @@ -11,6 +11,7 @@ import { WORKBENCH_TAB_PREFIX, useTabStore, type Tab, + type TabType, } from '../../stores/tabStore' import { useChatStore } from '../../stores/chatStore' import { useSessionStore } from '../../stores/sessionStore' @@ -34,8 +35,26 @@ import { SessionActivityButton } from '../activity/SessionActivityButton' import { useActivityPanelStore } from '../../stores/activityPanelStore' import { getSessionBrowsablePath } from '../../lib/sessionWorkspace' -const TAB_WIDTH = 180 const DRAG_START_THRESHOLD = 4 +// Fraction of the visible strip a chevron press travels. Tabs size to their +// titles, so a fixed pixel step would overshoot a row of short ones and +// 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 +// One glyph per tab kind, so the icon slot is never empty and every title +// starts at the same x. `trace` and `traces` share the Settings rail's glyph +// on purpose — a trace tab should read as that section, not as another chat. +const TAB_TYPE_ICON: Partial> = { + session: 'chat_bubble', + settings: 'settings', + scheduled: 'schedule', + market: 'storefront', + terminal: 'terminal', + trace: 'account_tree', + traces: 'account_tree', + workbench: 'view_sidebar', + subagent: 'smart_toy', +} const desktopHost = getDesktopHost() const isDesktopRuntime = desktopHost.isDesktop const EMPTY_DISMISSED_BACKGROUND_TASK_KEYS: readonly string[] = [] @@ -222,7 +241,8 @@ export function TabBar() { const scroll = (direction: 'left' | 'right') => { const el = scrollRef.current if (!el) return - el.scrollBy({ left: direction === 'left' ? -TAB_WIDTH : TAB_WIDTH, behavior: 'smooth' }) + const step = el.clientWidth * SCROLL_STEP_RATIO + el.scrollBy({ left: direction === 'left' ? -step : step, behavior: 'smooth' }) } const closeTabWithCleanup = useCallback((tab: Tab) => { @@ -405,7 +425,14 @@ export function TabBar() {
{canScrollLeft && ( @@ -611,49 +638,52 @@ const TabItem = forwardRef - {tab.type === 'session' && isRunning && ( - - )} - {tab.type === 'session' && tab.status === 'error' && ( - - )} - {tab.type === 'settings' && ( - settings - )} - {tab.type === 'scheduled' && ( - schedule - )} - {tab.type === 'terminal' && ( - terminal - )} - {tab.type === 'workbench' && ( - view_sidebar - )} - {/* Same glyph as the Settings rail entry the trace list sits behind, so a - trace tab reads as that section rather than as another chat. */} - {tab.type === 'trace' && ( - account_tree - )} + {/* + One fixed slot, not a run of conditional siblings. The status dot used + to be *inserted* ahead of the label, so a session shoved its own title + sideways the moment it started running and pulled it back when it + finished. Swapping the slot's contents keeps the title still. + */} + + {tab.type === 'session' && isRunning ? ( + + ) : tab.type === 'session' && tab.status === 'error' ? ( + + ) : ( + + {TAB_TYPE_ICON[tab.type] ?? TAB_TYPE_ICON.session} + + )} + - + {displayTitle}