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)

`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.

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

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

**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: twelve collapsible bands

**Deliverable.** 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; the modified dot (Workstream C) shows which bands hold an edit
without expanding them.

This needs no new `WidgetKind`: it is Workstream C's sections applied to a
grouping the frontend derives from parameter ids. Depends on C.

### 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; bands collapse with
modified state visible while collapsed; Vibrance and Saturation are one row
each; and no hex colour appears in any descriptor.

---

## 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.

---

## 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.