Skip to content

Commit b00bfe0

Browse files
alicodingclaude
andauthored
feat: display density -- Comfortable/Compact appearance preference (goal 0096) (#213)
Adds a Settings -> Appearance density control (Comfortable/Compact, Comfortable default) that tightens row-based surfaces' vertical padding without any component restructuring. A Go-persisted SettingsService preference (mirrors the existing MCP-write-toggle Get/Set shape) is applied as a data-density attribute on each window's document root -- both the main app and the Quick Panel's own separate Wails window -- via one shared frontend/src/shared/density.ts helper. Mechanism: a single --mill-density-row-pad-y custom property, defined at index.css's root under [data-density="compact"] with a companion- breakpoint floor that restores the 44px touch target, consumed by each in-scope surface's own CSS module (InventoryList, CommandPalette, QuickPanel, ReviewView/ActivityView's shared ListCard .card, Atlas's card-page .entry rows) via its own current value as the var() fallback -- so Comfortable (the attribute absent) stays byte-identical to today. DataTable cell padding is scoped out: its cellPadding preset sets --cell-padding-block directly on the Table element itself, which shadows any ancestor-scoped override per CSS custom-property cascade rules (a directly-matching rule always beats an inherited value) -- the same reason Primer's ActionList row padding IS reachable (it only reads --control-medium-paddingBlock, never sets it locally). Claude-Session: https://claude.ai/code/session_01FW5GkkAG8du7tNdYLk2zSd Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 7912ecc commit b00bfe0

16 files changed

Lines changed: 378 additions & 9 deletions

File tree

frontend/bindings/github.com/alicoding/mill/internal/services/settingssvc/settingsservice.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,17 @@ export function GetBuildInfo(): $CancellablePromise<$models.BuildInfo> {
160160
return $Call.ByID(2673585232);
161161
}
162162

163+
/**
164+
* GetDisplayDensity returns the persisted preference, defaulting to
165+
* Comfortable when unset or set to anything other than the one
166+
* recognized override -- Comfortable is required to match today's
167+
* unset behavior exactly (docs/goals/0096's "zero visual diff when
168+
* unset" acceptance bar).
169+
*/
170+
export function GetDisplayDensity(): $CancellablePromise<string> {
171+
return $Call.ByID(3640093526);
172+
}
173+
163174
/**
164175
* GetLaunchAtLogin queries the real OS state (System Events' login
165176
* items list) rather than a persisted preference -- authoritative even
@@ -357,6 +368,15 @@ export function SetAttentionIdleThreshold(seconds: number): $CancellablePromise<
357368
return $Call.ByID(454955395, seconds);
358369
}
359370

371+
/**
372+
* SetDisplayDensity persists the preference. Rejects any value besides
373+
* the two locked tiers so a typo'd/future caller can't wedge the
374+
* preference into a state no CSS selector matches.
375+
*/
376+
export function SetDisplayDensity(density: string): $CancellablePromise<void> {
377+
return $Call.ByID(1473451146, density);
378+
}
379+
360380
/**
361381
* SetKeybinding overrides commandID's binding to mods+key, rejecting a
362382
* combo already claimed by another command's override, or by a
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
import { test, expect } from './fixtures/server'
2+
3+
// docs/goals/0096-display-density.md: Comfortable/Compact, a
4+
// Settings-driven app root attribute (data-density) that tightens
5+
// row-based surfaces' vertical padding. Measures a real workflow row's
6+
// rendered height rather than any internal Primer class/DOM shape --
7+
// robust to how ActionList happens to structure its own markup, and a
8+
// direct proxy for the CSS custom property chain this goal wires
9+
// (index.css's --mill-density-row-pad-y -> InventoryList.module.css's
10+
// --control-medium-paddingBlock override).
11+
12+
async function firstWorkflowRow(page: import('@playwright/test').Page) {
13+
await page.getByRole('link', { name: 'Workflows' }).click()
14+
const row = page.locator('[data-testid="inventory-row"][data-entity="workflow"]').first()
15+
await expect(row).toBeVisible()
16+
return row
17+
}
18+
19+
async function setDensity(page: import('@playwright/test').Page, label: 'Comfortable' | 'Compact') {
20+
await page.getByRole('link', { name: 'Settings' }).click()
21+
await expect(page.getByTestId('settings-view')).toBeVisible()
22+
await page.getByTestId('density-control').getByRole('button', { name: label }).click()
23+
}
24+
25+
test('Compact visibly tightens a workflow row, persists across reload, and Comfortable restores the original height', async ({ page }) => {
26+
await page.goto('/')
27+
28+
const comfortableRow = await firstWorkflowRow(page)
29+
const comfortableBox = await comfortableRow.boundingBox()
30+
const comfortableHeight = comfortableBox?.height ?? 0
31+
expect(comfortableHeight).toBeGreaterThan(0)
32+
33+
await setDensity(page, 'Compact')
34+
const compactRow = await firstWorkflowRow(page)
35+
await expect.poll(async () => (await compactRow.boundingBox())?.height ?? 0).toBeLessThan(comfortableHeight)
36+
37+
await page.reload()
38+
const afterReloadRow = await firstWorkflowRow(page)
39+
await expect.poll(async () => (await afterReloadRow.boundingBox())?.height ?? 0).toBeLessThan(comfortableHeight)
40+
41+
// Leaves the shared e2e settings file back at the default, matching
42+
// this repo's other settings specs' own cleanup discipline.
43+
await setDensity(page, 'Comfortable')
44+
const restoredRow = await firstWorkflowRow(page)
45+
await expect.poll(async () => (await restoredRow.boundingBox())?.height ?? 0).toBe(comfortableHeight)
46+
})
47+
48+
test('Density select reflects the persisted preference on a fresh Settings visit', async ({ page }) => {
49+
await page.goto('/')
50+
await setDensity(page, 'Compact')
51+
52+
await page.reload()
53+
await page.getByRole('link', { name: 'Settings' }).click()
54+
await expect(page.getByTestId('density-control').getByRole('button', { name: 'Compact' })).toHaveAttribute('aria-pressed', 'true')
55+
56+
// Cleanup, same reasoning as the test above.
57+
await setDensity(page, 'Comfortable')
58+
})
59+
60+
test.describe('mobile viewport', () => {
61+
test.use({ viewport: { width: 390, height: 844 } })
62+
63+
test('Compact keeps a 44px workflow row touch target at the companion breakpoint', async ({ page }) => {
64+
await page.goto('/')
65+
await page.getByTestId('mobile-nav-toggle').click()
66+
await page.getByRole('link', { name: 'Settings' }).click()
67+
await expect(page.getByTestId('settings-view')).toBeVisible()
68+
await page.getByTestId('density-control').getByRole('button', { name: 'Compact' }).click()
69+
70+
await page.getByTestId('mobile-nav-toggle').click()
71+
const row = await firstWorkflowRow(page)
72+
const box = await row.boundingBox()
73+
expect(box?.height ?? 0).toBeGreaterThanOrEqual(44)
74+
75+
// Cleanup, same reasoning as the tests above.
76+
await page.getByTestId('mobile-nav-toggle').click()
77+
await page.getByRole('link', { name: 'Settings' }).click()
78+
await page.getByTestId('density-control').getByRole('button', { name: 'Comfortable' }).click()
79+
})
80+
})

frontend/src/app/App.tsx

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import { CommandPalette } from "./CommandPalette";
2323
import { ShortcutsHelpDialog } from "./ShortcutsHelpDialog";
2424
import { BuildIdentityBadge } from "./BuildIdentityBadge";
2525
import { COLOR_MODE_STORAGE_KEY, SIDEBAR_OPEN_STORAGE_KEY } from "./theme";
26+
import { applyDensity } from "../shared/density";
2627
import { pageIconFor, pageLabelFor } from './pageMeta'
2728
import { useMillNavigate } from './useMillNavigate'
2829
import { useKeymapDispatch } from './useKeymapDispatch'
@@ -205,6 +206,15 @@ function App() {
205206
SettingsService.IsIsolatedData().then(setIsIsolatedData).catch(console.error);
206207
}, []);
207208

209+
// Display density (docs/goals/0096): applied once here, on mount, so
210+
// a Compact preference holds from first paint even when Settings is
211+
// never opened -- SettingsView applies its own change instantly
212+
// (ahead of this fetch) when the control is used, this covers every
213+
// other launch.
214+
useEffect(() => {
215+
SettingsService.GetDisplayDensity().then((d) => applyDensity(d === 'compact' ? 'compact' : 'comfortable')).catch(console.error);
216+
}, []);
217+
208218
useEffect(() => {
209219
SettingsService.GetBuildInfo().then(setBuildInfo).catch(console.error);
210220
SettingsService.AppVersion().then(setAppVersion).catch(console.error);

frontend/src/app/CommandPalette.module.css

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,11 @@
2424
here). */
2525
.list {
2626
max-height: min(60vh, 480px);
27+
/* Display density (docs/goals/0096) -- same ActionList row-padding
28+
override as shared/InventoryList.module.css's own .list rule; see
29+
its comment for why this is safe to scope here rather than
30+
globally. */
31+
--control-medium-paddingBlock: var(--mill-density-row-pad-y);
2732
}
2833

2934
/* Workflow pin toggle (docs/goals/BACKLOG.md Standing #5) -- same pair

frontend/src/app/QuickPanel.module.css

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,14 @@
1111
background: var(--bgColor-default);
1212
color: var(--fgColor-default);
1313
overflow: hidden;
14+
/* Display density (docs/goals/0096) -- same ActionList row-padding
15+
override as shared/InventoryList.module.css's own .list rule (see
16+
its comment); scoped here rather than on the FilteredActionList
17+
element itself since QuickPanel.tsx is at architecture.md's
18+
500-line cap and can't take a className-prop edit -- .panel is
19+
already its ancestor, so the override reaches the same rows either
20+
way. */
21+
--control-medium-paddingBlock: var(--mill-density-row-pad-y);
1422
}
1523

