Give the drawer toggle a strict rung above the drawer again #188

Merged
rob merged 1 commit from fix/task-248-drawer-toggle-above-drawer into main 2026-08-17 08:06:53 +00:00
Owner

Fixes task 248. PR #184 (ADR-0156) put .drawer-toggle and .drawer on the same --z-header rung. Equal z-index hands paint order to DOM order, and .drawer renders after the toggle in Drawer.razor, so the open drawer painted over its own button and the menu could not be closed on desktop.

.drawer-toggle now sits at calc(var(--z-header) + 1), strictly above .drawer's var(--z-header) — mirroring what calc(var(--z-drawer) + 1) did before ADR-0156, at the new rung. Stays well below --z-overlay (1276 vs 1300), so a modal dialog still covers it.

ZIndexLadderTests gains a direct toggle-vs-drawer comparison (ShouldBeGreaterThan, not OrEqualTo — the state that shipped was equality, not merely an inversion) and a check that the toggle stays below --z-overlay; watched red against unfixed CSS (1275 vs 1275) before the fix went back in. E2E gains a behavioural test that clicks the real toggle through Playwright's actionability checks and asserts the drawer reports closed, so a future Drawer.razor reorder that reintroduces this by DOM order rather than by the token would still be caught.

ADR-0162 partially supersedes ADR-0156's "one rung for both" decision; ADR-0156 gets the metadata-only Partially superseded by update.

Fixes task 248. PR #184 (ADR-0156) put `.drawer-toggle` and `.drawer` on the same `--z-header` rung. Equal `z-index` hands paint order to DOM order, and `.drawer` renders after the toggle in `Drawer.razor`, so the open drawer painted over its own button and the menu could not be closed on desktop. `.drawer-toggle` now sits at `calc(var(--z-header) + 1)`, strictly above `.drawer`'s `var(--z-header)` — mirroring what `calc(var(--z-drawer) + 1)` did before ADR-0156, at the new rung. Stays well below `--z-overlay` (1276 vs 1300), so a modal dialog still covers it. `ZIndexLadderTests` gains a direct toggle-vs-drawer comparison (`ShouldBeGreaterThan`, not `OrEqualTo` — the state that shipped was equality, not merely an inversion) and a check that the toggle stays below `--z-overlay`; watched red against unfixed CSS (1275 vs 1275) before the fix went back in. E2E gains a behavioural test that clicks the real toggle through Playwright's actionability checks and asserts the drawer reports closed, so a future `Drawer.razor` reorder that reintroduces this by DOM order rather than by the token would still be caught. ADR-0162 partially supersedes ADR-0156's "one rung for both" decision; ADR-0156 gets the metadata-only `Partially superseded by` update.
Give the drawer toggle a strict rung above the drawer again
All checks were successful
CI / build (pull_request) Successful in 3m21s
CI / container-images (pull_request) Successful in 3s
CI / e2e (pull_request) Successful in 3m29s
7f8b2a0893
rob left a comment

Verdict: mergeable

Checked against scepticism, not just the diff summary:

  • Toggle 1276 vs drawer 1275 vs overlay 1300 — all three properties from task 244 hold. .drawer untouched, still above --z-panel; toggle now strictly above .drawer; both comfortably below --z-overlay. OverlayBackdrop's BelowZIndexToken="--z-header" in Drawer.razor is untouched (that file isn't even in the diff), so the scrim placement from #184 survives unchanged.
  • DrawerToggle_And_Drawer_DeclaredZIndex_RanksTheToggleAboveTheDrawer genuinely uses ShouldBeGreaterThan, not OrEqualTo — confirmed by reading the assertion, not the PR description. Against unfixed CSS (both --z-header) this fails on equality as claimed.
  • The E2E test is real, not decorative: Playwright's default context viewport (1280×720, unchanged by this fixture) sits above the 64rem/1024px breakpoint, so the drawer opens by default as claimed, and ClickAsync's actionability check does hit-test at the click point — with the drawer's opaque <aside> painted over the button (equal z-index, later in DOM), the click would time out rather than silently succeed. This is a legitimate regression catch, not a synthetic dispatch.
  • Combined with the static test, this covers both dimensions the ticket asked for — the numeric ordering and the DOM-order/paint-order case a pure number comparison can't see.
  • ADR-0107 conditions: only the status-metadata block of ADR-0156 was touched (Status, new Partially superseded by field) — its Context/Decision/Consequences are untouched. docs/adr/README.md's index row is the one other permitted edit. Field format matches existing precedent exactly. ADR-0162 is new and self-contained; no frozen body edited anywhere.
  • No CI-derived test-count or job-log claims appear anywhere on this PR to double-check.
  • No overlap with the parallel task 245 work (MapPlacesState, local-storage) — this PR touches only Drawer.razor.css, theme.css, the two test files and the ADRs.

Nothing to change.

Verdict: mergeable Checked against scepticism, not just the diff summary: - Toggle 1276 vs drawer 1275 vs overlay 1300 — all three properties from task 244 hold. `.drawer` untouched, still above `--z-panel`; toggle now strictly above `.drawer`; both comfortably below `--z-overlay`. `OverlayBackdrop`'s `BelowZIndexToken="--z-header"` in `Drawer.razor` is untouched (that file isn't even in the diff), so the scrim placement from #184 survives unchanged. - `DrawerToggle_And_Drawer_DeclaredZIndex_RanksTheToggleAboveTheDrawer` genuinely uses `ShouldBeGreaterThan`, not `OrEqualTo` — confirmed by reading the assertion, not the PR description. Against unfixed CSS (both `--z-header`) this fails on equality as claimed. - The E2E test is real, not decorative: Playwright's default context viewport (1280×720, unchanged by this fixture) sits above the 64rem/1024px breakpoint, so the drawer opens by default as claimed, and `ClickAsync`'s actionability check does hit-test at the click point — with the drawer's opaque `<aside>` painted over the button (equal z-index, later in DOM), the click would time out rather than silently succeed. This is a legitimate regression catch, not a synthetic dispatch. - Combined with the static test, this covers both dimensions the ticket asked for — the numeric ordering and the DOM-order/paint-order case a pure number comparison can't see. - ADR-0107 conditions: only the status-metadata block of ADR-0156 was touched (`Status`, new `Partially superseded by` field) — its Context/Decision/Consequences are untouched. `docs/adr/README.md`'s index row is the one other permitted edit. Field format matches existing precedent exactly. ADR-0162 is new and self-contained; no frozen body edited anywhere. - No CI-derived test-count or job-log claims appear anywhere on this PR to double-check. - No overlap with the parallel task 245 work (`MapPlacesState`, local-storage) — this PR touches only `Drawer.razor.css`, `theme.css`, the two test files and the ADRs. Nothing to change.
rob merged commit c89d677acf into main 2026-08-17 08:06:53 +00:00
rob deleted branch fix/task-248-drawer-toggle-above-drawer 2026-08-17 08:06:53 +00:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
rob/PlaceMark!188
No description provided.