From 4d5c236558a4f3193b96a5b4e378ffc2d5793c96 Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Mon, 27 Apr 2026 09:10:45 +0000
Subject: [PATCH] fix(web): a11y escape + initial focus on relay popover
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Popover advertises `aria-haspopup="dialog"` but had no Escape handler
and no focus pull. Global keybinding stack does not own this popover —
local listener required. Seed focus on settings link so keyboard users
land inside the dialog. Mark `aria-modal="false"` — popover does not
trap focus.

Tests: Escape closes, settings link focused on open, aria-modal="false".

Closes #352
---
 .../web/src/components/relay_signal_button.rs |  38 ++++++
 crates/web/tests/browser.rs                   | 122 ++++++++++++++++++
 2 files changed, 160 insertions(+)

diff --git a/crates/web/src/components/relay_signal_button.rs b/crates/web/src/components/relay_signal_button.rs
index d2bf4dec..cb8b8ae3 100644
--- a/crates/web/src/components/relay_signal_button.rs
+++ b/crates/web/src/components/relay_signal_button.rs
@@ -45,6 +45,40 @@ pub fn RelaySignalButton() -> impl IntoView {
 
     let open = RwSignal::new(false);
 
+    // Settings-link button receives focus when the popover opens, so
+    // keyboard users land inside the dialog instead of skipping past
+    // its contents. Same pattern used by `ConfirmDialog`.
+    let settings_link_ref = NodeRef::<leptos::html::Button>::new();
+
+    // Pull focus into the popover on each open transition. The global
+    // keybinding stack (`crate::keybindings`) does not know about this
+    // popover, so we own the initial-focus contract locally. The dialog
+    // is non-modal (`aria-modal="false"`) — we do not trap focus, just
+    // seed it; Tab continues to move through page chrome normally.
+    Effect::new(move |prev: Option<bool>| {
+        let is_open = open.get();
+        let was_open = prev.unwrap_or(false);
+        if is_open && !was_open {
+            request_animation_frame(move || {
+                if let Some(el) = settings_link_ref.get_untracked() {
+                    let _ = el.focus();
+                }
+            });
+        }
+        is_open
+    });
+
+    // Local Escape handler scoped to the popover. The global handler in
+    // `keybindings::install` only knows about the rail / palette / sheet
+    // stack; this popover is outside that stack, so without a local
+    // listener Escape would be a no-op while focus is inside the dialog.
+    let on_popover_keydown = move |ev: web_sys::KeyboardEvent| {
+        if ev.key() == "Escape" {
+            ev.stop_propagation();
+            open.set(false);
+        }
+    };
+
     let class_for = move || match relay.get() {
         RelayStatus::Reachable => "relay-signal-button relay-signal-button--ok",
         RelayStatus::Unreachable => "relay-signal-button relay-signal-button--warn",
@@ -94,6 +128,9 @@ pub fn RelaySignalButton() -> impl IntoView {
                     class="relay-popover"
                     role="dialog"
                     aria-label="relay status"
+                    aria-modal="false"
+                    tabindex="-1"
+                    on:keydown=on_popover_keydown
                 >
                     <header class="relay-popover__header">
                         {icons::icon_signal()}
@@ -106,6 +143,7 @@ pub fn RelaySignalButton() -> impl IntoView {
                     <button
                         class="relay-popover__settings-link"
                         type="button"
+                        node_ref=settings_link_ref
                         on:click=move |_| {
                             // Close the popover + open settings. The
                             // relay-picker tab lands with
diff --git a/crates/web/tests/browser.rs b/crates/web/tests/browser.rs
index 27d95892..efc73b87 100644
--- a/crates/web/tests/browser.rs
+++ b/crates/web/tests/browser.rs
@@ -11473,6 +11473,128 @@ mod phase_2b_sync_queue {
         );
     }
 
+    /// Issue #352 — Escape on the popover closes it. The popover is
+    /// outside the global close-stack in `keybindings::install`, so
+    /// without a local handler keyboard users could not dismiss it.
+    #[wasm_bindgen_test]
+    async fn relay_signal_button_escape_closes_popover() {
+        let container = mount_test(move || {
+            let InitialSignals {
+                app_state,
+                write,
+                trust_store: _,
+            } = create_signals();
+            write.queue.set_relay_status.set(RelayStatus::Reachable);
+            provide_context(app_state);
+            provide_context(write);
+            view! { <RelaySignalButton /> }
+        });
+        tick().await;
+
+        let btn = query(&container, ".relay-signal-button").expect("button must render");
+        simulate_click(&btn);
+        tick().await;
+
+        let popover = query(&container, ".relay-popover").expect("popover must be open");
+
+        let init = web_sys::KeyboardEventInit::new();
+        init.set_key("Escape");
+        let escape =
+            web_sys::KeyboardEvent::new_with_keyboard_event_init_dict("keydown", &init).unwrap();
+        popover
+            .dyn_ref::<web_sys::EventTarget>()
+            .unwrap()
+            .dispatch_event(&escape)
+            .unwrap();
+        tick().await;
+
+        assert!(
+            query(&container, ".relay-popover").is_none(),
+            "Escape keydown on the popover must close it"
+        );
+        assert_eq!(
+            btn.get_attribute("aria-expanded").as_deref(),
+            Some("false"),
+            "aria-expanded must flip back to false after Escape"
+        );
+    }
+
+    /// Issue #352 — opening the popover seeds focus on the
+    /// settings-link button so keyboard users land inside the dialog.
+    #[wasm_bindgen_test]
+    async fn relay_signal_button_focuses_settings_link_on_open() {
+        let container = mount_test(move || {
+            let InitialSignals {
+                app_state,
+                write,
+                trust_store: _,
+            } = create_signals();
+            write.queue.set_relay_status.set(RelayStatus::Reachable);
+            provide_context(app_state);
+            provide_context(write);
+            view! { <RelaySignalButton /> }
+        });
+        tick().await;
+
+        let btn = query(&container, ".relay-signal-button").expect("button must render");
+        simulate_click(&btn);
+        tick().await;
+
+        // Focus is queued via `request_animation_frame`, so wait one
+        // frame before asserting `document.activeElement`. A short
+        // timeout is more than long enough for rAF to fire in the
+        // headless test browser and matches the timing pattern used
+        // elsewhere in this file.
+        gloo_timers::future::TimeoutFuture::new(40).await;
+        tick().await;
+
+        let active = web_sys::window()
+            .unwrap()
+            .document()
+            .unwrap()
+            .active_element()
+            .expect("active element after open");
+        assert!(
+            active.class_list().contains("relay-popover__settings-link"),
+            "settings-link button must receive focus when the popover opens"
+        );
+    }
+
+    /// Issue #352 — popover advertises itself as a non-modal dialog.
+    /// The popover does not trap focus, so `aria-modal="false"` is the
+    /// honest signal to assistive tech.
+    #[wasm_bindgen_test]
+    async fn relay_signal_button_popover_is_non_modal() {
+        let container = mount_test(move || {
+            let InitialSignals {
+                app_state,
+                write,
+                trust_store: _,
+            } = create_signals();
+            write.queue.set_relay_status.set(RelayStatus::Reachable);
+            provide_context(app_state);
+            provide_context(write);
+            view! { <RelaySignalButton /> }
+        });
+        tick().await;
+
+        let btn = query(&container, ".relay-signal-button").expect("button must render");
+        simulate_click(&btn);
+        tick().await;
+
+        let popover = query(&container, ".relay-popover").expect("popover must be open");
+        assert_eq!(
+            popover.get_attribute("aria-modal").as_deref(),
+            Some("false"),
+            "popover is non-modal — must advertise aria-modal=\"false\""
+        );
+        assert_eq!(
+            popover.get_attribute("role").as_deref(),
+            Some("dialog"),
+            "popover must keep role=dialog so aria-haspopup=\"dialog\" stays accurate"
+        );
+    }
+
     /// `request_animation_frame` wrapper used to schedule signal
     /// updates after mount so the `Effect` subscribing to
     /// `device_online` has already run once with the `prev == true`