From b57e29db595e19b93cd06243a2dfc4ca82c0cc7b Mon Sep 17 00:00:00 2001 From: _Kerman Date: Tue, 4 Aug 2026 13:34:29 +0800 Subject: [PATCH 1/3] fix(web): tighten sidebar session spacing --- .../src/client/SidebarRoot.module.css | 5 ++ .../src/client/WorkspaceBrowser.module.css | 47 ++++++++++--------- .../ui-workspace/tests/browser-styles.spec.ts | 21 ++++++--- 3 files changed, 43 insertions(+), 30 deletions(-) diff --git a/packages/client/ui-sidebar/src/client/SidebarRoot.module.css b/packages/client/ui-sidebar/src/client/SidebarRoot.module.css index ebb47467af..badab085de 100644 --- a/packages/client/ui-sidebar/src/client/SidebarRoot.module.css +++ b/packages/client/ui-sidebar/src/client/SidebarRoot.module.css @@ -196,9 +196,14 @@ min-height: 0; display: flex; flex-direction: column; + margin-right: -12px; overflow: hidden; } +.collapsed .regionArea { + margin-right: 0; +} + /* Foot seat: a pure layout socket pinned under the region; the ui-settings trigger row inside owns its own geometry (49px wide row / 36px rail circle) and hover chrome. */ diff --git a/packages/client/ui-workspace/src/client/WorkspaceBrowser.module.css b/packages/client/ui-workspace/src/client/WorkspaceBrowser.module.css index 970fc64e8b..dc5b489c3a 100644 --- a/packages/client/ui-workspace/src/client/WorkspaceBrowser.module.css +++ b/packages/client/ui-workspace/src/client/WorkspaceBrowser.module.css @@ -8,6 +8,12 @@ min-height: 0; display: flex; flex-direction: column; + box-sizing: border-box; + padding-right: 12px; +} + +.root.rail { + padding-right: 0; } .iconButton { @@ -167,9 +173,14 @@ min-height: 0; display: flex; flex-direction: column; + margin-right: -12px; overflow: hidden; } +.rail .listArea { + margin-right: 0; +} + /* Relative for the bottom fade overlay. */ .treeBody { flex: 1; @@ -202,27 +213,28 @@ /* 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 and the 4px rhythm rides margins instead of gap. */ + 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 { + --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-bottom: 12px; - /* Row trailing content (the relative time, and the hover action buttons - that replace it) sits flush against the row's 8px right padding, so an - overlaid scrollbar covers it. Reserving the gutter keeps the bar beside - the rows instead of on top of them; `stable` holds the reservation when - the list is short enough not to scroll, so expanding a group does not - shift every row left. */ scrollbar-gutter: stable; } -.list > [role='treeitem'] + [role='treeitem'] { - margin-top: 4px; +.list::-webkit-scrollbar { + width: var(--dsh-session-list-scrollbar-width); } -.searchTree > [role='treeitem'] + [role='treeitem'] { - margin-top: 4px; +.list > [role='treeitem'] + [role='treeitem'], +.searchTree > [role='treeitem'] + [role='treeitem'], +.groupSection > * + * { + margin-top: 2px; } .searchStatus, @@ -237,22 +249,11 @@ color: var(--dsw-alias-label-secondary); } -/* One workspace section: header row + expanded session run. Rows inside - keep the former flat-list 4px gap as sibling margins; the inter-group - breathing room (figma 133:7661 batch separator, 20px after an expanded - run) rides the NEXT section's top margin so the last group adds none. */ -.groupSection > * + * { - margin-top: 4px; -} - +/* One workspace section: header row + a compact expanded session run. */ .groupSection + .groupSection { margin-top: 4px; } -.groupSection:has([aria-expanded='true']) + .groupSection { - margin-top: 20px; -} - .empty { padding: 16px 12px; color: var(--dsw-alias-label-tertiary); diff --git a/packages/client/ui-workspace/tests/browser-styles.spec.ts b/packages/client/ui-workspace/tests/browser-styles.spec.ts index d2ac07f0c2..55c0602090 100644 --- a/packages/client/ui-workspace/tests/browser-styles.spec.ts +++ b/packages/client/ui-workspace/tests/browser-styles.spec.ts @@ -1,8 +1,7 @@ /** * WorkspaceBrowser scroll-region style contract, asserted against the CSS text - * on disk: the session list reserves its scrollbar gutter so the scrollbar - * cannot overlay row trailing content, and reserves it whether or not the list - * currently overflows so expanding a group does not shift rows sideways. + * 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. */ import { readFileSync } from 'node:fs' import { fileURLToPath } from 'node:url' @@ -32,17 +31,25 @@ function declarations(className: string): Map | undefined { } describe('WorkspaceBrowser.module.css list', () => { + const root = declarations('root') + const listArea = declarations('listArea') const list = declarations('list') + const treeBody = declarations('treeBody') it('is the scrolling region', () => { expect(list).toBeDefined() expect(list!.get('overflow-y')).toBe('auto') }) - it('reserves the scrollbar gutter unconditionally', () => { - // Row trailing content sits flush against the row's right padding, so an - // overlay scrollbar covers it. `stable` keeps the reservation when the list - // is short enough not to scroll, so expanding a group does not shift rows. + 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('reserves the scrollbar inside the stable visual inset', () => { expect(list!.get('scrollbar-gutter')).toBe('stable') }) }) From c1ac85c15ffc343460d243bc2a848c52d178f607 Mon Sep 17 00:00:00 2001 From: _Kerman Date: Tue, 4 Aug 2026 14:11:44 +0800 Subject: [PATCH 2/3] fix(web): keep sidebar scrollbar inside inset --- apps/web/tests/sidebar-scrollbar.e2e.ts | 49 ++++++++++++-- .../sidebar-scrollbar/geometry.expected.md | 2 + .../src/client/SidebarRoot.module.css | 10 +-- .../ui-sidebar/tests/sidebar-styles.spec.ts | 38 +++++++++++ .../src/client/WorkspaceBrowser.module.css | 26 +++----- .../src/client/WorkspaceBrowser.tsx | 2 +- .../ui-workspace/tests/browser-styles.spec.ts | 64 +++++++++++-------- 7 files changed, 139 insertions(+), 52 deletions(-) create mode 100644 packages/client/ui-sidebar/tests/sidebar-styles.spec.ts 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') + }) }) From 369fad9faf61922b6b5f580fca66cf91e005bbfd Mon Sep 17 00:00:00 2001 From: _Kerman Date: Tue, 4 Aug 2026 14:56:30 +0800 Subject: [PATCH 3/3] fix(web): inset sidebar scrollbar by 2px --- apps/web/tests/sidebar-scrollbar.e2e.ts | 15 ++++++++++++--- .../sidebar-scrollbar/geometry.expected.md | 2 ++ .../src/client/WorkspaceBrowser.module.css | 13 ++++++++++--- .../ui-workspace/tests/browser-styles.spec.ts | 12 +++++++++--- 4 files changed, 33 insertions(+), 9 deletions(-) diff --git a/apps/web/tests/sidebar-scrollbar.e2e.ts b/apps/web/tests/sidebar-scrollbar.e2e.ts index c37a9d67c9..b690a9d916 100644 --- a/apps/web/tests/sidebar-scrollbar.e2e.ts +++ b/apps/web/tests/sidebar-scrollbar.e2e.ts @@ -106,7 +106,9 @@ 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. */ + /** Distance from the scrollbar's right edge to the sidebar edge. */ + scrollbarEdgeOffset: number + /** Distance from the first row background's right edge to the sidebar edge. */ rowEdgeInset: number /** Client-area right edge in viewport coordinates (`clientWidth` excludes the scrollbar band). */ clientRight: number @@ -172,6 +174,8 @@ function measureList(page: Page): Promise { const pseudoWidth = getComputedStyle(list, '::-webkit-scrollbar').width const barWidth = pseudoWidth === 'auto' ? 15 : Number.parseFloat(pseudoWidth) const listRect = list.getBoundingClientRect() + const sidebarEdge = list.parentElement?.getBoundingClientRect().right + if (sidebarEdge === undefined) throw new Error('sidebar session list has no layout parent') return { gutter: style.scrollbarGutter, width: pseudoWidth, @@ -183,7 +187,8 @@ function measureList(page: Page): Promise { hoverToken: resolve('--dsh-scrollbar-thumb-hover'), overflows: list.scrollHeight > list.clientHeight, band: listRect.width - list.clientWidth, - rowEdgeInset: listRect.right - row.getBoundingClientRect().right, + scrollbarEdgeOffset: sidebarEdge - listRect.right, + rowEdgeInset: sidebarEdge - row.getBoundingClientRect().right, clientRight: listRect.left + list.clientWidth, borderRight: listRect.right, timeRight: time.getBoundingClientRect().right, @@ -211,9 +216,11 @@ function measureRowInset(page: Page): Promise('[role="treeitem"]') if (row === null) throw new Error('no row in the sidebar list') + const sidebarEdge = list.parentElement?.getBoundingClientRect().right + if (sidebarEdge === undefined) throw new Error('sidebar session list has no layout parent') return { overflows: list.scrollHeight > list.clientHeight, - rowEdgeInset: list.getBoundingClientRect().right - row.getBoundingClientRect().right, + rowEdgeInset: sidebarEdge - row.getBoundingClientRect().right, } }) } @@ -247,6 +254,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`, + `- scrollbar inset from the sidebar edge: ${String(metrics.scrollbarEdgeOffset)}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)}`, @@ -325,6 +333,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.scrollbarEdgeOffset).toBe(2) 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 diff --git a/apps/web/tests/snapshots/sidebar-scrollbar/geometry.expected.md b/apps/web/tests/snapshots/sidebar-scrollbar/geometry.expected.md index 48961d7fb5..b0fb597418 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 +- scrollbar inset from the sidebar edge: 2px - row background inset from the sidebar edge: 12px - relative time covered by the bar: 0px - relative time ends inside the content area: true @@ -29,6 +30,7 @@ - --dsh-scrollbar-thumb-hover: rgb(84, 85, 87) - list overflows: true - reserved band: 8px +- scrollbar inset from the sidebar edge: 2px - row background inset from the sidebar edge: 12px - relative time covered by the bar: 0px - relative time ends inside the content area: true diff --git a/packages/client/ui-workspace/src/client/WorkspaceBrowser.module.css b/packages/client/ui-workspace/src/client/WorkspaceBrowser.module.css index e15f966044..0b8841a3a4 100644 --- a/packages/client/ui-workspace/src/client/WorkspaceBrowser.module.css +++ b/packages/client/ui-workspace/src/client/WorkspaceBrowser.module.css @@ -6,6 +6,7 @@ .root { --dsh-session-list-edge-inset: var(--dsh-sidebar-inline-padding); --dsh-session-list-scrollbar-width: 8px; + --dsh-session-list-scrollbar-offset: 2px; flex: 1; min-height: 0; display: flex; @@ -214,13 +215,19 @@ } /* 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. */ + under content overflow. The 2px edge offset, stable 8px themed scrollbar, + and remaining padding equal the shell's right inset, with or without + overflow, so moving the bar does not move the rows. */ .list { flex: 1; min-height: 0; overflow-y: auto; - padding-right: calc(var(--dsh-session-list-edge-inset) - var(--dsh-session-list-scrollbar-width)); + margin-right: var(--dsh-session-list-scrollbar-offset); + padding-right: calc( + var(--dsh-session-list-edge-inset) + - var(--dsh-session-list-scrollbar-width) + - var(--dsh-session-list-scrollbar-offset) + ); padding-bottom: 12px; scrollbar-gutter: stable; } diff --git a/packages/client/ui-workspace/tests/browser-styles.spec.ts b/packages/client/ui-workspace/tests/browser-styles.spec.ts index e9a52987ae..4165971bff 100644 --- a/packages/client/ui-workspace/tests/browser-styles.spec.ts +++ b/packages/client/ui-workspace/tests/browser-styles.spec.ts @@ -43,12 +43,18 @@ describe('WorkspaceBrowser.module.css list', () => { 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('--dsh-session-list-scrollbar-offset')).toBe('2px') 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(list?.get('margin-right')).toBe('var(--dsh-session-list-scrollbar-offset)') + expect(list?.get('padding-right')).toBe([ + 'calc(', + 'var(--dsh-session-list-edge-inset)', + '- var(--dsh-session-list-scrollbar-width)', + '- var(--dsh-session-list-scrollbar-offset)', + ')', + ].join(' ')) expect(declarations('.list::-webkit-scrollbar')).toBeUndefined() })