DarkRoom/docs/ui-refinement.md

# UI refinement: toward a Lightroom-shaped darkroom

TRACES: FR-UI-1 | FR-UI-3 | FR-DEV-3a | FR-CAT-4

## Why

The v0.1 UI proved the architecture: capability-driven controls, a windowed
grid, a GPU canvas with no CPU round trip. What it has not yet done is *feel*
like a photo editor. The gaps are structural rather than cosmetic, and this
document names them so they can be closed independently.

Four principles guide every change below.

1. **The image is the subject.** Chrome recedes; nothing competes with the
   photograph for attention or for colour.
2. **Colour is a signal, not a decoration.** The accent means *modified* or
   *active*. Everywhere it currently means "heading" or "chrome", it is
   spending a signal on noise.
3. **Navigation is continuous.** Moving between images should not be a change
   of screen. Lightroom's filmstrip is the mechanism; modal view switching is
   what it replaces.
4. **Density is earned.** A panel shows what has been touched; everything else
   collapses out of the way.

## Non-goals

- No new pipeline operations, and no change to how operations reach the panel.
  `AdjustPanel` must still learn its contents from the capability model and
  must still name no operation (FR-DEV-3a).
- No change to the catalog schema, the sync layer, or the render path.
- No light theme. The ground stays dark (see `theme.slint` preamble).

---

## Workstream S — The style layer

Prerequisite for everything else that touches colour. Lands before B–F.

### S1 — Near-neutral palette

**Problem.** The original palette was a warm "darkroom safelight" brown —
ground `#14120F`, R twelve points above B, and the same cast through every
surface and ink. That biases the work. Simultaneous contrast pushes perception
of the image *away* from its surround, so warm chrome makes a neutral
photograph read cool; the photographer corrects toward warm to compensate and
every export drifts yellow. The file's own preamble had the right instinct —
a light UI biases judgement — and stopped one step short. Warmth biases it
too, and more quietly, because a warm cast reads as *cosy* rather than
*wrong*.

**Done.** `theme.slint` now carries near-neutral greys with a 2–3 point cool
lift (pure R=G=B reads as dead; a trace of cool reads as instrument), and the
red accent is replaced by an achromatic `active` family. `warn-ink` is the
only hue left — a caution is genuinely a different kind of thing from an
active state.

Migration shims alias `accent`/`accent-dim`/`accent-hover` onto the new
tokens, so the tree compiles while call sites migrate. **They are temporary.**
Delete them once `grep -rn 'Theme.accent' ui/` is empty.

### S2 — `style.yaml` as the token source

**Problem.** Tokens live in Slint, so tuning a palette means editing a
language file, and nothing else — docs, tooling, a future export theme — can
read them.

**Deliverable.** `ui/dr-ui/style.yaml` becomes the source of truth for
**colours and lengths**: the whole current token set, nothing more.

- `ui/dr-ui/build.rs` reads it and generates `theme.slint` at compile time.
  Zero runtime cost, and a malformed file is a build error rather than a
  failure in front of a photographer mid-edit.
- The generated file carries a "do not edit" banner naming its source, and
  must land somewhere `.gitignore`d or clearly marked generated — a
  hand-edited generated file is a bug that hides for weeks.
- **The prose survives.** The reasoning in today's `theme.slint` preamble and
  its per-token comments is the most valuable thing in the file. YAML comments
  carry across into the generated Slint, or the generator emits them from
  structured fields. A token set with the *why* stripped out is a downgrade,
  however tidy the pipeline.
- Debug-only live reload behind a feature flag: re-read the YAML at startup so
  a palette can be tuned without a full rebuild. Off in release, where the
  generated constants are what ship.

**Not in the YAML.** Semantic components (S3). `PanelHeading` binds colour,
size, weight and letter-spacing into one concept — that is Slint, not data.
YAML holds leaf values; components compose them.

**Dependency.** Adds a YAML parser to the UI's build-dependencies. Note that
`serde_yaml` was deprecated in 2024; prefer a maintained alternative.

### S3 — Semantic components

**Problem.** `theme.slint` says what `surface` is. It says nothing about what
a *panel heading* is — so every file re-derives one. `IMAGE`, `ADJUST`, the
six launch-screen headings, `library.slint:267` each independently spell out
colour, size, weight and letter-spacing for a single concept. That is why one
accent reached forty call sites: there was no single place to change it.

