# Phase 10 — UI Review

**Audited:** 2026-03-22
**Baseline:** Abstract 6-pillar standards (no UI-SPEC.md exists)
**Screenshots:** Not captured (no dev server detected; Slint desktop app, not web)

---

## Pillar Scores

| Pillar | Score | Key Finding |
|--------|-------|-------------|
| 1. Copywriting | 3/4 | Refresh error copy is specific and useful; "Save"/"Cancel" on note editor are generic but contextually unambiguous |
| 2. Visuals | 3/4 | Amber error state on three-dots button is semantically correct; no tooltip on icon-only controls |
| 3. Color | 3/4 | Hardcoded hex values throughout but system is internally consistent dark theme |
| 4. Typography | 3/4 | 6 distinct px sizes in card.slint — slightly over the 4-size guideline; two weights is correct |
| 5. Spacing | 2/4 | Absolute pixel positioning (x:/y:) dominates card layout, creating fragile fixed geometry |
| 6. Experience Design | 2/4 | NoopClient refresh overrides missing — error UI paths unreachable during development; refresh-all has no apply_filters call on failure path |

**Overall: 16/24**

---

## Top 3 Priority Fixes

1. **NoopClient missing `refresh_recipient` and `refresh_all` Err overrides** — The amber three-dots error state, "Refresh failed - retry" menu text, and failure toast are all unreachable when running with seed data; developers cannot verify these UI paths without the overrides — Add `fn refresh_recipient` and `fn refresh_all` to the `NoopClient` impl block at `crates/app/src/main.rs:2413` returning `Err("offline: noop client".to_string())`

2. **Refresh-all failure path does not call `apply_filters`** — When refresh-all fails (line 1963-1967), the toast is shown but `apply_filters` is not called before or after; if any card was briefly in-flight state (future: async), the display would not update — Call `apply_filters(&w, &cards.borrow(), &rt.borrow())` at the end of the failure branch alongside `w.set_refresh_all_label("Refresh all".into())` at line 1970

3. **Absolute pixel card layout prevents responsive reflow** — `card.slint` positions every child element via explicit `x:` and `y:` pixel values (e.g., `x: 48px`, `y: 40px`, `y: 76px`); the card height is fixed at 196px in the grid — if any content (long recipient name, second contact line) overflows, there is no reflow; use `VerticalLayout` with `spacing` for the card body sections to make layout self-healing

---

## Detailed Findings

### Pillar 1: Copywriting (3/4)

**Passing:**
- `"Refresh failed - retry"` (card.slint:642) — specific, action-oriented, communicates both the state and the remedy
- `"Refreshing..."` (main.rs:1930, 1992) — clear in-flight label for both global and per-card contexts
- `"Refresh all"` / `"Refresh failed"` (main.rs:1965, 1970) — sensible fallback and restore labels
- `"1 edit pending"` / `"{N} edits pending"` (dashboard.slint:276) — correct singular/plural handling
- Archive menu copy `"Archive"` / `"Archive Now"` / `"Unarchive"` (card.slint:667) — contextually accurate three-state labels

**Minor issues:**
- `"Save"` and `"Cancel"` on the note editor (card.slint:375, 401) are generic but unambiguous in context — acceptable
- Refresh failure toast message falls back to a bare `"Refresh failed"` string (main.rs:1965) with no guidance on next steps; consider `"Refresh failed — click Refresh to retry"` to mirror the per-card "Refresh failed - retry" pattern

**No generic "OK", "Submit", "Click Here", or "No data" strings found in phase-modified files.**

---

### Pillar 2: Visuals (3/4)

**Passing:**
- Three-dots button background switches to amber (`#4a2810` / `#5a3818`) on `refresh-error` — semantic color signal is correct and differentiates error state from idle state (card.slint:453)
- Refresh-all button dims to `#3a5299` when disabled vs. active `#4a7cff` — clear disabled affordance (dashboard.slint:343)
- Archive cards dim to `opacity: 0.5` (card.slint:81) — appropriate visual de-emphasis
- F5 key handler documented in code and routed correctly (dashboard.slint:241)

