diff --git a/apps/web/tests/sidebar-scrollbar.e2e.ts b/apps/web/tests/sidebar-scrollbar.e2e.ts index 12b9588a97..c37a9d67c9 100644 --- a/apps/web/tests/sidebar-scrollbar.e2e.ts +++ b/apps/web/tests/sidebar-scrollbar.e2e.ts @@ -106,6 +106,8 @@ interface ListMetrics { overflows: boolean /** Border-box width minus client width: the space the scrollbar takes out of the content area. */ band: number + /** Distance from the first row background's right edge to the list border box. */ + rowEdgeInset: number /** Client-area right edge in viewport coordinates (`clientWidth` excludes the scrollbar band). */ clientRight: number /** Border-box right edge in viewport coordinates. */ @@ -133,6 +135,8 @@ function measureList(page: Page): Promise { if (list === null) throw new Error('sidebar session list not in the DOM') const time = list.querySelector('[class*="time"]') if (time === null) throw new Error('no row relative-time element in the sidebar list') + const row = list.querySelector('[role="treeitem"]') + if (row === null) throw new Error('no row in the sidebar list') // Each indirection variable is resolved through its own throwaway probe // appended to the list: `var()` substitution then happens where the list // sits in the cascade, which is the claim, and `color` normalizes whatever @@ -167,6 +171,7 @@ function measureList(page: Page): Promise { const style = getComputedStyle(list) const pseudoWidth = getComputedStyle(list, '::-webkit-scrollbar').width const barWidth = pseudoWidth === 'auto' ? 15 : Number.parseFloat(pseudoWidth) + const listRect = list.getBoundingClientRect() return { gutter: style.scrollbarGutter, width: pseudoWidth, @@ -177,9 +182,10 @@ function measureList(page: Page): Promise { token: resolve('--dsh-scrollbar-thumb'), hoverToken: resolve('--dsh-scrollbar-thumb-hover'), overflows: list.scrollHeight > list.clientHeight, - band: list.getBoundingClientRect().width - list.clientWidth, - clientRight: list.getBoundingClientRect().left + list.clientWidth, - borderRight: list.getBoundingClientRect().right, + band: listRect.width - list.clientWidth, + rowEdgeInset: listRect.right - row.getBoundingClientRect().right, + clientRight: listRect.left + list.clientWidth, + borderRight: listRect.right, timeRight: time.getBoundingClientRect().right, // The bar is drawn in the rightmost `barWidth` of the border box, whether // or not that space was reserved. Its width comes from the sheet where the @@ -188,7 +194,26 @@ function measureList(page: Page): Promise { // absent. Taking the UA width as the fallback is what keeps the assertion // honest: assuming 0 there would report no occlusion precisely in the // state that has it. - timeCoveredBy: Math.max(0, time.getBoundingClientRect().right - (list.getBoundingClientRect().right - barWidth)), + timeCoveredBy: Math.max(0, time.getBoundingClientRect().right - (listRect.right - barWidth)), + } + }) +} + +/** + * Measure only overflow and row inset, which remain observable when every + * session is hidden under a collapsed workspace group. + * @param page - the page under test. + * @returns the list overflow state and first row's trailing inset. + */ +function measureRowInset(page: Page): Promise> { + return page.evaluate(() => { + const list = document.querySelector('[role="tree"][aria-label="Sessions"]') + if (list === null) throw new Error('sidebar session list not in the DOM') + const row = list.querySelector('[role="treeitem"]') + if (row === null) throw new Error('no row in the sidebar list') + return { + overflows: list.scrollHeight > list.clientHeight, + rowEdgeInset: list.getBoundingClientRect().right - row.getBoundingClientRect().right, } }) } @@ -222,6 +247,7 @@ function renderGeometry(light: ListMetrics, dark: ListMetrics): string { `- --dsh-scrollbar-thumb-hover: ${metrics.hoverToken}`, `- list overflows: ${String(metrics.overflows)}`, `- reserved band: ${String(metrics.band)}px`, + `- row background inset from the sidebar edge: ${String(metrics.rowEdgeInset)}px`, `- relative time covered by the bar: ${String(metrics.timeCoveredBy)}px`, `- relative time ends inside the content area: ${String(metrics.timeRight <= metrics.clientRight)}`, `- content area ends before the border box: ${String(metrics.clientRight < metrics.borderRight)}`, @@ -299,6 +325,7 @@ describe('web e2e: sidebar session list scrollbar (reserved gutter / themed thum // drawn over it. Removing the declaration makes it exactly 0. The value // itself is not pinned — it tracks `scrollbar-width` and the platform. expect(metrics.band).toBeGreaterThan(0) + expect(metrics.rowEdgeInset).toBe(12) // The reported symptom, stated directly: no part of the row's relative time // lies under the bar. Measures 7 on clean master — the `h` of `1h` is the // covered part. Unlike the client-edge comparison below it does not go @@ -317,6 +344,20 @@ describe('web e2e: sidebar session list scrollbar (reserved gutter / themed thum expect(tripwire.pageErrors).toEqual([]) }, 60_000) + it('keeps the row background inset when overflow disappears', async () => { + onTestFailed(() => saveFailureShot(page, 'web-e2e-sidebar-scrollbar-stable-inset')) + expect(await measureRowInset(page)).toEqual({ overflows: true, rowEdgeInset: 12 }) + const bucket = page.getByText('Ungrouped', { exact: true }).locator('..').locator('..') + await bucket.click() + try { + await expect.poll(async () => (await measureRowInset(page)).overflows, { timeout: 10_000 }).toBe(false) + expect(await measureRowInset(page)).toEqual({ overflows: false, rowEdgeInset: 12 }) + } finally { + await expandSeededSessions(page) + } + expect(tripwire.pageErrors).toEqual([]) + }, 60_000) + it('renders the themed thumb through the WebKit path in both palettes', async () => { onTestFailed(() => saveFailureShot(page, 'web-e2e-sidebar-scrollbar-theme')) const light = await measureList(page) diff --git a/apps/web/tests/snapshots/sidebar-scrollbar/geometry.expected.md b/apps/web/tests/snapshots/sidebar-scrollbar/geometry.expected.md index 4349532ef8..48961d7fb5 100644 --- a/apps/web/tests/snapshots/sidebar-scrollbar/geometry.expected.md +++ b/apps/web/tests/snapshots/sidebar-scrollbar/geometry.expected.md @@ -12,6 +12,7 @@ - --dsh-scrollbar-thumb-hover: rgb(212, 212, 212) - list overflows: true - reserved band: 8px +- row background inset from the sidebar edge: 12px - relative time covered by the bar: 0px - relative time ends inside the content area: true - content area ends before the border box: true @@ -28,6 +29,7 @@ - --dsh-scrollbar-thumb-hover: rgb(84, 85, 87) - list overflows: true - reserved band: 8px +- row background inset from the sidebar edge: 12px - relative time covered by the bar: 0px - relative time ends inside the content area: true - content area ends before the border box: true diff --git a/packages/client/ui-sidebar/src/client/SidebarRoot.module.css b/packages/client/ui-sidebar/src/client/SidebarRoot.module.css index badab085de..fb593307ea 100644 --- a/packages/client/ui-sidebar/src/client/SidebarRoot.module.css +++ b/packages/client/ui-sidebar/src/client/SidebarRoot.module.css @@ -7,10 +7,11 @@ mid-slide. */ .root { + --dsh-sidebar-inline-padding: 12px; display: flex; flex-direction: column; height: 100%; - padding: 6px 12px; + padding: 6px var(--dsh-sidebar-inline-padding); box-sizing: border-box; background: var(--dsw-specific-sidebar-fill); color: var(--dsw-alias-label-primary); @@ -189,14 +190,15 @@ max-width: 0; } -/* Region seat: always mounted so the foot never moves; the browser inside - handles its own wide/rail content. */ +/* Region seat: always mounted so the foot never moves. Its trailing margin + cancels the wide shell inset so the nested scrollbar can sit at the sidebar + edge; the browser restores that inset inside its own rows. */ .regionArea { flex: 1; min-height: 0; display: flex; flex-direction: column; - margin-right: -12px; + margin-right: calc(-1 * var(--dsh-sidebar-inline-padding)); overflow: hidden; } diff --git a/packages/client/ui-sidebar/tests/sidebar-styles.spec.ts b/packages/client/ui-sidebar/tests/sidebar-styles.spec.ts new file mode 100644 index 0000000000..63721258c9 --- /dev/null +++ b/packages/client/ui-sidebar/tests/sidebar-styles.spec.ts @@ -0,0 +1,38 @@ +/** Sidebar shell inset contract shared with the nested workspace browser. */ +import { readFileSync } from 'node:fs' +import { fileURLToPath } from 'node:url' +import { describe, expect, it } from 'vitest' + +const css = readFileSync(fileURLToPath(new URL('../src/client/SidebarRoot.module.css', import.meta.url)), 'utf8') + +/** + * Declarations of one exact selector, keyed by property. + * @param selector - exact selector text. + * @returns the normalized declarations, or undefined when absent. + */ +function declarations(selector: string): Map | undefined { + const withoutComments = css.replace(/\/\*[\s\S]*?\*\//g, ' ') + for (const [, selectorList = '', body = ''] of withoutComments.matchAll(/([^{}]+)\{([^{}]*)\}/g)) { + if (!selectorList.split(',').map(value => value.trim()).includes(selector)) continue + const found = new Map() + for (const part of body.split(';')) { + const colon = part.indexOf(':') + if (colon === -1) continue + found.set(part.slice(0, colon).trim(), part.slice(colon + 1).trim().replace(/\s+/g, ' ')) + } + return found + } + return undefined +} + +describe('SidebarRoot.module.css inset', () => { + it('shares and cancels the wide shell trailing padding structurally', () => { + const root = declarations('.root') + expect(root?.get('--dsh-sidebar-inline-padding')).toBe('12px') + expect(root?.get('padding')).toBe('6px var(--dsh-sidebar-inline-padding)') + expect(declarations('.regionArea')?.get('margin-right')).toBe( + 'calc(-1 * var(--dsh-sidebar-inline-padding))', + ) + expect(declarations('.collapsed .regionArea')?.get('margin-right')).toBe('0') + }) +}) diff --git a/packages/client/ui-workspace/src/client/WorkspaceBrowser.module.css b/packages/client/ui-workspace/src/client/WorkspaceBrowser.module.css index dc5b489c3a..e15f966044 100644 --- a/packages/client/ui-workspace/src/client/WorkspaceBrowser.module.css +++ b/packages/client/ui-workspace/src/client/WorkspaceBrowser.module.css @@ -4,12 +4,14 @@ rail state renders only the two 36x36 icon controls. */ .root { + --dsh-session-list-edge-inset: var(--dsh-sidebar-inline-padding); + --dsh-session-list-scrollbar-width: 8px; flex: 1; min-height: 0; display: flex; flex-direction: column; box-sizing: border-box; - padding-right: 12px; + padding-right: var(--dsh-session-list-edge-inset); } .root.rail { @@ -173,7 +175,7 @@ min-height: 0; display: flex; flex-direction: column; - margin-right: -12px; + margin-right: calc(-1 * var(--dsh-session-list-edge-inset)); overflow: hidden; } @@ -195,7 +197,7 @@ .fade { position: absolute; left: 0; - right: 0; + right: var(--dsh-session-list-edge-inset); bottom: 0; height: 72px; background: linear-gradient(to bottom, transparent, var(--dsw-specific-sidebar-fill)); @@ -211,27 +213,19 @@ from { opacity: 0; } } -/* List: the only scrolling region. Block, not a flex column: as flex items - the 54/34 rows would shrink under content overflow; block children keep - their design heights do not shrink under content overflow. Its right - padding is the sole gap between row backgrounds and the sidebar edge; the - outer seats move their clip boundary out so they add no hidden inset. */ +/* List: the only scrolling region. Block children keep their design heights + under content overflow. The stable 8px themed scrollbar and the remaining + padding together equal the shell's right inset, with or without overflow. */ .list { - --dsh-session-list-edge-inset: 12px; - --dsh-session-list-scrollbar-width: 8px; flex: 1; min-height: 0; overflow-y: auto; - padding-right: var(--dsh-session-list-edge-inset); + padding-right: calc(var(--dsh-session-list-edge-inset) - var(--dsh-session-list-scrollbar-width)); padding-bottom: 12px; scrollbar-gutter: stable; } -.list::-webkit-scrollbar { - width: var(--dsh-session-list-scrollbar-width); -} - -.list > [role='treeitem'] + [role='treeitem'], +.flatList > * + *, .searchTree > [role='treeitem'] + [role='treeitem'], .groupSection > * + * { margin-top: 2px; diff --git a/packages/client/ui-workspace/src/client/WorkspaceBrowser.tsx b/packages/client/ui-workspace/src/client/WorkspaceBrowser.tsx index 93d432bd89..3607dc8640 100644 --- a/packages/client/ui-workspace/src/client/WorkspaceBrowser.tsx +++ b/packages/client/ui-workspace/src/client/WorkspaceBrowser.tsx @@ -238,7 +238,7 @@ function FlatList({ useSessions, open, forkSession, onSessionRename, onSessionAr const now = Date.now() return (
-
+
{rows.length === 0 && (
{t('empty.none')}
)} diff --git a/packages/client/ui-workspace/tests/browser-styles.spec.ts b/packages/client/ui-workspace/tests/browser-styles.spec.ts index 55c0602090..e9a52987ae 100644 --- a/packages/client/ui-workspace/tests/browser-styles.spec.ts +++ b/packages/client/ui-workspace/tests/browser-styles.spec.ts @@ -1,7 +1,7 @@ /** - * WorkspaceBrowser scroll-region style contract, asserted against the CSS text - * on disk: the session list keeps one stable right inset for row hover fills, - * with or without overflow, while outer clip seats add no hidden second inset. + * WorkspaceBrowser spacing contract, asserted against the CSS text on disk: + * row fills share the shell's trailing inset, the stable scrollbar counts + * inside it, and flat, grouped, and search views keep their intended rhythm. */ import { readFileSync } from 'node:fs' import { fileURLToPath } from 'node:url' @@ -10,46 +10,56 @@ import { describe, expect, it } from 'vitest' const css = readFileSync(fileURLToPath(new URL('../src/client/WorkspaceBrowser.module.css', import.meta.url)), 'utf8') /** - * Declarations of one class rule, keyed by property with whitespace collapsed. + * Declarations of one selector rule, keyed by property with whitespace collapsed. * Declaration order and trailing semicolons are normalized away. - * @param className - local class name, without the leading dot. + * @param selector - one exact selector, including a leading dot for local classes. * @returns the rule's declarations, or undefined when no such rule exists. */ -function declarations(className: string): Map | undefined { +function declarations(selector: string): Map | undefined { const withoutComments = css.replace(/\/\*[\s\S]*?\*\//g, ' ') - const match = new RegExp(String.raw`(^|[\s,}])\.${className}\s*\{([^{}]*)\}`).exec(withoutComments) - if (match === null) return undefined - const found = new Map() - // The body group is unconditional in the pattern; the fallback only satisfies - // noUncheckedIndexedAccess. - for (const part of (match[2] ?? '').split(';')) { - const colon = part.indexOf(':') - if (colon === -1) continue - found.set(part.slice(0, colon).trim(), part.slice(colon + 1).trim().replace(/\s+/g, ' ')) + for (const [, selectorList = '', body = ''] of withoutComments.matchAll(/([^{}]+)\{([^{}]*)\}/g)) { + if (!selectorList.split(',').map(value => value.trim()).includes(selector)) continue + const found = new Map() + for (const part of body.split(';')) { + const colon = part.indexOf(':') + if (colon === -1) continue + found.set(part.slice(0, colon).trim(), part.slice(colon + 1).trim().replace(/\s+/g, ' ')) + } + return found } - return found + return undefined } describe('WorkspaceBrowser.module.css list', () => { - const root = declarations('root') - const listArea = declarations('listArea') - const list = declarations('list') - const treeBody = declarations('treeBody') + const root = declarations('.root') + const listArea = declarations('.listArea') + const list = declarations('.list') it('is the scrolling region', () => { expect(list).toBeDefined() expect(list!.get('overflow-y')).toBe('auto') }) - it('keeps row backgrounds edge-flush with the scrolling region', () => { - expect(root?.get('padding-right')).toBe('12px') - expect(listArea?.get('margin-right')).toBe('-12px') - expect(treeBody?.get('margin-right')).toBeUndefined() - expect(list?.get('margin-right')).toBeUndefined() - expect(list?.get('padding-right')).toBe('var(--dsh-session-list-edge-inset)') + it('counts the themed scrollbar inside the shell trailing inset', () => { + expect(root?.get('--dsh-session-list-edge-inset')).toBe('var(--dsh-sidebar-inline-padding)') + expect(root?.get('--dsh-session-list-scrollbar-width')).toBe('8px') + expect(root?.get('padding-right')).toBe('var(--dsh-session-list-edge-inset)') + expect(listArea?.get('margin-right')).toBe('calc(-1 * var(--dsh-session-list-edge-inset))') + expect(declarations('.fade')?.get('right')).toBe('var(--dsh-session-list-edge-inset)') + expect(list?.get('padding-right')).toBe( + 'calc(var(--dsh-session-list-edge-inset) - var(--dsh-session-list-scrollbar-width))', + ) + expect(declarations('.list::-webkit-scrollbar')).toBeUndefined() }) - it('reserves the scrollbar inside the stable visual inset', () => { + it('reserves the scrollbar whether or not the list overflows', () => { expect(list!.get('scrollbar-gutter')).toBe('stable') }) + + it('keeps 2px between rows and 4px between workspace groups', () => { + expect(declarations('.flatList > * + *')?.get('margin-top')).toBe('2px') + expect(declarations(".searchTree > [role='treeitem'] + [role='treeitem']")?.get('margin-top')).toBe('2px') + expect(declarations('.groupSection > * + *')?.get('margin-top')).toBe('2px') + expect(declarations('.groupSection + .groupSection')?.get('margin-top')).toBe('4px') + }) })