From 7854dd31797f63fc2934c1652a84ec099892a989 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 29 Apr 2026 22:38:31 +0000 Subject: [PATCH 01/13] docs(plans): event-based-waits PR-3 implementation plan Ten tasks covering: - LifecycleState helper module (closed/opening/open/closing + reduced- motion check + advance fn). - Six per-component lifecycle wirings (grove_drawer canonical, then mobile_shell, confirm_dialog, bottom_sheet, tab_bar, action sheet). - Three browser tests for the lifecycle's documented failure modes (transitionend-on-driving-property, reduced-motion shortcut, unrelated-transitionend-ignored). - page.clock helper + longPressWithClock variant. Opt-in only; multi-peer specs stay clock-free per the spec's iroh-timer risk row. - README documenting the four-phase lifecycle vs. the categorical data-state usages on status_dot/grove_rail/peer_status_label. --- ...nt-based-waits-pr3-data-state-lifecycle.md | 779 ++++++++++++++++++ 1 file changed, 779 insertions(+) create mode 100644 docs/plans/2026-04-30-event-based-waits-pr3-data-state-lifecycle.md diff --git a/docs/plans/2026-04-30-event-based-waits-pr3-data-state-lifecycle.md b/docs/plans/2026-04-30-event-based-waits-pr3-data-state-lifecycle.md new file mode 100644 index 00000000..ce8c11d6 --- /dev/null +++ b/docs/plans/2026-04-30-event-based-waits-pr3-data-state-lifecycle.md @@ -0,0 +1,779 @@ +# Event-Based Waits PR-3 — `data-state` Lifecycle + `page.clock` Adoption + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Replace post-animation magic-number sleeps with a four-phase `data-state="closed|opening|open|closing"` lifecycle on six animated UI elements, and migrate `longPress`'s real 600 ms timer to `page.clock.runFor()` so touch tests don't pay wall-clock time per iteration. + +**Architecture:** Each animated component owns an `RwSignal<&'static str>` (or `LifecycleState` enum, see Task 1) reflecting `closed | opening | open | closing`. The signal is exposed via a `data-state` attribute on the root element. State advances are driven by `transitionend` filtered on the component's specific driving CSS property (`transform` for slide animations, `opacity` for fade animations). A reduced-motion shortcut snaps to the terminal state synchronously when `getComputedStyle(el).transitionDuration === '0s'`, so tests under `prefers-reduced-motion: reduce` don't hang. `page.clock.install()` is opt-in per spec; only the touch helpers (and any future debounce-sensitive specs) install it. Iroh's WASM transport runs on QUIC retransmit timers that are insensitive to `Date`/`setTimeout` patching, so installing the clock during touch tests does not destabilise gossip — confirmed by the PR-1 audit recorded in the design spec §`page.clock` for real durations. + +**Tech Stack:** Leptos 0.7 (signals + view!), `web_sys::TransitionEvent`, `web_sys::CssStyleDeclaration::transition_duration`, Playwright `page.clock` (since 1.45), wasm-bindgen-test for browser coverage. + +**Spec:** [`docs/specs/2026-04-27-event-based-waits-design.md`](../specs/2026-04-27-event-based-waits-design.md) §`data-state` attribute pattern + §`page.clock` for real durations + §"Implementation phasing" PR 3 entry. +**Predecessor:** PR-2 (#495, merged in `5cc9efb`). +**Tracking issue:** [#458](https://github.com/intendednull/willow/issues/458). + +--- + +## File Structure + +**Create:** +- `crates/web/src/components/lifecycle.rs` — shared `LifecycleState` enum + helper free fns. (Reasoning under Task 1.) +- `e2e/helpers/clock.ts` — `installPeerClock(page)` plus a per-test `usePageClock` fixture extension. Lives next to `helpers/touch.ts` so the touch migration is single-file from the test author's POV. + +**Modify:** +- `crates/web/src/components/grove_drawer.rs` — replace `data-open` with `data-state` lifecycle. +- `crates/web/src/components/mobile_shell.rs` — wire mobile drawer lifecycle. Currently uses an `RwSignal` with no `data-state`. +- `crates/web/src/components/confirm_dialog.rs` — add lifecycle (no current state attribute). +- `crates/web/src/components/bottom_sheet.rs` — replace `data-open` with `data-state`. +- `crates/web/src/components/tab_bar.rs` — replace `data-visible` with `data-state`. (NB: tab bar's hide/show is the "active tab indicator slide", per the spec.) +- `crates/web/src/components/message.rs` — verify the existing `data-state` at `:1265` and either replace its semantics or wire the same four-phase lifecycle to the `mobile-action-sheet-overlay` div. +- `crates/web/src/components/mod.rs` — `pub mod lifecycle;`. +- `crates/web/components.css` — for each component, ensure the open-class trigger plus reduced-motion override (`@media (prefers-reduced-motion: reduce) { ... transition: none }`) is present so the JS shortcut path is exercised in tests. +- `crates/web/tests/browser.rs` — append the `data-state` lifecycle test module covering the three failure modes (reduced-motion, mid-transition unmount, overlapping transitions). +- `e2e/helpers/touch.ts` — `longPress` swaps `page.waitForTimeout(durationMs)` for `await page.clock.runFor(durationMs)`. Same for the trailing 300 ms settle. +- `e2e/test-hooks.ts` — extend the `peer` fixture (or add a sibling `pageClock` fixture) so touch specs install the clock without per-spec boilerplate. +- `e2e/README.md` — document the `data-state` lifecycle convention (four phases vs. the categorical `data-state` on `status_dot`/`grove_rail`) and the `page.clock` opt-in pattern. + +**Untouched (legacy specs continue to import from the barrel):** +- `e2e/cross-browser-sync.spec.ts`, `e2e/join-links.spec.ts`, `e2e/multi-peer-mobile.spec.ts`, `e2e/permissions.spec.ts`, `e2e/worker-nodes.spec.ts`, `e2e/multi-peer-sync.spec.ts` (PR-2 pilot — already on `./test-hooks`). +- `e2e/mobile.spec.ts` and `e2e/mobile-actions.spec.ts` — these will benefit from `page.clock` once they migrate (file-by-file via #458). PR-3 only changes the helper. + +**Why a shared `lifecycle.rs` module instead of inlining?** The spec's open-question §"Should the `data-state` attribute pattern be lifted into a shared Leptos helper component" defers shared-helper-ness, but the *enum + advance fn + reduced-motion fn* are pure data with no Leptos signal dependency. Lifting just those three into a free-function helper (no component wrapper) avoids six near-identical 30-line blocks while honouring the "first apply, then refactor" guidance — there is nothing to refactor later because the duplication is already collapsed at the cheap, non-invasive level. The component-level wrapping (`RwSignal` + the `transitionend` closure on the root node) stays inline so each component can choose its own driving property without indirection. + +--- + +## Task 0: Preflight — verify PR-2 baseline still passes + +**Files:** none. + +PR-2 just landed (`5cc9efb`) — confirm the worktree starts from a green baseline before any Rust changes. + +- [ ] **Step 1: Confirm git state** + +```bash +git status +git log --oneline -5 +``` + +Expected: clean tree on `claude/event-based-waits-pr3`, head is the PR-2 merge commit (`5cc9efb`) or the new branch root. + +- [ ] **Step 2: Run the cheap subset of `just check-all`** + +```bash +just fmt +just clippy +``` + +Expected: zero errors, zero warnings. Skip `just test` and `just test-browser` here — they're slow and the next tasks will fail loudly long before a regression in those would. + +- [ ] **Step 3: No commit (read-only baseline).** Move to Task 1. + +--- + +## Task 1: Add the `lifecycle` helper module + +**Files:** +- Create: `crates/web/src/components/lifecycle.rs` +- Modify: `crates/web/src/components/mod.rs` + +This module owns the three pure-data primitives that every animated component reuses: the `LifecycleState` enum, the `advance(state)` transition fn, and the `is_zero_duration(element)` reduced-motion check. Components instantiate their own `RwSignal` and own their own `transitionend` listener, so this module has no Leptos dependency and is straightforwardly unit-testable. + +- [ ] **Step 1: Write the module** + +```rust +// crates/web/src/components/lifecycle.rs +// +// Four-phase lifecycle helpers for animated components. See +// docs/specs/2026-04-27-event-based-waits-design.md §`data-state` attribute +// pattern. Apply via `RwSignal` + a `transitionend` closure +// on the component's root element (filtered on the component's specific +// driving CSS property). +// +// `data-state` lifecycle is reserved for the four animated phases +// (closed/opening/open/closing). For categorical states (online/offline/away +// on `status_dot.rs`, idle/loading/connected on `grove_rail.rs`, +// presence labels in `peer_status_label.rs`) keep using `data-state` with +// custom strings — those usages are orthogonal and pre-date this module. + +use web_sys::{Element, HtmlElement}; + +/// Animated component's transition phase. +/// +/// `Opening` and `Closing` are set imperatively when the user triggers the +/// transition; `Open` and `Closed` are flipped by the component's +/// `transitionend` listener (or the reduced-motion shortcut). +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum LifecycleState { + Closed, + Opening, + Open, + Closing, +} + +impl LifecycleState { + /// String form for the `data-state` attribute. Keep in sync with e2e + /// tests asserting `toHaveAttribute('data-state', ...)`. + pub const fn as_str(self) -> &'static str { + match self { + Self::Closed => "closed", + Self::Opening => "opening", + Self::Open => "open", + Self::Closing => "closing", + } + } +} + +/// Advance the lifecycle on `transitionend`. +/// +/// `Opening` → `Open`, `Closing` → `Closed`. Other states are unchanged +/// (a stray `transitionend` while already terminal is a no-op, not an error). +pub const fn advance(state: LifecycleState) -> LifecycleState { + match state { + LifecycleState::Opening => LifecycleState::Open, + LifecycleState::Closing => LifecycleState::Closed, + terminal => terminal, + } +} + +/// Returns true if the element has no animation duration (reduced motion or +/// untransitioned). When this returns true, callers must snap to the +/// terminal lifecycle state synchronously without waiting for `transitionend` +/// — otherwise the test hangs because no event will fire. +/// +/// Reads `getComputedStyle(el).transition-duration`. Empty string and "0s" +/// both count as zero. Multi-property transitions (comma-separated values) +/// are conservatively treated as non-zero unless every component is "0s", +/// matching the spec's "if computed-zero, skip wait" semantics. +pub fn is_zero_duration(element: &Element) -> bool { + let Some(window) = web_sys::window() else { return false; }; + let Ok(Some(computed)) = window.get_computed_style(element) else { return false; }; + let Ok(duration) = computed.get_property_value("transition-duration") else { return false; }; + if duration.is_empty() { + return true; + } + duration + .split(',') + .all(|d| matches!(d.trim(), "" | "0s" | "0ms")) +} + +/// Convenience: convert an `&HtmlElement` to `&Element` for `is_zero_duration`. +pub fn is_zero_duration_html(html: &HtmlElement) -> bool { + is_zero_duration(html.as_ref()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn advance_opening_to_open() { + assert_eq!(advance(LifecycleState::Opening), LifecycleState::Open); + } + + #[test] + fn advance_closing_to_closed() { + assert_eq!(advance(LifecycleState::Closing), LifecycleState::Closed); + } + + #[test] + fn advance_terminal_states_idempotent() { + assert_eq!(advance(LifecycleState::Open), LifecycleState::Open); + assert_eq!(advance(LifecycleState::Closed), LifecycleState::Closed); + } + + #[test] + fn as_str_round_trip() { + for state in [ + LifecycleState::Closed, + LifecycleState::Opening, + LifecycleState::Open, + LifecycleState::Closing, + ] { + // No round-trip parser by design — `as_str` is for the DOM attribute. + assert!(!state.as_str().is_empty()); + } + } +} +``` + +- [ ] **Step 2: Wire the module into `mod.rs`** + +Open `crates/web/src/components/mod.rs` and add `pub mod lifecycle;` in alphabetical position (between `letters` and `member_list` if they're present; or wherever the pattern reads). + +- [ ] **Step 3: Verify it compiles + native tests pass** + +```bash +cargo check -p willow-web +cargo test -p willow-web --lib lifecycle::tests +``` + +Expected: zero errors, 4 tests pass. The `is_zero_duration` fn is gated on `web_sys::window()` so the unit tests skip it (it would only run under wasm-bindgen-test); coverage for that fn comes in Task 8. + +- [ ] **Step 4: WASM build sanity** + +```bash +just check-wasm +``` + +Expected: zero errors. `web_sys::Element` and `CssStyleDeclaration::get_property_value` are both stable and already used elsewhere in the crate. + +- [ ] **Step 5: Commit** + +```bash +git add crates/web/src/components/lifecycle.rs crates/web/src/components/mod.rs +git commit -m "feat(web): add LifecycleState enum + transition helpers + +Four-phase data-state lifecycle (closed/opening/open/closing) for +animated components. Pure-data primitives only — components own their +own RwSignal and transitionend closure. Reduced-motion shortcut reads +computed transition-duration; if zero, callers snap to terminal state +synchronously (otherwise the test hangs because no transitionend +fires under prefers-reduced-motion). + +Per docs/specs/2026-04-27-event-based-waits-design.md §data-state +attribute pattern. The shared-helper-component question stays open +per the spec's defer guidance — only the data primitives are lifted." +``` + +--- + +## Tasks 2–7: Per-component lifecycle wiring + +All six components follow the same 4-block pattern from Task 2 (the canonical reference below). For Tasks 3–7, **only the diff vs. Task 2** is documented — the Effect, on_transition_end closure, and view! changes are mechanically identical apart from the driving property and (for the action sheet) the source of truth signal. + +| Task | Component file | Driving CSS property | Source of truth | Existing attr being replaced | +|------|----------------|---------------------|-----------------|------------------------------| +| 2 | `grove_drawer.rs` | `transform` | `open: Signal` prop | `data-open` | +| 3 | `mobile_shell.rs` | `transform` | `drawer_open: RwSignal` | (none — adding) | +| 4 | `confirm_dialog.rs` | `opacity` | `visible: ReadSignal` prop | (none — adding) | +| 5 | `bottom_sheet.rs` | `opacity` | `open: Signal` prop | `data-open` | +| 6 | `tab_bar.rs` | `transform` | `visible: Signal` prop | `data-visible` | +| 7 | `message.rs` action sheet | `transform` | `show_sheet: Memo` | class-name toggling | + +Per-task structure: edit → `just check-wasm` → commit. No separate Step 2 for CSS audit unless the existing attribute (`data-open` / `data-visible`) is keyed by CSS selectors. + +## Task 2: Wire `grove_drawer.rs` lifecycle (canonical reference) + +**Files:** +- Modify: `crates/web/src/components/grove_drawer.rs` + +`grove_drawer.rs` is the simplest of the six (220 LOC, single signal). This is the reference implementation; subsequent components (`mobile_shell`, `confirm_dialog`, `bottom_sheet`, `tab_bar`, action sheet) follow the same pattern with their own driving property. + +Driving property: `transform` (the slide-in/out is a `translateX`). + +The current file uses `data-open=move || open.get()` at `:71`. Replace with `data-state=move || lifecycle.get().as_str()`. The `open: Signal` prop stays — internally we maintain a `RwSignal` mirrored from `open` via an Effect. + +- [ ] **Step 1: Edit the component** + +Find the existing `data-open=move || open.get()` attribute and replace its surrounding open/close logic with the lifecycle-driven version. The diff is: + +```rust +// At the top of the component body, after props are destructured: +use crate::components::lifecycle::{LifecycleState, advance, is_zero_duration}; +use leptos::ev::TransitionEvent; +use leptos::html::Aside; // or whatever element the drawer is + +let drawer_ref: NodeRef