diff --git a/README.md b/README.md index e411c6f..0ae5382 100644 --- a/README.md +++ b/README.md @@ -98,6 +98,12 @@ ramp monotonicity) that guard the brand tokens; see colour system, its relationship to the UI design spec, and every deliberate deviation from it. +Layout follows [`apps/studio/docs/ui-layout.md`](apps/studio/docs/ui-layout.md): +which controller owns the arrangement, the fixed effect set every view change +must apply, each view's pane priority and degradation order, when a numeric +geometry argument is legitimate, and which of these rules are not yet +mechanically enforced. + `ui_check.py` includes a `record channel` section as well; see [`apps/studio/docs/record-channel.md`](apps/studio/docs/record-channel.md) for why recording and calibration sampling no longer hang off the UI refresh timer, diff --git a/apps/studio/docs/ui-layout.md b/apps/studio/docs/ui-layout.md new file mode 100644 index 0000000..63ee908 --- /dev/null +++ b/apps/studio/docs/ui-layout.md @@ -0,0 +1,128 @@ +# Studio UI layout contract + +Rules for the Studio window's layout, and for the tests that prove it. The colour equivalent is +[`ui-palette.md`](ui-palette.md); this file is the layout half. + +Scope: `apps/studio`. The same rules apply to `apps/autocal` where it has panes of its own. + +## 1. Ownership + +| Concern | Owner | +|---|---| +| Top-level panel arrangement, presets, splitter sizes, which panes are visible | `view_layout.py` (`ViewLayoutController`) | +| A panel's internal layout, its own minimums, how it degrades when narrow | that panel's module | +| Colour and appearance | theme tokens + `build_qss` (see `ui-palette.md`) | +| Shared spacing and minimum sizes | the token module named in §4 | + +A panel MUST NOT set the size of another panel, and MUST NOT reach into the splitter. If a panel +needs more room, it raises its own minimum and lets the controller allocate. + +**One owner per region.** A region's geometry is decided in exactly one place. The `app_window.py` +extractions (`ViewLayoutController`, `DisplayPrefsController`) are how this is achieved; new layout +behaviour belongs in a controller, not in `app_window.py`. + +## 2. View transitions + +The three top-level views are arrangements of the *same* window. Switching between them is a single +operation with a fixed set of effects. + +**Every path that changes the view MUST go through `apply_preset()`.** It is the only place that +applies the complete set, and `apply_preset` deliberately performs all of: + +1. sets `view_preset` and the persisted preference; +2. sets `show_cal` from the preset; +3. sets the rail's source view (`replay` vs `live`); +4. sets the run-button role (`playback` vs `acquire`); +5. applies panel visibility; +6. applies the preset's splitter sizes; +7. switches the chart/replay source and clears replay-only state when leaving replay; +8. updates the brand bar, view checks and replay actions. + +**MUST NOT** write `_view_preset`, the `view_preset` preference, or the splitter sizes directly. +Doing so produces a state that looks correct in the nav and wrong everywhere else — stale chart +source, wrong run role, sizes from the previous view. + +### The transition that broke + +`ViewLayoutController.on_cal_panel_toggled(False)` set `_view_preset = "live"` and persisted it +without calling `apply_preset("live")`. The sequence **replay → calibration → calibration off** +therefore ended with `view_preset == "live"` while the rail still reported the replay source and no +live splitter sizes had been requested. The live branch (`on_cal_panel_toggled(True)`) did call +`apply_preset`, which is what made the asymmetry easy to miss. + +This transition MUST be covered by a regression test that asserts the whole effect set of §2, in the +controller and again through a real `StudioMainWindow`. Asserting only `view_preset`, `show_cal` and +the brand bar — as the existing toggle test does — is what let it through. + +## 3. Per-view arrangement + +Each view is described by what it is *for*, so priority disputes have a reference. + +| View | Primary pane (gets width first) | Companion panes | Degradation as width shrinks, in order | +|---|---|---|---| +| `live` | measurement panes (2D/3D) | charts, settings rail | charts narrow to their minimum, then settings rail collapses, then charts scroll | +| `replay` | charts (history browsing is the point) | 2D pane, rail | 2D pane narrows, then hides where the layout allows | +| `calibration` | `cal_panel` — its six-column table and action row need width | rest | the table scrolls horizontally rather than compressing columns below readability | + +The 3D pane is a peer of the 2D pane, never a replacement for it. Where both are visible they share +the measurement region; where only one is, it takes the region. + +**Squeeze is a designed state, not an accident.** For each pane, name what happens at minimum width. +"Everything gets smaller" is not a degradation order. + +## 4. Numeric geometry + +Not every numeric argument is a defect; blanket replacement of numbers with names is churn. The rule +distinguishes by *reason*: + +| Kind | Rule | +|---|---| +| **Structural constants** — 1 px separator, 1 px rule, a colour-bar's height, a native handle that must stay at zero size | Keep, with a named constant and a comment stating why the number is the value | +| **Shared spacing** — margins, gaps, padding shared by more than one panel | MUST come from the shared token module | +| **Content-driven width** — a field, a spin box, a label that holds text | MUST derive from `sizeHint()`, `fontMetrics()`, or a token for its role. A literal that must hold the longest locale string MUST be justified in a comment | +| **A flexible panel's own minimum** | MUST be a named constant with the reason recorded | + +The token module (shared spacing and minimum sizes) does not exist yet; creating it is a follow-up. +Until it does, new shared spacing MUST NOT be introduced as a literal in more than one place. + +**Text must fit.** Any control whose text can change with locale MUST hold the longest supported +string without clipping. + +## 5. Enforcement gaps to close + +These are stated because the contract is not currently enforced where it claims to be. + +1. **The palette prohibition is not mechanically enforced.** `ui-palette.md` states that hex + literals are not allowed in widget code, but `ui_check.audit_palette()` reads the theme JSON and + checks values and contrast only — it never scans source. Hex literals exist in widget modules + today. Either add the source scan or correct the claim in `ui-palette.md`. +2. **The containment check is position-blind.** `test_ui_smoke.test_children_stay_inside_their_parent` + compares `child.width()/height()` against the parent's, and never looks at `x`/`y`. Since + `QWidget.geometry()` is parent-relative, a child shifted outside its parent — or overlapping a + sibling — passes as long as it is no larger. It MUST compare the full rectangle. +3. **Rendered 3D pixels are tested but conditionally.** `test_scene3d_gl_smoke.py` asserts on pixels + through `grabFramebuffer`, and skips without a real GL context. That is the right design; the gap + is that the skip is invisible in an ordinary run. Where the GL gate is required, a skip MUST be + reported as a failure of the run, not counted as a pass. + +## 6. Tests + +The existing suites stay: state assertions are cheap and catch real regressions. What is added is +assertions about the **visible result**. + +| Tier | What it asserts | Where | +|---|---|---| +| **Transition** | the full effect set of §2, for every path that changes the view, including the breaking sequence | `test_view_layout.py` (controller) and a window-level test through `StudioMainWindow` | +| **Geometry** | full child rectangles inside parents, no sibling overlap among laid-out peers, required text fits | extend `test_ui_smoke.py` | +| **Matrix** | the above under combinations that historically broke: minimum window size, longest locale, both themes, sensor type, pane state | parametrised in `test_ui_smoke.py` | +| **Rendered** | a few pinned 2D and toolbar snapshots for human review | `ui_snapshot.py` output, reviewed not asserted | +| **GL pixels** | rendered 3D, in the real-context suite only | `test_scene3d_gl_smoke.py` | + +**On waiting:** `processEvents()` dispatches pending paint events, so it is sufficient to let a +layout settle. It does not assert what was painted, and it does not guarantee that later +asynchronous work has finished. Tests that depend on completion MUST wait for the signal or +condition, not for a fixed number of `processEvents()` calls. `pytest-qt` makes that easier and is +not a prerequisite. + +**Golden images, if adopted:** pin the window size, scale factor, font and theme, or they will flake +and be disabled. A flaky render gate is worse than none.