**Deliverable.** `widgets.slint` grows from three primitives into the style
layer:

- `PanelHeading` — the `IMAGE`/`ADJUST`/launch headings. One definition, one
  place to decide headings are ink rather than accent.
- `Label`, `Value`, `Caption` — text roles, so `text-sm` + `ink-dim` stops
  being copy-pasted. `Value` carries the modified state, since a value that
  differs from its default is the one thing worth spotting at a glance.
- `Panel` — surface + rule + padding, currently rebuilt in four places.
- `Field` — the launch screen's text input with its focus border.

**The rule this establishes.** Files consume components. Raw `Theme.*` is for
*composing* a component, not for styling a call site. A new colour literal or
a bare `Theme.ink-faint` in a screen file is a signal that a component is
missing.

**Sweep.** Migrating call sites onto these components removes the `accent`
references as a side effect — the forty sites collapse into a handful of
component definitions. That is the point: fix the cause, not the symptom.
Delete the S1 shims when the grep comes back empty.

**Done when.** `grep -rn 'Theme.accent' ui/` is empty, the shims are gone, no
screen file styles a heading inline, and the UI carries no hue outside
`warn-ink`.

---

## Workstream P — Plural presentation hints

Implements ARCH §4.3a. Touches `core/dr-pipeline` and `ui/dr-ui/src/develop.rs`;
no Slint change beyond what falls out of it.

**Problem.** `Presentation.widget` is a single `WidgetKind`. An operation can
name one preferred control and nothing else, so a curve cannot say "a curve
editor is best, a parametric band control would do, and sliders are fine" and
let the frontend choose. Since layout class already drives compact-vs-expanded,
this is not hypothetical: a curve editor is comfortable in a 280px panel and
unusable in a 120px one, and today the frontend has no sanctioned way to
decide that — its only options are the named widget or nothing.

**Deliverable.**

- `Presentation.widget` becomes `widgets: &'static [WidgetKind]`, in descending
  preference. The frontend takes the first it implements and can afford.
- A `WidgetDemand` describing what a widget inherently requires — candidates:
  two-dimensional direct manipulation, precision pointing, a minimum count of
  simultaneous values. **No pixels, no breakpoints, no DPI, no platform names.**
  Those are frontend thresholds and live in `dr-ui`.
- `develop.rs` chooses per operation: walk the hint list, take the first whose
  demands the current layout satisfies, else fall through to plain scalars.
  The existing `ParamRow.kind` string is where exhaustive matching is currently
  lost — the choice must happen in Rust against the enum, before flattening.
- The tone curve declares `[Curve]` with its real demands. Nothing else needs
  to change; operations wanting sliders still say nothing at all.

**Verification.** The existing `the_curve_collapses_to_a_single_row` test in
`develop.rs` covers the happy path. Add its complement: with a layout that
cannot satisfy the curve's demands, the same capability must produce ten
addressable scalar rows and remain fully editable. That test is the contract —
it is what makes the fallback real rather than aspirational.

**Do not** let a demand grow a `min_width`. If one seems necessary, the
demand vocabulary is wrong; widen the vocabulary, not the abstraction.

---

## Workstream M — The colour mixer is unreadable

Found in use, not in review. The panel currently renders the mixer as
thirty-six anonymous sliders reading `Hue 0 / Sat 16 / Lum 0` twelve times
over, with nothing saying which band any row belongs to. Three separate
defects meet here.

### M1 — Band identity is lost (a bug, not a style issue) — **done**

`labels.rs` has no `param.mixer.*` entries, so all thirty-six keys fall
through to a default that yields the bare channel name. The core is not at
fault: it declares `param.mixer.orange.sat`, and `BANDS` carries `key:
"orange"` with `hue: 30.0`. The identity is present in the capability and
discarded at resolution.

**Deliverable.** Resolve mixer keys to their band. A row reads `Orange · Sat`,
or `Sat` under a band heading — M3 decides which.

**Landed** as a `band.*` catalogue resolving the *subject* of a faceted
parameter (M2), rather than as entries for the thirty-six `param.mixer.*`
keys. Three bands are catalogued to something other than their key: chartreuse
reads "Yellow-Green" and spring "Blue-Green", because a photographer looking
for foliage does not scan a list for "Spring".

