add the Studio UI layout contract
Studio had a colour contract with a mechanical audit but no layout equivalent,
which is where the recurring visual defects live: window flashing, the label
under the 3D view, the platform-not-moving report, and the 2D/3D sync work all
passed a 3,408-assertion tool suite because that suite asserts widget state, not
the visible result.
apps/studio/docs/ui-layout.md states the contract: ViewLayoutController owns the
top-level arrangement and each panel owns its internal layout; every path that
changes the view must go through apply_preset(); each view's pane priority and
degradation order; and which numeric geometry arguments are legitimate (a 1px
separator is not the same defect as a magic width on a flexible panel).
It also records the one transition that is broken today.
on_cal_panel_toggled(False) sets view_preset = "live" directly instead of
calling apply_preset("live"), so the replay -> calibration -> calibration-off
sequence leaves the rail on the replay source, the run button in playback role,
and the previous view's splitter sizes. The live branch does call apply_preset,
which is what makes the asymmetry easy to miss.
Three claimed enforcement gaps are stated so the contract is not mistaken for
something already enforced: audit_palette reads theme JSON and never scans
source for the hex literals ui-palette.md forbids; the containment test compares
width and height without x/y, so a shifted or overlapping child passes; and the
GL pixel suite skips invisibly without a real context.
Verified: doc_lint default and --all both exit 0; the new document is registered
in the workspace inventory (63 collected, 63 inventoried, 0 missing) and linked
from README.md beside the palette reference. No source file changed.
This commit is contained in:
@@ -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,
|
||||
|
||||
@@ -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.
|
||||
Reference in New Issue
Block a user