Skip to content

fix(ui): unify mobile Source Control row density across Queue, List, and Tree #156

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ClaudiaFang wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(ui): unify mobile Source Control row density across Queue, List, and Tree #156

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 38 additions & 6 deletions styles.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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; }
}

Expand Down
90 changes: 90 additions & 0 deletions tests/ui/source-control/SourceControlView.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
Loading