### M2 — Bands carry their centre hue as capability data — **done**

**Deliverable.** `ParamDescriptor` gains an optional band hue in degrees, set
by `band_params!` from `BANDS`. The UI converts degrees to a swatch.

**Landed** as `descriptor::Facet` — a little wider than "a band hue", and the
width is what M3 turned out to need. A parameter may say which **aspect** it
adjusts (the channel) and which **subject** it adjusts it on (the band), with
the subject's hue attached where the subject is a colour. `ParamDescriptor` is
otherwise unchanged and `faceted()` is a const builder step, so the thirty-six
descriptors stay `static` and every other operation says nothing at all.

The hue reaches the screen as `Swatch` in `widgets.slint`, which owns the
saturation and brightness. Those two are *not* in `style.yaml`: it holds
colours and lengths, and a third section for two floats used in one component
buys less than it costs. `Theme.swatch` — the square's size — is a length and
does live there.

**Why this is not a §4.3a violation.** A band's centre hue is a *fact about
the operation* — the mixer genuinely acts on the 30° band, and that number is
what it acts on. The core says "this parameter belongs to the band centred at
30°". It does not say what colour to draw, at what saturation or lightness, or
whether to draw a swatch at all. Those conversions are presentation and live
in `dr-ui`; the swatch's saturation and lightness belong in `style.yaml`.

The line to hold: a hue in degrees is data. A hex colour in a descriptor
would be the core deciding appearance, and is forbidden.

**Swatches are the one sanctioned exception to the achromatic palette.** A
swatch is not chrome — it is data identifying which hue band a row edits,
exactly as an image is data. That is categorically different from an accent
decorating a heading, which is what the palette rule forbids. Keep them small
and let them identify, never dominate.

### M3 — Structure: three runs of twelve, not twelve of three — **done**

~~The mixer renders as twelve `Section`s — one per band, titled by band name,
carrying its swatch — each holding Hue, Sat and Lum. Collapsed by default.~~