**Issues:**
- The three-dots button (`"\u{2026}"`) is icon-only with no tooltip or accessible label. Slint does not have `aria-label`, but a `tooltip` property or adjacent visible label would help discoverability. When the button turns amber, there is no accompanying text hint — users must hover to reveal the context menu to discover the error.
- The refresh-all button shows "Refreshing..." during in-flight state but there is no animated or pulsing affordance to indicate activity is ongoing — static label change only.
- Info icon `"\u{24D8}"` on cards (card.slint:145) is icon-only with no tooltip — undiscoverable affordance for summary panel.

---

### Pillar 3: Color (3/4)

**System analysis:**
The entire UI uses a consistent dark theme with a narrow, internally coherent palette:
- Background: `#1a1e2a` (window), `#242838` (card), `#12151f` (card grid), `#2d3348` (panels)
- Accent blue: `#4a7cff` (primary actions), `#7ea8ff` (secondary/status)
- Text: `#e0e4ef` (primary), `#8a92a8` (secondary), `#6b7590` (tertiary)
- Warning: `#f0a030` (amber, used for stale/missing/unassigned)
- Error: `#ef5350` (red, remove button)
- Success/add: `#4caf50` (green, add button)

**New in Phase 10:**
- Amber error state `#4a2810` / `#5a3818` for three-dots button (card.slint:453) — consistent with the existing `#f0a030` amber family; slightly dark-shifted for background use rather than text

**Issues:**
- 70+ hardcoded hex values in card.slint alone — the system is consistent but has no CSS variables or Slint global color tokens; a palette change requires grep-and-replace across all files
- `#3a5299` (disabled refresh-all) is a one-off — not shared with any other disabled element color in the codebase; minor inconsistency

**No accent overuse issues; color application is thematically intentional.**

---

### Pillar 4: Typography (3/4)

**Font sizes in card.slint (phase-relevant file):**
- 10px — not present in card.slint (present in dashboard.slint settings button)
- 11px — badge labels, save/cancel buttons, missing-label, stale badge, close button
- 12px — body copy (status date, item label, note text, menu items, summary body)
- 13px — recipient name, item square initials, summary section headers
- 16px — three-dots ellipsis glyph, back arrow
- 18px — add button "+" glyph

That is 6 distinct sizes. Abstract standard flags anything over 4.

**Font weights:**
- `font-weight: 700` — avatar initial, item square initials (card.slint:108, 235)
- `font-weight: 600` — summary section headers (card.slint:733, 737, 741, 745)
- Implicit default weight for all body text

Two distinct explicit weights is within the 2-weight guideline. The additional implicit default (normal) is a third de-facto weight but is unavoidable in Slint.

**Phase 10 additions did not introduce new font sizes or weights.** The 6-size issue is pre-existing.

---

### Pillar 5: Spacing (2/4)

**Dominant pattern: absolute pixel positioning**

All child elements in `RecipientCard` are positioned via explicit `x:` / `y:` pixel coordinates:
- Avatar at `x: 14px, y: 10px` (card.slint:99)
- Recipient name at `x: 48px, y: 10px` (card.slint:118)
- Contact secondary at `x: 48px, y: 24px` (card.slint:137)
- Status pill at `x: 14px, y: 40px` (card.slint:155)
- Item label at `x: 14px, y: 76px` (card.slint:184)
- Item squares area at `x: 14px, y: 96px` (card.slint:196)
- Note area at `x: 14px, y: 148px` (card.slint:325)
- Missing label at `x: 14px, y: 170px` (card.slint:418)

This creates a rigid 196px fixed-height card with hardwired vertical rhythm. If a second contact line is present (`contact-secondary != ""`), it renders at y:24 but the rest of the layout does not shift — the line can overlap with the status pill row.

**Phase 10 additions:**
- The three-dots button position (`x: parent.width - 30px, y: 10px`) is correctly anchored to parent width — this is the only dynamic positioning in the card.
- No new spacing issues introduced in Phase 10; the absolute-layout pattern is pre-existing.

**Summary panel popup uses VerticalLayout with padding/spacing** — this is the correct pattern and should be applied to the card body itself.

---

### Pillar 6: Experience Design (2/4)

**What Phase 10 introduced:**

