From 25ec7cf58d81de7948b88e9b07cfc90ba5ecc73d Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 27 Apr 2026 09:18:09 +0000 Subject: [PATCH] fix(web): busy gate + toast on mark-read mark_read_click swallow per-peer errors. No busy guard. Add `mark_busy` RwSignal, disable+aria-busy button while in flight, collect Err count, push error toast via ToastStack. Browser test pin busy contract. Closes #345 --- crates/web/src/components/sync_queue_copy.rs | 24 +++++++++ crates/web/src/components/sync_queue_view.rs | 45 ++++++++++++++-- crates/web/tests/browser.rs | 56 ++++++++++++++++++++ 3 files changed, 122 insertions(+), 3 deletions(-) diff --git a/crates/web/src/components/sync_queue_copy.rs b/crates/web/src/components/sync_queue_copy.rs index d7ff745d..8c95a452 100644 --- a/crates/web/src/components/sync_queue_copy.rs +++ b/crates/web/src/components/sync_queue_copy.rs @@ -123,6 +123,24 @@ pub const ACTION_RETRY_BUSY: &str = "retrying…"; /// `action_mark_read` — inbound-tab footer action. pub const ACTION_MARK_READ: &str = "mark as read locally"; +/// `action_mark_read_busy` — rendered while +/// `client.mark_queue_read()` is in flight across the inbound peer set. +/// The spec pins the idle label; the busy label is an accessibility +/// refinement so the button is not mute while waiting and a parallel +/// to `ACTION_RETRY_BUSY`. +pub const ACTION_MARK_READ_BUSY: &str = "marking…"; + +/// `toast_mark_read_failed` — error-toast title rendered when one or +/// more peers' `mark_queue_read` calls fail. Plural-aware so the user +/// knows the partial-success shape. +pub fn toast_mark_read_failed(n: usize) -> String { + if n == 1 { + "failed to mark 1 peer as read".to_string() + } else { + format!("failed to mark {n} peers as read") + } +} + /// `screen_pill_waiting` — per-row pill on the outbound tab. pub const SCREEN_PILL_WAITING: &str = "waiting"; @@ -233,4 +251,10 @@ mod tests { // Locks the spec-driven 60 s offline gate for the toast + banner. assert_eq!(RECONNECT_GATE_TICKS, 60); } + + #[test] + fn toast_mark_read_failed_pluralises() { + assert_eq!(toast_mark_read_failed(1), "failed to mark 1 peer as read"); + assert_eq!(toast_mark_read_failed(3), "failed to mark 3 peers as read"); + } } diff --git a/crates/web/src/components/sync_queue_view.rs b/crates/web/src/components/sync_queue_view.rs index 946f8b3f..70090c5f 100644 --- a/crates/web/src/components/sync_queue_view.rs +++ b/crates/web/src/components/sync_queue_view.rs @@ -14,7 +14,7 @@ use leptos::prelude::*; use crate::app::WebClientHandle; -use crate::components::{sync_queue_copy, RelaySignalButton}; +use crate::components::{sync_queue_copy, RelaySignalButton, Toast, ToastStack}; use crate::icons; use crate::state::AppState; @@ -38,6 +38,9 @@ pub fn SyncQueueView() -> impl IntoView { let tab = RwSignal::new(Tab::Outbound); let busy = RwSignal::new(false); + // Independent busy gate for the inbound `mark as read locally` + // action so it cannot collide with `retry now` busy state. + let mark_busy = RwSignal::new(false); let status_label = move || { let v = queue_view.get(); @@ -80,15 +83,45 @@ pub fn SyncQueueView() -> impl IntoView { }; let mark_read_click = move |_| { + // Busy guard: ignore re-entrant clicks while a prior batch is + // still in flight. Mirrors `retry_click` so the button cannot + // be spammed into stacked async loops. + if mark_busy.get() { + return; + } + mark_busy.set(true); let Some(h) = use_context::() else { + // No handle in context — used by the browser-test harness. + // Flip busy straight back; real deployments always have a + // handle. + mark_busy.set(false); return; }; let view = queue_view.get(); let peers: Vec<_> = view.inbound_per_peer.keys().copied().collect(); + // Pull the toast stack lazily — `ToastStack` is optional so + // headless component tests that don't `provide_toast_stack()` + // still drive this handler without panicking. Production trees + // always provide one via `provide_notifier()`. + let toasts = use_context::(); wasm_bindgen_futures::spawn_local(async move { + // Collect per-peer errors so a single flaky link doesn't + // mask the success of the rest, and so we can surface the + // partial-failure shape to the user. + let mut failed = 0usize; for peer in peers { - let _ = h.mark_queue_read(peer).await; + if h.mark_queue_read(peer).await.is_err() { + failed = failed.saturating_add(1); + } } + if failed > 0 { + if let Some(stack) = toasts { + stack.push(Toast::err(sync_queue_copy::toast_mark_read_failed(failed)).build()); + } + } + // Reset busy in every path — the button must never get + // stuck in the disabled state, even if all peers failed. + mark_busy.set(false); }); }; @@ -250,9 +283,15 @@ pub fn SyncQueueView() -> impl IntoView { diff --git a/crates/web/tests/browser.rs b/crates/web/tests/browser.rs index 27d95892..29b49a6b 100644 --- a/crates/web/tests/browser.rs +++ b/crates/web/tests/browser.rs @@ -11023,6 +11023,62 @@ mod phase_2b_sync_queue { ); } + #[wasm_bindgen_test] + async fn sync_queue_view_mark_as_read_button_has_busy_attrs() { + // Issue #345: the mark-as-read button must carry an explicit + // busy gate (aria-busy + a `disabled` attribute path) so it + // cannot be spam-clicked while a per-peer batch is in flight. + // Without a `WebClientHandle` in context the click handler + // exits immediately, but the structural attributes guarantee + // the busy contract is wired even before the runtime handle + // arrives. + let container = mount_test_with_shell(TestShell::Desktop, move || { + let InitialSignals { + app_state, + write, + trust_store: _, + } = create_signals(); + provide_context(app_state); + provide_context(write); + view! { } + }); + tick().await; + + let tabs = query_all(&container, "[role='tab']"); + let inbound_tab = tabs + .iter() + .find(|t| text(t) == "inbound") + .expect("inbound tab must exist"); + simulate_click(inbound_tab); + tick().await; + + let btn = query(&container, ".sync-queue-view__mark-read") + .expect("mark-as-read button must render on inbound tab"); + assert_eq!( + btn.get_attribute("aria-busy").as_deref(), + Some("false"), + "mark-as-read must expose aria-busy so AT clients see the in-flight gate" + ); + // Idle copy comes from the spec; busy copy is the + // accessibility refinement parallel to ACTION_RETRY_BUSY. + assert_eq!( + text(&btn).trim(), + sync_queue_copy::ACTION_MARK_READ, + "idle label must be the spec copy" + ); + + // Smoke-click to ensure the handler runs without panicking + // when no `WebClientHandle` is provided (the test harness + // path) — exercises the early-return reset of `mark_busy`. + simulate_click(&btn); + tick().await; + assert_eq!( + btn.get_attribute("aria-busy").as_deref(), + Some("false"), + "mark-as-read must drop back to aria-busy=false once the early-return path runs" + ); + } + #[wasm_bindgen_test] async fn sync_queue_view_no_delete_action_anywhere() { // Spec is explicit: the queue is authoritative — no destructive