**Superseded, twice over.** The lids came off the develop column entirely
(Workstream C's sections are now plain headings), so "collapsed by default"
had nothing left to mean. And the grouping was the wrong way round: an edit is
almost never "everything about orange", it is "the saturation of the greens",
made by comparing one channel across neighbouring bands. Twelve band sections
put the twelve rows you want to compare in twelve different places.

**Landed.** Three runs — Hue, Saturation, Luminance — of twelve rows each,
under the operation's own heading. `develop.rs` stacks the rows by aspect
(`presentation_order`) and marks the first of each run; `adjust.slint` names
the run once and draws the rest. Each row is a swatch, a track and a readout on
one line: the swatch *is* the label, which is what makes twelve rows fit where
four did, and the band name lives on as the row's accessible label so the
control is not colour-only.

The mixer's declaration order is untouched — it declares band by band, which
is the order the shader wants. Rearranging it for the panel would have been
the core laying out a screen (§4.3a); doing it in `develop.rs` is the same
frontend-side derivation that decides there are groups at all.

Nothing here is mixer-specific: any operation whose parameters carry facets
groups this way, and one that carries none is untouched.

### M4 — A single-parameter operation should not cost a heading

Vibrance and Saturation each render a section heading above one slider,
spending two lines and a visual break on one control. An operation whose
parameters number one wants to *be* a named row, not a group containing one.

**Deliverable.** The panel collapses a single-parameter operation into one
row labelled by the operation. Derived frontend-side from the parameter
count — the core says nothing about it, per §4.3a.

**Done when.** Every mixer row says which band it edits; the rows for one
channel read as a run rather than as twelve unrelated sliders; Vibrance and
Saturation are one row each; and no hex colour appears in any descriptor.
**All four are met.**

---

## Workstream V — Vertical density

The panel spends too much height on too little information. Reported from
use, and it compounds Workstream M: at thirty-six mixer rows the waste is
measured in whole screens.

**The arithmetic.** `ParamSlider` is a fixed 46px carrying an 11px label and a
3px track. The track region is `Theme.touch-target / 2` — 22px — which is a
finger-sized allowance drawn for a pointer, and the label occupies a line of
its own above it. Every section heading adds a further `Theme.gap` (12px)
spacer plus a 2px rule. Twelve mixer bands at three rows each is roughly
1650px of panel, most of it air.

**The cause is layout, not spacing tokens.** Shaving pixels uniformly would
compress the readable parts along with the waste. The row is stacked when it
could be inline: name left, value right, track beneath — which is what
Lightroom does, in about 32px.

**Deliverable.**

- Rework `ParamSlider` so label and value share one line and the track sits
  under them. Target ~32px per row, down from 46px.
- The *drawn* track shrinks; the **TouchArea does not**. FR-UI-3 is about the
  finger, and `Button` already establishes the pattern — draw at control
  height, grow the hit target past the ink and centre it. A denser panel must
  not become a less touchable one.
- Section heading spacing comes from one token, not an inline `Rectangle {
  height: Theme.gap }`. A spacer rectangle written inline is how the panel
  ended up with spacing nobody can adjust centrally.
- Re-check the compact layout class after the change: rows that work at 280px
  may crowd at narrower widths, where the touch overhang also matters most.

**Constraint.** Density is not the goal; *legibility per pixel* is. If a row
gets shorter and harder to read, it has failed. The value readout in
particular carries the modified signal and must stay scannable.

**Done when.** A parameter row is ~32px, hit targets still meet FR-UI-3 under
touch, heading spacing is tokenised, and the panel reads as easily at the new
density as the old.

---

## Workstream A — Shared chrome primitives

**Problem.** Buttons are hand-rolled `Rectangle` + `TouchArea` pairs in
`app.slint`, `library.slint`, and `launch.slint`, at three different sizes
(64×20, 110×28, 88×28) with three near-identical hover/press treatments. Any
consistency in the chrome is currently coincidental.

**Deliverable.** A new `ui/dr-ui/ui/widgets.slint` exporting:

- `Button` — `text`, `enabled`, `primary` (bool), `clicked()`. Height meets
  `Theme.touch-target` under compact layout and may be denser when expanded.
  Press and hover states derive from theme tokens, not literals.
- `IconButton` — square, for toolbar affordances that carry a glyph.
- `Section` — a collapsible container: `title`, `modified` (bool),
  `expanded` (in-out bool), a default child slot. Draws the disclosure
  triangle and the modified dot. Workstream C consumes this.

Every existing hand-rolled button is replaced by `Button`. The visual result
should be a *narrower* range of sizes than today, not a wider one.

**Theme additions.** `theme.slint` gains what the widgets need and no more:
`radius-sm`/`radius`, a `hover` and `pressed` surface token, and a
`modified` token aliased to `accent`. Adding tokens is preferred over
literals appearing in widget bodies.

**Done when.** No `TouchArea` inside a `Rectangle` styled as a button remains
in `app.slint`, `library.slint`, or `library.slint`'s header. `cargo build`
clean, app launches, every button still fires its callback.

---

## Workstream B — Canvas presentation

**Problem.** The canvas fills its container edge to edge. The image reads as a
texture rather than a print, and there is no visual separation between the
photograph and the panel beside it.

**Deliverable.** In `app.slint`'s `canvas-area`:

- Inset the image by a margin that scales with the layout class — generous
  when `expanded`, tighter when compact, never zero. The surrounding field is
  `Theme.ground`.
- A subtle 1px `Theme.rule` border on the image bounds, so a dark photograph
  does not bleed into the dark ground. This requires knowing the *fitted*
  rectangle, not the container — if that proves awkward in Slint, a shadow or
  a very slightly lighter mat behind the image is an acceptable substitute.
  Pick one and say which in the summary.
- Empty and error states keep their current copy and centring.

**Constraint.** `canvas-resized` must continue to report the *drawable* pixel
size — the render target follows the image area, not the container. Getting
this wrong shows up as a soft or stretched image, so verify the reported size
changes when the margin does.

**Done when.** The image sits in a visible field with margin, the border or
mat is present, and resizing the window still produces a crisp canvas.

---

## Workstream C — Collapsible adjust sections

**Problem.** `AdjustPanel` renders every parameter of every operation, always
expanded. This is tolerable at today's operation count and unusable at
fifteen. Lightroom's right panel is a stack of collapsible modules whose
headers report whether anything inside has been touched.

**Deliverable.** Rework the `for row[i] in root.rows` body in `adjust.slint`
to group by operation and wrap each group in `Section` (Workstream A).

The hard part is that `rows` is a **flat** model with a `starts-group` flag —
Slint cannot easily nest a `for` inside a group boundary derived at runtime.
Two viable approaches; pick one and justify it briefly:

1. **Flatten the collapse.** Keep the flat `for`, add an `expanded` bool per
   op-index held in the panel, and make each non-heading row `visible: false`
   and zero-height when its group is collapsed. Simple, no Rust change.
2. **Nest the model.** Have Rust supply `[[ParamRow]]` — one inner model per
   operation. Cleaner Slint, but changes the `ParamRow` contract and the
   `develop.rs` code that builds it.

Approach 1 is likely correct for this pass; prefer it unless it proves
unworkable.

**Modified indicator.** A group is modified when any row in it has
`value != default-value`.

**This must be derived in `dr-ui`, not supplied by the core** (ARCH §4.3a).
A `group_modified` flag on a descriptor would be the core deciding the panel
has groups at all, which is a composition decision. `develop.rs` already holds
both the capabilities and the live values, so it can aggregate per operation
while flattening — that is frontend-side derivation and stays on the right
side of the line. What it must not do is ask the core for the answer.

The same reasoning condemns the existing `starts-group` flag, which is the
core telling the panel where to draw section breaks. It predates this contract;
fold it into the same pass and derive grouping from `op-index` changes instead.

**Also.** The per-group `reset` should live on the section header, alongside
the existing global `reset`.

**Invariant.** This file must still name no operation. Collapse state is keyed
by `op-index`, never by label.

**Done when.** Sections collapse and expand, collapsed state survives a slider
drag elsewhere in the panel, headers show a modified dot that appears and
disappears as values move off and back to default, and adding an operation to
the pipeline still requires no edit to `adjust.slint`.

---

## Workstream D — Grid refinement

**Problem.** Cells are boxes first and images second: `image-fit: contain` on
a square cell leaves landscape shots floating in dead space, the cell surface
contrasts with the ground so the grid reads as a rhythm of rectangles, and
there is hover state but no *selection* state.

**Deliverable.** In `library.slint`:

- Cells crop to fill (`image-fit: cover`) with `clip: true`, so the grid is a
  rhythm of images. The filename caption stays.
- Cell background moves to `Theme.ground` or very near it; the frame recedes.
- A **selected** cell gets a persistent accent ring. Add
  `in property <int> selected-index` to `LibraryGrid`, defaulting to -1, and
  have `library_ui.rs` set it when a cell is clicked. Hover stays distinct
  from selection — a dimmer treatment.
- Keyboard navigation: arrow keys move the selection, Enter opens it. This
  needs a `FocusScope` over the grid and a `selection-moved(int)` callback.

**Done when.** The grid reads as images rather than boxes, the current image
is unambiguous, and arrows plus Enter navigate it without the mouse.

### Keyboard navigation — **done**

Arrows walk the grid, shift+arrow extends the selection, Home/End reach the
ends, PageUp/PageDown move by a screenful, and `Return` opens what the cursor
is on. With the judgement keys the grid already bound, a culling pass is now a
keyboard job end to end — which is the point: a cull is thousands of decisions,
and reaching for the mouse between each is the difference between an hour and
an evening.

**The cursor is a library ordinal, not a row of the loaded window** — that is
the whole design, and it is the same argument the selection already made by
keying on ids. The window is a few screenfuls around wherever the user is
looking; a cursor held as a row would stop at the window's edge or, worse, keep
counting into cells that belong to other photographs. Walking out of the window
reloads it around the new position, exactly as scrolling does.

The same fault was already live in the **anchor**, which was a window row: a
shift-click after a scroll extended from whatever image had drifted into that
row. It is now an ordinal too, and `apply_press` takes the window's offset to
map between the two. A range longer than the loaded window truncates to what is
loaded — selection is by id, and an image the catalog has not been asked for
has no id to select — which is the honest failure, not the silent one.

`select_row` is shared by the pointer and the keyboard so the two cannot drift:
"click here, shift+down twice" has to mean what "click here, shift-click there"
means. The one deliberate difference is that a plain arrow collapses the
selection onto the cursor, where a plain *click* on an already-selected cell
leaves it alone — that exception exists so a multi-image drag can start from
one of its members, and there is no drag behind a keystroke.

**Not done here:** a cursor marker distinct from the selection ring. A plain
arrow selects what it lands on, so the ring shows it; only during a shift
extension is the moving end indistinguishable from the rest of the range.
That wants a `cursor` flag on `LibraryCell` and a second ring treatment.

**Still open in D:** cells cropping to fill, the cell background receding to
the ground, and hover reading distinctly from selection.

---

## Workstream E — Chrome hierarchy

**Problem.** `StatusBar` mixes three unrelated things: navigation (`‹
Library`), identity (filename, position), and spike telemetry (fps, adapter,
backend, layout-class). The telemetry earned its place while assumption A1 was
open; it is now permanent furniture competing with the photograph.

**Deliverable.**

- The top strip carries identity and navigation only: filename, position,
  and the library affordance.
- Telemetry moves behind a toggle — a keyboard shortcut (suggest `` ` ``) that
  reveals a small diagnostics overlay in a corner of the canvas, carrying
  backend, adapter, fps, and layout class. Default off.
- The accent stops being used for chrome. `backend` in the status bar, the
  `IMAGE` and `ADJUST` panel headings, and the section headings in
  `adjust.slint` all move to `Theme.ink-faint` or `ink-dim`. After this pass,
  accent should appear only on: modified values, the modified dot, the curve
  line, slider fill, selection, and progress.

**Done when.** A fresh launch shows no fps counter and no accent-coloured
chrome; `` ` `` toggles the diagnostics overlay; every previously-visible
diagnostic is still reachable.

---

## Workstream F — Filmstrip and unified view

**The big one.** Depends on A (for `Button`) and D (for cell treatment and
selection). Should land last.

**Problem.** Library and Develop are mutually exclusive screens
(`show-library` in `app.slint`). Every move between images is a change of
screen. Lightroom's continuity comes from the grid never fully leaving: it
collapses to a filmstrip along the bottom of the develop view, and clicking a
neighbour is navigation, not a mode change.

**Deliverable.**

- A `Filmstrip` component in a new `ui/dr-ui/ui/filmstrip.slint`, consuming
  the **same** `[LibraryCell]` model and the same `selected-index` as
  `LibraryGrid`. Horizontal, ~90px tall, scrolls to keep the selection
  visible.
- Develop gains the filmstrip along its bottom edge, visible when a library is
  open (i.e. when the current model is non-empty). Command-line file sets get
  it too — they are also a list of images.
- Clicking a filmstrip cell loads that image. Arrow keys drive both the
  filmstrip and the existing next/prev, which become the same action.
- `show-library` becomes a *mode* rather than a screen swap: Grid mode and
  Develop mode over one shared library state, toggled by a `G`/`D` shortcut
  and by the existing buttons. The `can-return-to-library` special case and
  the `‹ Library` button both disappear.

**Windowing constraint.** The filmstrip and the grid must share one windowed
model, not hold two. `LibraryController` currently keys its `WINDOW` on grid
scroll position; the filmstrip's window follows the *selection* instead. This
is the genuinely hard part of the workstream — the window must move as
selection walks past its edge, and thumbnail requests must not thrash when it
does. Resolve this explicitly rather than by widening `WINDOW`.

**Done when.** Selecting an image in the grid enters develop with the
filmstrip showing neighbours; arrows walk the filmstrip and load images;
`G`/`D` toggles modes with selection preserved in both directions; a
17k-image library still holds a bounded number of live cells and does not
re-fetch thumbnails on every keystroke.

---

## Sequencing

```
A (primitives) ──┬── C (sections)
                 ├── B (canvas)      ── E (chrome)
                 └── D (grid)        ──┐
                                       ├── F (filmstrip)
                                       ┘
```

A, B, D, and E are independent of each other once A lands; C depends on A;
F depends on A and D. B and E both touch `app.slint`, so they should not run
concurrently.

## Verification, all workstreams

- `cargo build` clean, no new Slint warnings — in particular no binding-loop
  warnings, which `app.slint` already comments on at length and which can
  panic at runtime.
- The app launches and reaches the grid.
- No workstream may break FR-DEV-3a: adding a pipeline operation must still
  surface in the panel with no UI edit.