From c5ce38f94d6f79682c29464005cbbbe57dbec40a Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 27 Apr 2026 06:29:04 +0000 Subject: [PATCH] fix(web): wire active-row a11y on search listbox MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Listbox row pass `selected=Signal::derive(|| false)` always — keyboard + AT users see no active option. Add `active_index` signal in `SearchUiState`, derive `selected` per row from flat in-display-order index, set `aria-activedescendant` on input, move index on Up/Down/Home/End. Reset on result/scope change. Browser test prove active row carry `aria-selected="true"`, others `false`, swap reactively, and cross-group flat-index walk pick right row under BTreeMap-grouped scope. Closes #344 --- crates/web/src/components/search/input.rs | 68 +++++- crates/web/src/components/search/results.rs | 51 ++++- crates/web/src/components/search/surface.rs | 11 + crates/web/src/state.rs | 11 + crates/web/tests/browser.rs | 233 ++++++++++++++++++++ 5 files changed, 367 insertions(+), 7 deletions(-) diff --git a/crates/web/src/components/search/input.rs b/crates/web/src/components/search/input.rs index 750b72db..fa9e9278 100644 --- a/crates/web/src/components/search/input.rs +++ b/crates/web/src/components/search/input.rs @@ -7,7 +7,13 @@ //! - `Esc` with a non-empty query clears it; `Esc` with an empty query //! closes the surface (spec §Desktop — Escape contract); //! - `aria-controls="search-results-list"` + `aria-autocomplete="list"` -//! + a placeholder that mirrors the active scope. +//! + a placeholder that mirrors the active scope; +//! - `ArrowUp` / `ArrowDown` / `Home` / `End` move +//! [`SearchUiState::active_index`](crate::state::SearchUiState::active_index) +//! so keyboard users can see which result row is the activation +//! target. The active row is also announced via +//! `aria-activedescendant`, which points at the row's +//! `id="search-row-{message_id}"` per WAI-ARIA listbox guidance. use leptos::prelude::*; use willow_client::SearchScope; @@ -37,6 +43,27 @@ pub fn SearchInput( let placeholder = move || placeholder_for(&state.search.scope.get()); + // Length of the *flat, in-display-order* result list. Computed + // here (not in the keydown handler) so the bound stays current + // when results are streamed in. + let result_count = move || { + super::results::flat_ordered( + &state.search.results.get_untracked(), + &state.search.scope.get_untracked(), + ) + .len() + }; + + // Active row's DOM id, or `None` when there are no results. Used + // for `aria-activedescendant` — per WAI-ARIA, the attribute must + // be omitted (not blank) when no option is active. + let active_descendant = Memo::new(move |_| { + let flat = + super::results::flat_ordered(&state.search.results.get(), &state.search.scope.get()); + let i = state.search.active_index.get(); + flat.get(i).map(|r| format!("search-row-{}", r.message_id)) + }); + let on_keydown = move |ev: web_sys::KeyboardEvent| match ev.key().as_str() { "Escape" => { ev.prevent_default(); @@ -50,6 +77,44 @@ pub fn SearchInput( ev.prevent_default(); on_submit.run(state.search.query.get_untracked()); } + "ArrowDown" => { + let n = result_count(); + if n == 0 { + return; + } + ev.prevent_default(); + let cur = state.search.active_index.get_untracked(); + // Wrap to top at the tail. Wrapping is the listbox-pattern + // default and matches how command palettes elsewhere in + // the app behave. + let next = if cur + 1 >= n { 0 } else { cur + 1 }; + write.search.set_active_index.set(next); + } + "ArrowUp" => { + let n = result_count(); + if n == 0 { + return; + } + ev.prevent_default(); + let cur = state.search.active_index.get_untracked(); + let next = if cur == 0 { n - 1 } else { cur - 1 }; + write.search.set_active_index.set(next); + } + "Home" => { + if result_count() == 0 { + return; + } + ev.prevent_default(); + write.search.set_active_index.set(0); + } + "End" => { + let n = result_count(); + if n == 0 { + return; + } + ev.prevent_default(); + write.search.set_active_index.set(n - 1); + } _ => {} }; @@ -73,6 +138,7 @@ pub fn SearchInput( aria-label="local search input" aria-autocomplete="list" aria-controls="search-results-list" + aria-activedescendant=move || active_descendant.get() prop:value=move || state.search.query.get() on:input=move |ev| write.search.set_query.set(event_target_value(&ev)) on:keydown=on_keydown diff --git a/crates/web/src/components/search/results.rs b/crates/web/src/components/search/results.rs index a300a433..bf8f9683 100644 --- a/crates/web/src/components/search/results.rs +++ b/crates/web/src/components/search/results.rs @@ -20,8 +20,37 @@ use willow_client::{SearchIndexBuildStatus, SearchResult, SearchScope}; use super::row::ResultRow; use crate::state::AppState; +/// Compute the cumulative flat-index offset of every group in +/// `groups`. Returns a vector of the same length whose `i`th entry is +/// the count of rows that appear *before* group `i` in the rendered +/// listbox. Combined with the row's intra-group index, this yields the +/// row's global flat index — the unit `active_index` is expressed in. +fn group_offsets(groups: &[(String, Vec)]) -> Vec { + let mut offsets = Vec::with_capacity(groups.len()); + let mut running = 0usize; + for (_, items) in groups { + offsets.push(running); + running += items.len(); + } + offsets +} + +/// Flatten grouped results into the order rows appear in the listbox. +/// `active_index` indexes into this vector, so keyboard navigation in +/// `` and `aria-selected` rendering here agree on what +/// "row N" means. +pub(super) fn flat_ordered(rows: &[SearchResult], scope: &SearchScope) -> Vec { + group_results(rows, scope) + .into_iter() + .flat_map(|(_, items)| items) + .collect() +} + /// Group results per the spec's scope-dependent rules. -fn group_results(rows: &[SearchResult], scope: &SearchScope) -> Vec<(String, Vec)> { +pub(super) fn group_results( + rows: &[SearchResult], + scope: &SearchScope, +) -> Vec<(String, Vec)> { match scope { SearchScope::ThisChannel(_) | SearchScope::ThisLetter(_) => { vec![(String::new(), rows.to_vec())] @@ -69,12 +98,15 @@ pub fn ResultsList( let groups = Memo::new(move |_| group_results(&state.search.results.get(), &state.search.scope.get())); + let active_index = state.search.active_index; let sections = move || { - groups - .get() + let groups_now = groups.get(); + let offsets = group_offsets(&groups_now); + groups_now .into_iter() - .map(|(label, items)| { + .enumerate() + .map(|(group_idx, (label, items))| { let header = if label.is_empty() { None } else { @@ -87,13 +119,20 @@ pub fn ResultsList( }) }; + let base = offsets[group_idx]; let rows: Vec = items .into_iter() - .map(|r| { + .enumerate() + .map(|(intra, r)| { + // Flat (in-display-order) index of this row. + // The `active_index` signal is expressed in the + // same units so a row's `selected` derives from + // a single equality check. + let flat = base + intra; view! { } diff --git a/crates/web/src/components/search/surface.rs b/crates/web/src/components/search/surface.rs index 8bdc82ea..42a0f260 100644 --- a/crates/web/src/components/search/surface.rs +++ b/crates/web/src/components/search/surface.rs @@ -33,6 +33,17 @@ pub fn SearchSurface( let state = use_context::().expect("AppState"); let write = use_context::().expect("AppWriteSignals"); + // Whenever the result set or scope changes, snap keyboard focus + // back to the first row. Without this, an `active_index` from a + // prior result set could outlive its data and point past the new + // tail (or at a different message entirely), breaking + // `aria-activedescendant` and `aria-selected`. + Effect::new(move |_| { + let _ = state.search.results.get(); + let _ = state.search.scope.get(); + write.search.set_active_index.set(0); + }); + // Debounced query driver: 120 ms after the last keystroke, parse // the query and run against the index under the current scope. // diff --git a/crates/web/src/state.rs b/crates/web/src/state.rs index 27c03122..86c9859a 100644 --- a/crates/web/src/state.rs +++ b/crates/web/src/state.rs @@ -124,6 +124,13 @@ pub struct SearchUiState { /// True while a 120 ms debounce timer is outstanding — UI dims the /// stale results row by 15 % per spec §Performance envelope. pub debouncing: ReadSignal, + /// Index of the keyboard-active result row in the flat (in-display- + /// order) results vector. Drives `aria-selected` per row and + /// `aria-activedescendant` on the search input. Reset to `0` when + /// the result set or scope changes. When `results` is empty, the + /// value is meaningless and consumers must not render + /// `aria-activedescendant`. + pub active_index: ReadSignal, } /// Tightened connection state companion to `NetworkState::connection_status`. @@ -323,6 +330,7 @@ pub struct SearchUiWriteSignals { pub set_status: WriteSignal, pub set_recents: WriteSignal>, pub set_debouncing: WriteSignal, + pub set_active_index: WriteSignal, } #[derive(Clone, Copy)] @@ -561,6 +569,7 @@ pub fn create_signals() -> InitialSignals { let (search_status, set_search_status) = signal(SearchIndexBuildStatus::default()); let (search_recents, set_search_recents) = signal(Vec::::new()); let (search_debouncing, set_search_debouncing) = signal(false); + let (search_active_index, set_search_active_index) = signal(0usize); // Persist scope on every change so the user's preference survives // a reload. Run on wasm only — native tests don't mount this state. @@ -660,6 +669,7 @@ pub fn create_signals() -> InitialSignals { status: search_status, recents: search_recents, debouncing: search_debouncing, + active_index: search_active_index, }, queue: QueueUiState { view: queue_view, @@ -746,6 +756,7 @@ pub fn create_signals() -> InitialSignals { set_status: set_search_status, set_recents: set_search_recents, set_debouncing: set_search_debouncing, + set_active_index: set_search_active_index, }, queue: QueueWriteSignals { set_view: set_queue_view, diff --git a/crates/web/tests/browser.rs b/crates/web/tests/browser.rs index be717932..27d95892 100644 --- a/crates/web/tests/browser.rs +++ b/crates/web/tests/browser.rs @@ -9549,6 +9549,239 @@ async fn phase_2e_recent_chip_has_listitem_role() { assert_eq!(text(&clear).trim(), "clear all recents"); } +// ── Phase 2e — active-row a11y wiring ─────────────────────────────────────── +// +// Closes #344. The listbox previously hard-coded `selected=false` on every +// ``, so keyboard / AT users had no way to tell which row Enter +// would activate. These tests verify the data wiring: the row whose flat +// (in-display-order) index equals `SearchUiState::active_index` carries +// `aria-selected="true"`, every other row carries `aria-selected="false"`, +// and moving `active_index` reactively swaps that bit. + +mod phase_2e_search_active_row { + use super::*; + use willow_client::{SearchResult, SearchScope}; + use willow_web::components::ResultsList; + use willow_web::state::{create_signals, InitialSignals}; + + fn fixture_result(id: &str, body: &str, ts: u64) -> SearchResult { + SearchResult { + message_id: id.into(), + channel_id: "general".into(), + channel_name: "general".into(), + grove_id: Some("grove-fixture".into()), + letter_id: None, + author_display_name: "Mira".into(), + author_handle: "mira".into(), + timestamp_ms: ts, + body: body.into(), + matched_ranges: Vec::new(), + } + } + + /// Mount `` with a seeded result set. Pins scope to + /// `ThisChannel` so grouping is the trivial single-implicit-group + /// shape — keeps the test focused on the active-row wiring rather + /// than the cross-group flat-index walk (which has its own + /// dedicated test). + /// Tuple of writers stashed during mount so the test body can drive + /// the seeded `` after it's on the DOM. + type StashedWriters = (WriteSignal, WriteSignal>); + + fn mount_results_with( + results: Vec, + active: usize, + ) -> (web_sys::HtmlElement, StashedWriters) { + let cell: std::rc::Rc>> = + std::rc::Rc::new(std::cell::Cell::new(None)); + let cell_for_mount = cell.clone(); + + let container = mount_test(move || { + let InitialSignals { + app_state, + write, + trust_store: _, + } = create_signals(); + + // Seed: we're scoped to one channel and have N results. The + // surface effect that resets `active_index` on result-set + // change is *not* mounted here, so the test owns the + // signal directly. + write + .search + .set_scope + .set(SearchScope::ThisChannel("general".into())); + write.search.set_results.set(results.clone()); + write.search.set_active_index.set(active); + + cell_for_mount.set(Some(( + write.search.set_active_index, + write.search.set_results, + ))); + + provide_context(app_state); + provide_context(write); + + view! { + + } + }); + + let writers = cell.take().expect("signals stashed during mount"); + (container, writers) + } + + #[wasm_bindgen_test] + async fn active_row_carries_aria_selected_true_others_false() { + let results = vec![ + fixture_result("m-0", "first hit", 30_000), + fixture_result("m-1", "second hit", 20_000), + fixture_result("m-2", "third hit", 10_000), + ]; + let (container, _writers) = mount_results_with(results, 1); + tick().await; + + let rows = query_all(&container, ".search-result-row"); + assert_eq!( + rows.len(), + 3, + "fixture mounts three rows; got {}", + rows.len() + ); + + // Row 0 — not active. + assert_eq!( + rows[0].get_attribute("aria-selected").as_deref(), + Some("false"), + "non-active rows must report aria-selected=\"false\"" + ); + // Row 1 — active. + assert_eq!( + rows[1].get_attribute("aria-selected").as_deref(), + Some("true"), + "the row at active_index must carry aria-selected=\"true\" \ + — without this, screen readers never see a selected option \ + in the listbox (regression guard for #344)" + ); + // Row 2 — not active. + assert_eq!( + rows[2].get_attribute("aria-selected").as_deref(), + Some("false") + ); + } + + #[wasm_bindgen_test] + async fn moving_active_index_swaps_selected_row_reactively() { + let results = vec![ + fixture_result("m-0", "first", 30_000), + fixture_result("m-1", "second", 20_000), + ]; + let (container, (set_active, _set_results)) = mount_results_with(results, 0); + tick().await; + + let rows = query_all(&container, ".search-result-row"); + assert_eq!( + rows[0].get_attribute("aria-selected").as_deref(), + Some("true") + ); + assert_eq!( + rows[1].get_attribute("aria-selected").as_deref(), + Some("false") + ); + + set_active.set(1); + tick().await; + + let rows = query_all(&container, ".search-result-row"); + assert_eq!( + rows[0].get_attribute("aria-selected").as_deref(), + Some("false"), + "row 0 must lose its selection when active_index moves to 1" + ); + assert_eq!( + rows[1].get_attribute("aria-selected").as_deref(), + Some("true"), + "row 1 must claim selection when active_index moves to 1" + ); + } + + #[wasm_bindgen_test] + async fn active_index_indexes_flat_in_display_order_across_groups() { + // Scope `AllGrovesAndLetters` groups by grove id (BTreeMap-sorted), + // so this fixture lands two grove-a rows before three grove-b + // rows. `active_index = 3` therefore must select the *first* + // grove-b row — proving `active_index` indexes into the flat + // in-display-order list, not the unsorted raw results vec. + let cell: std::rc::Rc>>> = + std::rc::Rc::new(std::cell::Cell::new(None)); + let cell_for_mount = cell.clone(); + + let container = mount_test(move || { + let InitialSignals { + app_state, + write, + trust_store: _, + } = create_signals(); + + // Push the grove-b rows first to prove ordering doesn't come + // from input order — only from the grouped display order. + let mut results = Vec::new(); + for (i, ts) in [(0u32, 10_000u64), (1, 20_000), (2, 30_000)].iter() { + let mut r = fixture_result(&format!("b-{i}"), "body b", *ts); + r.grove_id = Some("grove-b".into()); + results.push(r); + } + for (i, ts) in [(0u32, 40_000u64), (1, 50_000)].iter() { + let mut r = fixture_result(&format!("a-{i}"), "body a", *ts); + r.grove_id = Some("grove-a".into()); + results.push(r); + } + + write.search.set_scope.set(SearchScope::AllGrovesAndLetters); + write.search.set_results.set(results); + write.search.set_active_index.set(3); + + cell_for_mount.set(Some(write.search.set_active_index)); + + provide_context(app_state); + provide_context(write); + + view! { + + } + }); + let _ = cell.take(); + tick().await; + + let rows = query_all(&container, ".search-result-row"); + assert_eq!( + rows.len(), + 5, + "two grove-a rows + three grove-b rows = 5 visible rows" + ); + + // The selected row's id encodes its `message_id`. grove-a sorts + // before grove-b under BTreeMap, so flat index 3 = first grove-b + // row, which (sorted by timestamp_ms desc) is `b-2`. + let selected = query(&container, ".search-result-row[aria-selected=\"true\"]") + .expect("exactly one row must claim aria-selected=\"true\""); + assert_eq!( + selected.id(), + "search-row-b-2", + "active_index=3 under grove grouping must light up the first \ + grove-b row (b-2 by ts-desc), not a raw-index row" + ); + + // And no other row may share the bit. + let all_selected = query_all(&container, ".search-result-row[aria-selected=\"true\"]"); + assert_eq!( + all_selected.len(), + 1, + "exactly one row may carry aria-selected=\"true\" at a time" + ); + } +} + // ── Foundation tokens (Phase 0) ───────────────────────────────────────────── // // Closes Task 14 of `docs/plans/2026-04-19-ui-phase-0-foundation.md`.