1. **In-flight state for refresh-all:** Button disabled + label changes to "Refreshing..." — correctly prevents double-submit.
2. **In-flight state for per-card refresh:** Menu item disabled + label changes to "Refreshing..." — correct.
3. **Refresh error state:** `refresh_error: true` drives amber three-dots button + "Refresh failed - retry" menu text — correct signaling.
4. **Archive persistence:** `ArchiveStore::load_or_create` and `persist()` on every mutation — correct; manual archive/TBA state survives restarts.
5. **Manual archive preservation through recompute:** New gate in `compute_archive_state` preserves manually-set non-Active states — closes a real data integrity bug.
6. **DashboardRuntime injection:** `with_archive_store` constructor keeps test isolation intact.

**Defects found:**

**DEFECT 1 — NoopClient refresh overrides absent (main.rs:2411-2471):**
The 10-04 SUMMARY states: "Added `refresh_recipient` and `refresh_all` overrides to `NoopClient` returning `Err('offline: noop client')`, making amber indicator, 'Refresh failed - retry' text, and failure toast all reachable during seed-data development testing."

However, the `NoopClient` impl block at lines 2413-2471 does not contain these methods. The trait defaults return `Ok(())`, meaning all refresh calls succeed silently and the error UI is unreachable with seed data. The overrides that DO exist at lines 2492-2496 are inside `FakeClient` in the `#[cfg(test)]` module — not in `NoopClient`.

Impact: Developers running the app with seed data cannot observe or verify the amber error state, "Refresh failed - retry" copy, or failure toast.

Fix: Add to `NoopClient` (after the `add_item` method, before the closing `}`):
```rust
fn refresh_recipient(&self, _recipient_id: &str) -> Result<(), String> {
    Err("offline: noop client".to_string())
}

fn refresh_all(&self) -> Result<(), String> {
    Err("offline: noop client".to_string())
}
```

**DEFECT 2 — Refresh-all failure path skips `apply_filters` (main.rs:1963-1967):**
On the failure branch, the code shows the toast and then falls through to the button state restore. The `apply_filters` call at line 1962 only runs on the success path. On failure, if any future async work marks cards as in-flight before this point, those cards would remain in stale display state.

Currently this is benign (synchronous execution, no in-flight mark before dispatch), but it is an asymmetry that will become a real bug if the refresh is ever made async. The success and failure paths should be symmetrical.

Fix: Add `apply_filters(&w, &cards.borrow(), &rt.borrow());` after `show_toast(msg, false);` at line 1966.

**Passing experience patterns:**
- Destructive remove action has a confirm step (`remove-confirming` state in card.slint:47, confirm-remove-clicked callback)
- Archive undo is supported via toast with Undo button (dashboard.slint:638-677)
- Empty state is handled: seed cards are shown until real data arrives
- Settings connection status indicator gives live feedback (dashboard.slint:313-333)
- Pending edit count indicator warns of unsynced changes (dashboard.slint:273-279)

---

## Files Audited

- `crates/app/ui/card.slint` — RecipientCard component with refresh-error state, three-dots amber background, refresh menu copy
- `crates/app/ui/dashboard.slint` — DashboardWindow with refresh-all button, F5 handler, card grid callback wiring, CardData struct
- `crates/app/src/main.rs` — on_refresh_all_clicked, on_card_refresh callbacks, NoopClient, archive init wiring
- `crates/app/src/dashboard/archive.rs` — ArchiveStore with load_or_create, persist, serde derives, compute_archive_state manual-preservation gate
- `crates/app/src/dashboard/mod.rs` — DashboardRuntime with_archive_store constructor (referenced via grep)
- `.planning/phases/10-refresh-auto-archive-wiring/10-01-PLAN.md`
- `.planning/phases/10-refresh-auto-archive-wiring/10-02-PLAN.md`
- `.planning/phases/10-refresh-auto-archive-wiring/10-03-PLAN.md`
- `.planning/phases/10-refresh-auto-archive-wiring/10-04-PLAN.md`
- `.planning/phases/10-refresh-auto-archive-wiring/10-01-SUMMARY.md`
- `.planning/phases/10-refresh-auto-archive-wiring/10-02-SUMMARY.md`
- `.planning/phases/10-refresh-auto-archive-wiring/10-03-SUMMARY.md`
- `.planning/phases/10-refresh-auto-archive-wiring/10-04-SUMMARY.md`