1624
.status {

frontend/src/app/QuickPanelApp.tsx

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
1+
import { useEffect } from 'react'
12
import { ThemeProvider, BaseStyles } from '@primer/react'
23
import { QuickPanel } from './QuickPanel'
34
import { COLOR_MODE_STORAGE_KEY } from './theme'
5+
import { SettingsService } from '../shared/bindings'
6+
import { applyDensity } from '../shared/density'
47

58
// docs/adr/0033-quick-panel-second-window.md: the dedicated shell for
69
// the Quick Panel's own Wails window (loaded at the '#/quickpanel' hash
@@ -13,6 +16,23 @@ import { COLOR_MODE_STORAGE_KEY } from './theme'
1316
// 'auto' independently of it.
1417
export function QuickPanelApp() {
1518
const initialColorMode = (localStorage.getItem(COLOR_MODE_STORAGE_KEY) as 'light' | 'dark' | 'auto' | null) ?? 'auto'
19+
20+
// Display density (docs/goals/0096): a second, independent instance
21+
// of App.tsx's own mount-time fetch+apply -- this window is a
22+
// separate Wails webview/JS context (goal 0017's per-window fetch
23+
// pattern, QuickPanel.tsx's own comment), so App.tsx's effect never
24+
// reaches it. Re-applied on every window focus (not just mount) since
25+
// QuickPanel.tsx itself is at architecture.md's 500-line cap and
26+
// can't grow its own focus-triggered refresh list to include this.
27+
useEffect(() => {
28+
const fetchAndApply = () => {
29+
SettingsService.GetDisplayDensity().then((d) => applyDensity(d === 'compact' ? 'compact' : 'comfortable')).catch(console.error)
30+
}
31+
fetchAndApply()
32+
window.addEventListener('focus', fetchAndApply)
33+
return () => window.removeEventListener('focus', fetchAndApply)
34+
}, [])
35+
1636
return (
1737
<ThemeProvider colorMode={initialColorMode}>
1838
<BaseStyles>

frontend/src/app/index.css

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,34 @@
3838
(deliberately, not here) -- see that file's header comment for why
3939
this needed a real separate file rather than living in this one. */
4040

41+
/* Display density (docs/goals/0096): one row-vertical-padding lever,
42+
read by every in-scope surface's own CSS module (InventoryList,
43+
CommandPalette, QuickPanel, ReviewView's ListCard .card, Atlas's
44+
card-page .entry rows) via var(--mill-density-row-pad-y, <that
45+
surface's own current value>) -- undefined here at rest, so
46+
Comfortable (the attribute absent) falls through to each surface's
47+
own unchanged fallback with zero diff. Scoped to [data-density] on
48+
the document root, not .app-shell: the Quick Panel is a second,
49+
separate Wails window/document with no .app-shell element of its own
50+
(docs/adr/0033), and this rule must reach both. 4px is Primer's own
51+
--base-size-4 step, ~67% of ActionList's 6px default row padding --
52+
the dominant lever among the in-scope surfaces' current values. */
53+
:root[data-density="compact"] {
54+
--mill-density-row-pad-y: var(--base-size-4, 4px);
55+
}
56+
/* Touch-target floor (docs/goals/0096's DoR: "floor wins over
57+
density"): at the companion breakpoint, compact's row padding
58+
restores to whatever keeps a single-line ActionList row's total
59+
height at or above the 44px minimum target (20px label line-height +
60+
2*12px), overriding the desktop compact value above rather than
61+
compounding with it. Comfortable is untouched at any width -- this
62+
query only ever fires under [data-density="compact"]. */
63+
@media (max-width: 767px) {
64+
:root[data-density="compact"] {
65+
--mill-density-row-pad-y: var(--base-size-12, 12px);
66+
}
67+
}
68+
4169
* { box-sizing: border-box; }
4270

4371
html {

frontend/src/atlas/AtlasCardPage.module.css

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,11 @@
131131
.entry {
132132
border: 1px solid var(--borderColor-muted);
133133
border-radius: 6px;
134-
padding: 8px 10px;
134+
/* Display density (docs/goals/0096): only the vertical component
135+
reads the density var -- see shared/ListCard.module.css's .card
136+
for the same split and its own reasoning. */
137+
padding-block: var(--mill-density-row-pad-y, 8px);
138+
padding-inline: 10px;
135139
cursor: pointer;
136140
}
137141
.entry:hover {

frontend/src/locales/en/views.json

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,10 @@
1919
"themeLabel": "Color theme",
2020
"lightLabel": "Light theme",
2121
"darkLabel": "Dark theme",
22-
"systemLabel": "Match system theme"
22+
"systemLabel": "Match system theme",
23+
"densityLabel": "Density",
24+
"comfortableOption": "Comfortable",
25+
"compactOption": "Compact"
2326
},
2427
"general": {
2528
"launchAtLoginLabel": "Launch Mill at login",

frontend/src/shared/InventoryList.module.css

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,22 @@
1414
}
1515
.muted { color: var(--fgColor-muted); }
1616

17+
/* Display density (docs/goals/0096): overrides Primer's own
18+
ActionList.Item row-padding token, scoped to just this list (not
19+
globally -- --control-medium-paddingBlock also drives Button
20+
padding elsewhere in the app, which density must never touch). A
21+
directly-set value on the row's own element would shadow an
22+
inherited one (confirmed against the installed @primer/react's
23+
compiled ActionList CSS -- DataTable's cellPadding preset works the
24+
same way, which is why table cell padding stays out of density's
25+
reach), but ActionList never sets this token on itself, only reads
26+
it, so setting it here on an ancestor reaches every row underneath
27+
uncontested. Undefined var(--mill-density-row-pad-y) at Comfortable
28+
falls through to Primer's own unchanged default. */
29+
.list {
30+
--control-medium-paddingBlock: var(--mill-density-row-pad-y);
31+
}
32+
1733
/* Single-line label discipline (goal 0007's dense-row pattern): the
1834
label ellipsizes against whatever width ActionList's content slot
1935
grants it; badges hold the line and never wrap underneath. min-width

0 commit comments

Comments
 (0)