From a1f01b3394bc49fc1ba0ad3df101769d2cf412fb Mon Sep 17 00:00:00 2001 From: tianyao Date: Wed, 2 Sep 2026 02:19:24 +0000 Subject: [PATCH] fix(ui): unify mobile Source Control row density across Queue, List, and Tree MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Queue rows already set the compact mobile row baseline; List and Tree file rows, plus tree folder rows, now read the same shared --scv-mobile-row-* custom properties instead of drifting independently. List mode's folder-path suffix now yields horizontal space before the filename (disproportionate flex-shrink) so long paths ellipsis first and the row never wraps to a second line. Also normalizes the Queue→Repository vertical gap and both sections' header→first-row padding to the same values. CSS-only; no changes to SourceControlViewModel, selection semantics, sync behavior, tree shaping, scroll persistence, or Queue/Repository responsibilities. --- styles.css | 44 +++++++-- .../source-control/SourceControlView.test.ts | 90 +++++++++++++++++++ 2 files changed, 128 insertions(+), 6 deletions(-) diff --git a/styles.css b/styles.css index 756374f..bd5b5b7 100644 --- a/styles.css +++ b/styles.css @@ -41,6 +41,12 @@ overflow: hidden; padding: 0; container-type: inline-size; + /* Shared mobile row-density baseline: the Sync Queue's existing compact + row (unchanged) is the visual reference. Repository List and Tree file + rows, plus tree folder rows, read these same values instead of each + maintaining their own effective height. */ + --scv-mobile-row-min-height: 30px; + --scv-mobile-row-padding: 6px 10px; } .scv-header { @@ -312,7 +318,10 @@ body.is-mobile .scv-view-toggle-label { display: none; } /* ── Selected-for-sync workspace ───────────────────────────── */ .scv-selected-section { flex-shrink: 0; - margin: 6px 8px 4px 8px; + /* No bottom margin: Repository Changes' own header already contributes + the same 6px top padding used above the Queue, so Queue→Repository + stays a single 6px gap instead of stacking on top of it. */ + margin: 6px 8px 0 8px; padding: 6px 0; border-radius: var(--radius-m); background: var(--background-secondary); @@ -324,7 +333,9 @@ body.is-mobile .scv-view-toggle-label { display: none; } display: flex; align-items: center; gap: 6px; - padding: 2px 10px 6px 10px; + /* Bottom padding matches .scv-repository-header's, so both sections' + header→first-row gap reads the same. */ + padding: 2px 10px 4px 10px; color: var(--text-muted); font-size: 0.74em; font-weight: 600; @@ -533,6 +544,8 @@ body.is-mobile .scv-view-toggle-label { display: none; } .scv-change-item { display: flex; align-items: center; + flex-wrap: nowrap; + overflow: hidden; gap: 7px; min-height: 30px; padding: 5px 12px 5px 8px; @@ -600,11 +613,15 @@ body.is-mobile .scv-view-toggle-label { display: none; } } /* List-view variant: the name shrinks to its content so the folder path - suffix can sit on the right, disambiguating flat rows without nesting. */ -.scv-change-item-list .scv-change-name { flex: 0 1 auto; } + suffix can sit on the right, disambiguating flat rows without nesting. + flex-shrink stays low relative to .scv-change-path's (below) so the path + yields space first when the row is tight -- the filename stays readable + longest, both ellipsis rather than wrap. */ +.scv-change-item-list .scv-change-name { flex: 0 1 auto; flex-shrink: 1; min-width: 0; } .scv-change-path { - flex: 1 1 auto; + flex: 0 1 auto; + flex-shrink: 20; min-width: 0; margin-left: auto; overflow: hidden; @@ -1072,10 +1089,25 @@ body.is-mobile .scv-change-menu { min-width: 28px; min-height: 28px; } } /* ── Mobile adjustments ─────────────────────────────────────────── */ +/* Row-density baseline (Sync Queue/Repository List/Repository Tree file and + folder rows all read the same var(--scv-mobile-row-*) values -- see + .scv-root) applies both by platform (phone/tablet, body.is-mobile) and by + narrow panel width (@container), so a compact row shows up whichever + condition made the layout tight. */ +body.is-mobile .scv-change-item, +body.is-mobile .scv-tree-folder-row { + min-height: var(--scv-mobile-row-min-height); + padding: var(--scv-mobile-row-padding); +} + @container (max-width: 480px) { .scv-filter-menu { padding: 6px 8px; gap: 3px; } .scv-filter-option { padding: 4px 8px; font-size: 0.76em; } - .scv-change-item { padding: 6px 10px; } + .scv-change-item, + .scv-tree-folder-row { + min-height: var(--scv-mobile-row-min-height); + padding: var(--scv-mobile-row-padding); + } .scv-change-name { font-size: 0.76em; } } diff --git a/tests/ui/source-control/SourceControlView.test.ts b/tests/ui/source-control/SourceControlView.test.ts index df0fa0e..a98321b 100644 --- a/tests/ui/source-control/SourceControlView.test.ts +++ b/tests/ui/source-control/SourceControlView.test.ts @@ -1588,6 +1588,96 @@ describe('SourceControlView', () => { expect(onSync).toHaveBeenCalledWith([{ changeId: toChangeId('c-1'), action: undefined }]); }); + + describe('row density parity (Queue / Repository Tree / Repository List)', () => { + it('renders Queue, Tree, and List file rows all as .scv-change-item so they share one CSS density baseline', () => { + Platform.isMobile = true; + const { view, selection } = buildView([ + { id: toChangeId('c-1'), path: 'notes/a.md', kind: 'local-only' }, + { id: toChangeId('c-2'), path: 'notes/b.md', kind: 'local-only' }, + ]); + selection.selectForSync(toChangeId('c-1')); + view.render(container); + + const queueRow = container.querySelector('.scv-selected-section .scv-change-item'); + const treeRow = container.querySelector('.scv-changes-tree .scv-change-item'); + expect(queueRow).not.toBeNull(); + expect(treeRow).not.toBeNull(); + expect(queueRow?.classList.contains('scv-change-item')).toBe(true); + expect(treeRow?.classList.contains('scv-change-item')).toBe(true); + // Neither carries the list-only variant class -- both stay on + // the shared bare-row density baseline. + expect(queueRow?.classList.contains('scv-change-item-list')).toBe(false); + expect(treeRow?.classList.contains('scv-change-item-list')).toBe(false); + + (container.querySelector('.scv-view-toggle-btn[data-view="list"]') as HTMLButtonElement).click(); + const listRow = container.querySelector('.scv-changes-tree .scv-change-item'); + expect(listRow?.classList.contains('scv-change-item')).toBe(true); + expect(listRow?.classList.contains('scv-change-item-list')).toBe(true); + }); + + it('List mode keeps filename, folder path, diff stat, and the row menu inside one single-row element', () => { + Platform.isMobile = true; + const { view } = buildView([ + { id: toChangeId('c-1'), path: 'deep/nested/folder/report.md', kind: 'local-modified' }, + ]); + view.render(container); + (container.querySelector('.scv-view-toggle-btn[data-view="list"]') as HTMLButtonElement).click(); + + const rows = container.querySelectorAll('.scv-changes-tree .scv-change-item'); + // Exactly one row for the one change -- no extra wrapper rows + // that would indicate the content spilled onto a second line. + expect(rows).toHaveLength(1); + const row = rows[0] as HTMLElement; + expect(row.querySelector('.scv-change-name-text')?.textContent).toBe('report.md'); + expect(row.querySelector('.scv-change-path')?.textContent).toBe('deep/nested/folder'); + expect(row.querySelector('.scv-change-menu')).not.toBeNull(); + }); + + it('Tree mode omits the folder-path suffix (folders already convey location)', () => { + Platform.isMobile = true; + const { view } = buildView([ + { id: toChangeId('c-1'), path: 'deep/nested/folder/report.md', kind: 'local-modified' }, + ]); + view.render(container); + + const row = container.querySelector('.scv-changes-tree .scv-change-item') as HTMLElement; + expect(row.querySelector('.scv-change-path')).toBeNull(); + }); + + it('renders tree folder rows with the shared .scv-tree-folder-row class alongside file rows', () => { + Platform.isMobile = true; + const { view } = buildView([ + { id: toChangeId('c-1'), path: 'notes/a.md', kind: 'local-only' }, + { id: toChangeId('c-2'), path: 'notes/b.md', kind: 'local-only' }, + ]); + view.render(container); + + expect(container.querySelector('.scv-tree-folder-row')).not.toBeNull(); + expect(container.querySelector('.scv-tree-folder-row .scv-change-item')).toBeNull(); + }); + }); + + it('renders the mobile sync bar exactly once, as a sibling after the scrollable body (space reserved once, not overlaid)', () => { + Platform.isMobile = true; + const { view, selection } = buildView([ + { id: toChangeId('c-1'), path: 'a.md', kind: 'local-only' }, + ]); + selection.selectForSync(toChangeId('c-1')); + view.render(container); + + const bars = container.querySelectorAll('.scv-mobile-sync-bar'); + expect(bars).toHaveLength(1); + const bar = container.querySelector('.scv-mobile-sync-bar') as HTMLElement; + const body = container.querySelector('.scv-body'); + expect(body).not.toBeNull(); + // Sibling of .scv-body (same parent), not nested inside it or + // inside the independently-scrolling changes region -- so it sits + // in normal flow and its height is reserved exactly once. + expect(bar.parentElement).toBe(body?.parentElement); + expect(body?.contains(bar)).toBe(false); + expect(container.querySelector('.scv-changes-region')?.contains(bar)).toBe(false); + }); }); describe('header info strip', () => {