From 3e88650ba0ebecaf304ac39ad621a5d5fecb60fd Mon Sep 17 00:00:00 2001 From: Jason Lee Date: Sat, 26 Sep 2026 23:01:10 +0800 Subject: [PATCH 1/4] input: Revert accessibility focus change and cover tab traversal --- crates/component/src/input/input.rs | 149 ++----------- crates/kit/Cargo.toml | 4 + crates/kit/tests/input_focus.rs | 335 ++++++++++++++++++++++++++++ 3 files changed, 363 insertions(+), 125 deletions(-) create mode 100644 crates/kit/tests/input_focus.rs diff --git a/crates/component/src/input/input.rs b/crates/component/src/input/input.rs index 6ef800a5bf..69af2aa802 100644 --- a/crates/component/src/input/input.rs +++ b/crates/component/src/input/input.rs @@ -485,35 +485,21 @@ impl Input { let Some(gpui::accesskit::ActionData::Value(value)) = data else { return; }; - if !state.presentation(cx).is_editable() { - return; - } state.replace_all(value.to_string(), window, cx); } - fn handle_accessibility_focus(state: &TextInputState, window: &mut Window, cx: &mut App) { - if !state.presentation(cx).is_disabled() { - state.focus(window, cx); - } - } - /// This method must after the refine_style. fn render_editor( input_state: TextInputState, search_panel: Option, - focus_scope: &gpui::FocusHandle, _: &Window, ) -> impl IntoElement { - v_flex() - .track_focus(focus_scope) - .size_full() - .children(search_panel) - .child( - div() - .relative() - .flex_1() - .child(input_state.into_any_element()), - ) + v_flex().size_full().children(search_panel).child( + div() + .relative() + .flex_1() + .child(input_state.into_any_element()), + ) } } @@ -675,18 +661,14 @@ impl RenderOnce for Input { if input_focused { sync_native_content_type(window, content_type, presentation.is_editable()); } - // The semantic frame tracks the editor; addons retain their own focus scopes. - let [prefix_focus, suffix_focus, editor_scope_focus] = window - .use_keyed_state(("input-addon-focus", state.entity_id()), cx, |_, cx| { - [cx.focus_handle(), cx.focus_handle(), cx.focus_handle()] + let frame_focus_handle = window + .use_keyed_state(("input-frame-focus", state.entity_id()), cx, |_, cx| { + cx.focus_handle() }) .read(cx) .clone(); let focused = input_focused - || (!presentation.is_disabled() - && [&prefix_focus, &suffix_focus, &editor_scope_focus] - .iter() - .any(|focus| focus.contains_focused(window, cx))); + || (frame_focus_handle.contains_focused(window, cx) && !presentation.is_disabled()); let gap_x = match self.size { Size::Small => px(4.), @@ -731,7 +713,7 @@ impl RenderOnce for Input { BaseInput::new(id) .focused(focused) .disabled(disabled) - .track_focus(presentation.focus_handle()) + .track_focus(&frame_focus_handle) .styles(|styles| { styles.focused(|style| { style.when( @@ -747,11 +729,7 @@ impl RenderOnce for Input { this.aria_placeholder(placeholder) }) .when_some(accessibility_value, |this, value| this.aria_value(value)) - .on_a11y_action(AccessibleAction::Focus, { - let state = state.clone(); - move |_, window, cx| Self::handle_accessibility_focus(&state, window, cx) - }) - .when(presentation.is_editable(), |this| { + .when(!disabled, |this| { this.on_a11y_action(AccessibleAction::SetValue, move |data, window, cx| { Self::handle_accessibility_set_value(&accessibility_state, data, window, cx); }) @@ -785,17 +763,11 @@ impl RenderOnce for Input { ) .children(prefix.map(|p| { div() - .track_focus(&prefix_focus) .when(presentation.is_disabled(), |this| this.opacity(0.5)) .child(p) })) .when(presentation.is_multi_line(), |this| { - this.child(Self::render_editor( - state.clone(), - overlays.search, - &editor_scope_focus, - window, - )) + this.child(Self::render_editor(state.clone(), overlays.search, window)) }) .when(!presentation.is_multi_line(), |this| { this.child(state.clone().into_any_element()) @@ -804,7 +776,6 @@ impl RenderOnce for Input { this.pr(self.size.input_px()).child( h_flex() .id("suffix") - .track_focus(&suffix_focus) .gap(gap_x) .items_center() .cursor_default() @@ -1030,17 +1001,15 @@ mod tests { ) -> impl IntoElement { let state = self.state.clone(); let emitted = self.emitted.clone(); - div() - .child(Input::new(&state)) - .on_prepaint(move |_, window, cx| { - let input = Input::new(&state).render(window, cx).into_element(); - let mut node = gpui::accesskit::Node::new(Role::TextInput); - input.write_a11y_info(&mut node); - *emitted.lock().unwrap() = Some(( - node.value().map(ToOwned::to_owned), - node.supports_action(AccessibleAction::SetValue), - )); - }) + div().on_prepaint(move |_, window, cx| { + let input = Input::new(&state).render(window, cx).into_element(); + let mut node = gpui::accesskit::Node::new(Role::TextInput); + input.write_a11y_info(&mut node); + *emitted.lock().unwrap() = Some(( + node.value().map(ToOwned::to_owned), + node.supports_action(AccessibleAction::SetValue), + )); + }) } } @@ -1063,84 +1032,14 @@ mod tests { let base: TextInputState = state.clone().into(); cx.update(|window, cx| { Input::handle_accessibility_set_value(&base, None, window, cx); - Input::handle_accessibility_focus(&base, window, cx); - assert!(base.presentation(cx).focus_handle().is_focused(window)); - window.draw(cx).clear(cx); }); assert_eq!(state.read_with(cx, |state, _| state.value()), "initial"); - let changes = std::rc::Rc::new(std::cell::Cell::new(0)); - let observed = changes.clone(); - let _subscription = cx.update(|_, cx| { - cx.subscribe(&state, move |_, event: &super::super::InputEvent, _| { - if matches!(event, super::super::InputEvent::Change) { - observed.set(observed.get() + 1); - } - }) - }); - let action = gpui::accesskit::ActionData::Value("updated🦀".into()); - cx.update(|window, cx| { - Input::handle_accessibility_set_value(&base, Some(&action), window, cx); - }); - assert_eq!(state.read_with(cx, |state, _| state.value()), "updated🦀"); - assert_eq!(changes.get(), 1); - for disabled in [false, true] { - cx.update(|window, cx| { - base.set_disabled(disabled, cx); - base.set_readonly(!disabled, cx); - window.blur(cx); - Input::handle_accessibility_focus(&base, window, cx); - assert_eq!( - base.presentation(cx).focus_handle().is_focused(window), - !disabled - ); - let action = gpui::accesskit::ActionData::Value("rejected".into()); - Input::handle_accessibility_set_value(&base, Some(&action), window, cx); - }); - assert_eq!(state.read_with(cx, |state, _| state.value()), "updated🦀"); - assert_eq!(changes.get(), 1); - } - cx.update(|window, cx| { - base.set_disabled(false, cx); - base.set_readonly(false, cx); - Input::handle_accessibility_focus(&base, window, cx); - window.draw(cx).clear(cx); - window.dispatch_action(Box::new(super::super::Undo), cx); - }); - assert_eq!(state.read_with(cx, |state, _| state.value()), "initial"); + let action = gpui::accesskit::ActionData::Value("updated".into()); cx.update(|window, cx| { - state.update(cx, |state, cx| state.set_masked(true, window, cx)); Input::handle_accessibility_set_value(&base, Some(&action), window, cx); - window.draw(cx).clear(cx); - }); - assert_eq!(state.read_with(cx, |state, _| state.value()), "updated🦀"); - assert_eq!(*captured.lock().unwrap(), Some((None, true))); - } - - #[gpui::test] - fn accessibility_set_value_preserves_exact_editor_text(cx: &mut gpui::TestAppContext) { - use gpui::{AppContext as _, Render}; - - struct Probe(Entity); - - impl Render for Probe { - fn render(&mut self, _: &mut Window, _: &mut gpui::Context) -> impl IntoElement { - div().child(crate::input::Editor::new(&self.0)) - } - } - - cx.update(crate::init); - let (probe, cx) = cx.add_window_view(|window, cx| { - Probe(cx.new(|cx| crate::input::EditorState::new(window, cx).language("rust"))) - }); - let editor = probe.read_with(cx, |probe, _| probe.0.clone()); - let state: TextInputState = editor.clone().into(); - let action = gpui::accesskit::ActionData::Value("(".into()); - - cx.update(|window, cx| { - Input::handle_accessibility_set_value(&state, Some(&action), window, cx) }); - assert_eq!(editor.read_with(cx, |editor, _| editor.value()), "("); + assert_eq!(state.read_with(cx, |state, _| state.value()), "updated"); } #[gpui::test] diff --git a/crates/kit/Cargo.toml b/crates/kit/Cargo.toml index c35b927957..5ab16a5685 100644 --- a/crates/kit/Cargo.toml +++ b/crates/kit/Cargo.toml @@ -98,6 +98,10 @@ required-features = ["test-support", "component"] name = "input" required-features = ["test-support", "component"] +[[test]] +name = "input_focus" +required-features = ["test-support", "component"] + [[test]] name = "ui" required-features = ["test-support", "component"] diff --git a/crates/kit/tests/input_focus.rs b/crates/kit/tests/input_focus.rs new file mode 100644 index 0000000000..1ddbf3f494 --- /dev/null +++ b/crates/kit/tests/input_focus.rs @@ -0,0 +1,335 @@ +mod common; + +use gpui_kit::{ + AppContext, Context, Entity, Focusable, TestAppContext, Window, WindowHandle, + base::Root, + component::{ + button::Button, + input::{AnyInputState, Editor, EditorState, Input, InputState, Textarea, TextareaState}, + }, + div, point, + prelude::*, + px, size, + test::TestWindowExt, +}; + +const INPUT_IDS: [&str; 3] = ["first", "second", "third"]; + +struct InputFocus { + inputs: [Entity; 3], + buttons: bool, + prefix_clicks: usize, + suffix_clicks: usize, +} + +impl Render for InputFocus { + fn render(&mut self, _: &mut Window, cx: &mut Context) -> impl IntoElement { + div().size_full().flex().flex_col().p_4().gap_4().children( + INPUT_IDS.into_iter().enumerate().map(|(ix, id)| { + Input::new(&self.inputs[ix]) + .id(id) + .w_96() + .prefix(div().child("Prefix")) + .suffix(div().child("Suffix")) + .when(self.buttons && id == "second", |input| { + input + .prefix(Button::new("prefix-button").label("Prefix").on_click( + cx.listener(|this, _, _, cx| { + this.prefix_clicks += 1; + cx.notify(); + }), + )) + .suffix(Button::new("suffix-button").label("Suffix").on_click( + cx.listener(|this, _, _, cx| { + this.suffix_clicks += 1; + cx.notify(); + }), + )) + }) + }), + ) + } +} + +fn open_inputs(cx: &mut TestAppContext, buttons: bool) -> (WindowHandle, Entity) { + cx.update(gpui_kit::init); + common::open_window(cx, Some(size(px(640.), px(480.))), |window, cx| { + cx.new(|cx| InputFocus { + inputs: std::array::from_fn(|_| cx.new(|cx| InputState::new(window, cx))), + buttons, + prefix_clicks: 0, + suffix_clicks: 0, + }) + }) +} + +fn press(handle: WindowHandle, key: &str, cx: &mut TestAppContext) { + cx.update_window(handle.into(), |_, window, cx| window.press(key, cx)) + .unwrap(); + // Focus subscriptions and deferred actions must settle before checking the + // destination or typing; otherwise addon focus theft can go unnoticed. + cx.run_until_parked(); +} + +fn assert_focus( + handle: WindowHandle, + view: &Entity, + expected: &'static str, + cx: &mut TestAppContext, +) { + cx.update_window(handle.into(), |_, window, cx| { + window.render_frame(cx); + assert_eq!(window.find(expected).focused(), Some(true), "{expected}"); + for (ix, id) in INPUT_IDS.into_iter().enumerate() { + assert_eq!( + view.read(cx).inputs[ix].focus_handle(cx).is_focused(window), + id == expected, + "editor focus for {id} when {expected} should be focused", + ); + } + if view.read(cx).buttons { + for id in ["prefix-button", "suffix-button"] { + assert_eq!( + window.find(id).focused(), + Some(id == expected), + "addon focus for {id} when {expected} should be focused", + ); + } + } else { + for id in INPUT_IDS { + assert_eq!(window.find(id).focused(), Some(id == expected), "{id}"); + } + } + }) + .unwrap(); +} + +#[gpui_kit::test] +fn reverse_tab_cycles_three_inputs_with_passive_addons(cx: &mut TestAppContext) { + let order = INPUT_IDS; + let (handle, view) = open_inputs(cx, false); + cx.update_window(handle.into(), |_, window, cx| window.click(order[2], cx)) + .unwrap(); + cx.run_until_parked(); + assert_focus(handle, &view, order[2], cx); + + // Start with reverse traversal: duplicate entries used to leave the last + // editor focused on the very first Shift-Tab. Include wraparound and repeat + // each cycle to catch registrations accumulating across frames. + for (key, destinations) in [ + ("shift-tab", [order[1], order[0], order[2]]), + ("tab", order), + ] { + for _ in 0..2 { + for destination in destinations { + press(handle, key, cx); + assert_focus(handle, &view, destination, cx); + cx.update_window(handle.into(), |_, window, cx| window.input("x", cx)) + .unwrap(); + cx.run_until_parked(); + } + } + } + + cx.update_window(handle.into(), |_, window, cx| { + window.render_frame(cx); + for (ix, id) in INPUT_IDS.into_iter().enumerate() { + assert_eq!(window.find(id).value(), Some("xxxx"), "{id}"); + assert_eq!(view.read(cx).inputs[ix].read(cx).value(), "xxxx", "{id}"); + } + }) + .unwrap(); +} + +#[gpui_kit::test] +fn tab_cycles_keep_prefix_and_suffix_buttons_focused(cx: &mut TestAppContext) { + let (handle, view) = open_inputs(cx, true); + cx.update_window(handle.into(), |_, window, cx| window.click("third", cx)) + .unwrap(); + cx.run_until_parked(); + + for (key, destinations) in [ + ( + "shift-tab", + ["suffix-button", "second", "prefix-button", "first", "third"], + ), + ( + "tab", + ["first", "prefix-button", "second", "suffix-button", "third"], + ), + ] { + for destination in destinations { + press(handle, key, cx); + assert_focus(handle, &view, destination, cx); + if destination.ends_with("-button") { + // Native button activation completes on key-up. `press` + // dispatches only the keystroke, so send both events here. + cx.update_window(handle.into(), |_, window, cx| { + let keystroke = gpui_kit::Keystroke::parse("enter").unwrap(); + window.dispatch_event( + gpui_kit::PlatformInput::KeyDown(gpui_kit::KeyDownEvent { + keystroke: keystroke.clone(), + is_held: false, + prefer_character_input: false, + }), + cx, + ); + window.dispatch_event( + gpui_kit::PlatformInput::KeyUp(gpui_kit::KeyUpEvent { keystroke }), + cx, + ); + }) + .unwrap(); + cx.run_until_parked(); + assert_focus(handle, &view, destination, cx); + } else { + cx.update_window(handle.into(), |_, window, cx| window.input("x", cx)) + .unwrap(); + cx.run_until_parked(); + } + } + } + + cx.update_window(handle.into(), |_, window, cx| { + window.render_frame(cx); + let view = view.read(cx); + assert_eq!(view.prefix_clicks, 2); + assert_eq!(view.suffix_clicks, 2); + for (ix, id) in INPUT_IDS.into_iter().enumerate() { + assert_eq!(window.find(id).value(), Some("xx"), "{id}"); + assert_eq!(view.inputs[ix].read(cx).value(), "xx", "{id}"); + } + }) + .unwrap(); +} + +enum MultilineField { + Textarea(Entity), + Editor(Entity), +} + +impl MultilineField { + fn state(&self) -> AnyInputState { + match self { + Self::Textarea(state) => state.clone().into(), + Self::Editor(state) => state.clone().into(), + } + } +} + +struct MultilineFocus { + previous: Entity, + field: MultilineField, +} + +impl Render for MultilineFocus { + fn render(&mut self, _: &mut Window, _: &mut Context) -> impl IntoElement { + div() + .size_full() + .flex() + .flex_col() + .p_4() + .gap_4() + .child(Input::new(&self.previous).id("previous").w_96()) + .child(match &self.field { + MultilineField::Textarea(state) => { + Textarea::new(state).w_96().h(px(160.)).into_any_element() + } + MultilineField::Editor(state) => { + Editor::new(state).w_96().h(px(160.)).into_any_element() + } + }) + } +} + +fn exercise_multiline_body_focus(cx: &mut TestAppContext, code_editor: bool) { + cx.update(gpui_kit::init); + let (handle, view) = common::open_window(cx, Some(size(px(640.), px(480.))), |window, cx| { + cx.new(|cx| MultilineFocus { + previous: cx.new(|cx| InputState::new(window, cx)), + field: if code_editor { + MultilineField::Editor(cx.new(|cx| EditorState::new(window, cx))) + } else { + MultilineField::Textarea(cx.new(|cx| TextareaState::new(window, cx))) + }, + }) + }); + cx.update_window(handle.into(), |_, window, cx| window.click("previous", cx)) + .unwrap(); + cx.run_until_parked(); + + let (state, id) = cx + .update_window(handle.into(), |_, window, cx| { + window.render_frame(cx); + let view = view.read(cx); + let state = view.field.state(); + assert!(view.previous.focus_handle(cx).is_focused(window)); + assert!(!state.focus_handle(cx).is_focused(window)); + let (entity_id, body) = match &view.field { + MultilineField::Textarea(state) => { + (state.entity_id(), state.read(cx).text_bounds()) + } + MultilineField::Editor(state) => (state.entity_id(), state.read(cx).text_bounds()), + }; + let id = gpui_kit::ElementId::from(("input", entity_id)); + let body = body.expect("multiline text body must be laid out"); + assert!(body.size.width > px(8.) && body.size.height > px(8.)); + // Use measured text geometry, excluding the frame padding and the + // code editor gutter. This hits the nested editor focus scope. + let position = body.origin + point(px(4.), px(4.)); + let offset = position - window.find(id.clone()).bounds().origin; + window.click_at(id.clone(), offset, cx); + (state, id) + }) + .unwrap(); + cx.run_until_parked(); + + cx.update_window(handle.into(), |_, window, cx| { + window.render_frame(cx); + assert!(state.focus_handle(cx).is_focused(window)); + assert!(!view.read(cx).previous.focus_handle(cx).is_focused(window)); + assert_eq!(window.find(id.clone()).focused(), Some(true)); + window.input("abcd", cx); + assert_eq!(state.value(cx), "abcd"); + }) + .unwrap(); + // Delete at the caret, move left, then extend the selection left. The + // replacement proves that both movement and selection reached the editor. + press(handle, "backspace", cx); + cx.update_window(handle.into(), |_, _, cx| assert_eq!(state.value(cx), "abc")) + .unwrap(); + press(handle, "left", cx); + press(handle, "shift-left", cx); + cx.update_window(handle.into(), |_, window, cx| { + window.input("Z", cx); + assert_eq!(state.value(cx), "aZc"); + assert_eq!(window.find(id.clone()).value(), Some("aZc")); + assert!(state.focus_handle(cx).is_focused(window)); + // Multiline Shift-Tab is OutdentInline. Exercise the same public + // traversal primitive Root uses without overriding that key binding. + window.focus_prev(cx); + }) + .unwrap(); + cx.run_until_parked(); + cx.update_window(handle.into(), |_, window, cx| { + window.render_frame(cx); + assert!(view.read(cx).previous.focus_handle(cx).is_focused(window)); + assert!(!state.focus_handle(cx).is_focused(window)); + assert_eq!(window.find("previous").focused(), Some(true)); + assert_eq!(window.find(id).focused(), Some(false)); + window.input("previous", cx); + assert_eq!(view.read(cx).previous.read(cx).value(), "previous"); + assert_eq!(state.value(cx), "aZc"); + }) + .unwrap(); +} + +#[gpui_kit::test] +fn textarea_body_click_focuses_editor_and_supports_editing(cx: &mut TestAppContext) { + exercise_multiline_body_focus(cx, false); +} + +#[gpui_kit::test] +fn editor_body_click_focuses_editor_and_supports_editing(cx: &mut TestAppContext) { + exercise_multiline_body_focus(cx, true); +} From f85b13c5547b646aa2d0df0a1625a12868f4e4cd Mon Sep 17 00:00:00 2001 From: Jason Lee Date: Sat, 26 Sep 2026 23:02:50 +0800 Subject: [PATCH 2/4] input: Restore guarded accessibility actions without changing focus ownership --- crates/component/src/input/input.rs | 109 +++++++++++++++++++++++++--- 1 file changed, 97 insertions(+), 12 deletions(-) diff --git a/crates/component/src/input/input.rs b/crates/component/src/input/input.rs index 69af2aa802..ef455b87bc 100644 --- a/crates/component/src/input/input.rs +++ b/crates/component/src/input/input.rs @@ -485,9 +485,18 @@ impl Input { let Some(gpui::accesskit::ActionData::Value(value)) = data else { return; }; + if !state.presentation(cx).is_editable() { + return; + } state.replace_all(value.to_string(), window, cx); } + fn handle_accessibility_focus(state: &TextInputState, window: &mut Window, cx: &mut App) { + if !state.presentation(cx).is_disabled() { + state.focus(window, cx); + } + } + /// This method must after the refine_style. fn render_editor( input_state: TextInputState, @@ -729,7 +738,11 @@ impl RenderOnce for Input { this.aria_placeholder(placeholder) }) .when_some(accessibility_value, |this, value| this.aria_value(value)) - .when(!disabled, |this| { + .on_a11y_action(AccessibleAction::Focus, { + let state = state.clone(); + move |_, window, cx| Self::handle_accessibility_focus(&state, window, cx) + }) + .when(presentation.is_editable(), |this| { this.on_a11y_action(AccessibleAction::SetValue, move |data, window, cx| { Self::handle_accessibility_set_value(&accessibility_state, data, window, cx); }) @@ -1001,15 +1014,17 @@ mod tests { ) -> impl IntoElement { let state = self.state.clone(); let emitted = self.emitted.clone(); - div().on_prepaint(move |_, window, cx| { - let input = Input::new(&state).render(window, cx).into_element(); - let mut node = gpui::accesskit::Node::new(Role::TextInput); - input.write_a11y_info(&mut node); - *emitted.lock().unwrap() = Some(( - node.value().map(ToOwned::to_owned), - node.supports_action(AccessibleAction::SetValue), - )); - }) + div() + .child(Input::new(&state)) + .on_prepaint(move |_, window, cx| { + let input = Input::new(&state).render(window, cx).into_element(); + let mut node = gpui::accesskit::Node::new(Role::TextInput); + input.write_a11y_info(&mut node); + *emitted.lock().unwrap() = Some(( + node.value().map(ToOwned::to_owned), + node.supports_action(AccessibleAction::SetValue), + )); + }) } } @@ -1032,14 +1047,84 @@ mod tests { let base: TextInputState = state.clone().into(); cx.update(|window, cx| { Input::handle_accessibility_set_value(&base, None, window, cx); + Input::handle_accessibility_focus(&base, window, cx); + assert!(base.presentation(cx).focus_handle().is_focused(window)); + window.draw(cx).clear(cx); }); assert_eq!(state.read_with(cx, |state, _| state.value()), "initial"); - let action = gpui::accesskit::ActionData::Value("updated".into()); + let changes = std::rc::Rc::new(std::cell::Cell::new(0)); + let observed = changes.clone(); + let _subscription = cx.update(|_, cx| { + cx.subscribe(&state, move |_, event: &super::super::InputEvent, _| { + if matches!(event, super::super::InputEvent::Change) { + observed.set(observed.get() + 1); + } + }) + }); + let action = gpui::accesskit::ActionData::Value("updated🦀".into()); + cx.update(|window, cx| { + Input::handle_accessibility_set_value(&base, Some(&action), window, cx); + }); + assert_eq!(state.read_with(cx, |state, _| state.value()), "updated🦀"); + assert_eq!(changes.get(), 1); + for disabled in [false, true] { + cx.update(|window, cx| { + base.set_disabled(disabled, cx); + base.set_readonly(!disabled, cx); + window.blur(cx); + Input::handle_accessibility_focus(&base, window, cx); + assert_eq!( + base.presentation(cx).focus_handle().is_focused(window), + !disabled + ); + let action = gpui::accesskit::ActionData::Value("rejected".into()); + Input::handle_accessibility_set_value(&base, Some(&action), window, cx); + }); + assert_eq!(state.read_with(cx, |state, _| state.value()), "updated🦀"); + assert_eq!(changes.get(), 1); + } cx.update(|window, cx| { + base.set_disabled(false, cx); + base.set_readonly(false, cx); + Input::handle_accessibility_focus(&base, window, cx); + window.draw(cx).clear(cx); + window.dispatch_action(Box::new(super::super::Undo), cx); + }); + assert_eq!(state.read_with(cx, |state, _| state.value()), "initial"); + cx.update(|window, cx| { + state.update(cx, |state, cx| state.set_masked(true, window, cx)); Input::handle_accessibility_set_value(&base, Some(&action), window, cx); + window.draw(cx).clear(cx); + }); + assert_eq!(state.read_with(cx, |state, _| state.value()), "updated🦀"); + assert_eq!(*captured.lock().unwrap(), Some((None, true))); + } + + #[gpui::test] + fn accessibility_set_value_preserves_exact_editor_text(cx: &mut gpui::TestAppContext) { + use gpui::{AppContext as _, Render}; + + struct Probe(Entity); + + impl Render for Probe { + fn render(&mut self, _: &mut Window, _: &mut gpui::Context) -> impl IntoElement { + div().child(crate::input::Editor::new(&self.0)) + } + } + + cx.update(crate::init); + let (probe, cx) = cx.add_window_view(|window, cx| { + Probe(cx.new(|cx| crate::input::EditorState::new(window, cx).language("rust"))) + }); + let editor = probe.read_with(cx, |probe, _| probe.0.clone()); + let state: TextInputState = editor.clone().into(); + let action = gpui::accesskit::ActionData::Value("(".into()); + + cx.update(|window, cx| { + Input::handle_accessibility_set_value(&state, Some(&action), window, cx) }); - assert_eq!(state.read_with(cx, |state, _| state.value()), "updated"); + assert_eq!(editor.read_with(cx, |editor, _| editor.value()), "("); } #[gpui::test] From 70b6eafabdd0ac174407f4cdd22b585f31a4cea1 Mon Sep 17 00:00:00 2001 From: Jason Lee Date: Sat, 26 Sep 2026 23:21:54 +0800 Subject: [PATCH 3/4] input: Preserve accessibility actions after merging revert --- crates/component/src/input/input.rs | 109 +++++++++++++++++++++++++--- 1 file changed, 97 insertions(+), 12 deletions(-) diff --git a/crates/component/src/input/input.rs b/crates/component/src/input/input.rs index 69af2aa802..ef455b87bc 100644 --- a/crates/component/src/input/input.rs +++ b/crates/component/src/input/input.rs @@ -485,9 +485,18 @@ impl Input { let Some(gpui::accesskit::ActionData::Value(value)) = data else { return; }; + if !state.presentation(cx).is_editable() { + return; + } state.replace_all(value.to_string(), window, cx); } + fn handle_accessibility_focus(state: &TextInputState, window: &mut Window, cx: &mut App) { + if !state.presentation(cx).is_disabled() { + state.focus(window, cx); + } + } + /// This method must after the refine_style. fn render_editor( input_state: TextInputState, @@ -729,7 +738,11 @@ impl RenderOnce for Input { this.aria_placeholder(placeholder) }) .when_some(accessibility_value, |this, value| this.aria_value(value)) - .when(!disabled, |this| { + .on_a11y_action(AccessibleAction::Focus, { + let state = state.clone(); + move |_, window, cx| Self::handle_accessibility_focus(&state, window, cx) + }) + .when(presentation.is_editable(), |this| { this.on_a11y_action(AccessibleAction::SetValue, move |data, window, cx| { Self::handle_accessibility_set_value(&accessibility_state, data, window, cx); }) @@ -1001,15 +1014,17 @@ mod tests { ) -> impl IntoElement { let state = self.state.clone(); let emitted = self.emitted.clone(); - div().on_prepaint(move |_, window, cx| { - let input = Input::new(&state).render(window, cx).into_element(); - let mut node = gpui::accesskit::Node::new(Role::TextInput); - input.write_a11y_info(&mut node); - *emitted.lock().unwrap() = Some(( - node.value().map(ToOwned::to_owned), - node.supports_action(AccessibleAction::SetValue), - )); - }) + div() + .child(Input::new(&state)) + .on_prepaint(move |_, window, cx| { + let input = Input::new(&state).render(window, cx).into_element(); + let mut node = gpui::accesskit::Node::new(Role::TextInput); + input.write_a11y_info(&mut node); + *emitted.lock().unwrap() = Some(( + node.value().map(ToOwned::to_owned), + node.supports_action(AccessibleAction::SetValue), + )); + }) } } @@ -1032,14 +1047,84 @@ mod tests { let base: TextInputState = state.clone().into(); cx.update(|window, cx| { Input::handle_accessibility_set_value(&base, None, window, cx); + Input::handle_accessibility_focus(&base, window, cx); + assert!(base.presentation(cx).focus_handle().is_focused(window)); + window.draw(cx).clear(cx); }); assert_eq!(state.read_with(cx, |state, _| state.value()), "initial"); - let action = gpui::accesskit::ActionData::Value("updated".into()); + let changes = std::rc::Rc::new(std::cell::Cell::new(0)); + let observed = changes.clone(); + let _subscription = cx.update(|_, cx| { + cx.subscribe(&state, move |_, event: &super::super::InputEvent, _| { + if matches!(event, super::super::InputEvent::Change) { + observed.set(observed.get() + 1); + } + }) + }); + let action = gpui::accesskit::ActionData::Value("updated🦀".into()); + cx.update(|window, cx| { + Input::handle_accessibility_set_value(&base, Some(&action), window, cx); + }); + assert_eq!(state.read_with(cx, |state, _| state.value()), "updated🦀"); + assert_eq!(changes.get(), 1); + for disabled in [false, true] { + cx.update(|window, cx| { + base.set_disabled(disabled, cx); + base.set_readonly(!disabled, cx); + window.blur(cx); + Input::handle_accessibility_focus(&base, window, cx); + assert_eq!( + base.presentation(cx).focus_handle().is_focused(window), + !disabled + ); + let action = gpui::accesskit::ActionData::Value("rejected".into()); + Input::handle_accessibility_set_value(&base, Some(&action), window, cx); + }); + assert_eq!(state.read_with(cx, |state, _| state.value()), "updated🦀"); + assert_eq!(changes.get(), 1); + } cx.update(|window, cx| { + base.set_disabled(false, cx); + base.set_readonly(false, cx); + Input::handle_accessibility_focus(&base, window, cx); + window.draw(cx).clear(cx); + window.dispatch_action(Box::new(super::super::Undo), cx); + }); + assert_eq!(state.read_with(cx, |state, _| state.value()), "initial"); + cx.update(|window, cx| { + state.update(cx, |state, cx| state.set_masked(true, window, cx)); Input::handle_accessibility_set_value(&base, Some(&action), window, cx); + window.draw(cx).clear(cx); + }); + assert_eq!(state.read_with(cx, |state, _| state.value()), "updated🦀"); + assert_eq!(*captured.lock().unwrap(), Some((None, true))); + } + + #[gpui::test] + fn accessibility_set_value_preserves_exact_editor_text(cx: &mut gpui::TestAppContext) { + use gpui::{AppContext as _, Render}; + + struct Probe(Entity); + + impl Render for Probe { + fn render(&mut self, _: &mut Window, _: &mut gpui::Context) -> impl IntoElement { + div().child(crate::input::Editor::new(&self.0)) + } + } + + cx.update(crate::init); + let (probe, cx) = cx.add_window_view(|window, cx| { + Probe(cx.new(|cx| crate::input::EditorState::new(window, cx).language("rust"))) + }); + let editor = probe.read_with(cx, |probe, _| probe.0.clone()); + let state: TextInputState = editor.clone().into(); + let action = gpui::accesskit::ActionData::Value("(".into()); + + cx.update(|window, cx| { + Input::handle_accessibility_set_value(&state, Some(&action), window, cx) }); - assert_eq!(state.read_with(cx, |state, _| state.value()), "updated"); + assert_eq!(editor.read_with(cx, |editor, _| editor.value()), "("); } #[gpui::test] From 526fb4c7f28175fe55091a04674f00d43c81ede8 Mon Sep 17 00:00:00 2001 From: Jason Lee Date: Sat, 26 Sep 2026 23:49:07 +0800 Subject: [PATCH 4/4] test: Render story gallery examples in separate frames --- .../tests/story_gallery_host.rs | 75 +++++++++++++++++-- examples/js_story/fixtures/all-examples.js | 33 +++++--- 2 files changed, 90 insertions(+), 18 deletions(-) diff --git a/crates/component-shell/tests/story_gallery_host.rs b/crates/component-shell/tests/story_gallery_host.rs index 5a4a453f47..bb5a52f470 100644 --- a/crates/component-shell/tests/story_gallery_host.rs +++ b/crates/component-shell/tests/story_gallery_host.rs @@ -196,6 +196,36 @@ fn dock_story_materializes_real_panels_dock_and_tabs(cx: &mut TestAppContext) { #[gpui::test] fn every_registered_story_example_materializes(cx: &mut TestAppContext) { cx.update(gpui_component_shell::init); + let surfaces = std::rc::Rc::new(std::cell::RefCell::new(Vec::::new())); + let registered_surfaces = surfaces.clone(); + let selected = std::rc::Rc::new(std::cell::RefCell::new(None::)); + let selected_surface = selected.clone(); + gpui_shell::export_module( + gpui_shell::HostModule::new("story-gallery-fixture") + .function("register_surfaces", move |args| { + *registered_surfaces.borrow_mut() = args + .get(0) + .and_then(gpui_shell::HostValue::as_array) + .expect("fixture surface list") + .iter() + .map(|value| value.as_str().expect("surface name").to_owned()) + .collect(); + Ok(gpui_shell::HostValue::Null) + }) + .function("selected_surface", move |_| { + Ok(gpui_shell::HostValue::from( + selected_surface.borrow().clone(), + )) + }), + ) + .expect("register fixture host module"); + struct FixtureModule; + impl Drop for FixtureModule { + fn drop(&mut self) { + gpui_shell::clear_exported_modules(); + } + } + let _fixture_module = FixtureModule; let runtime = gpui_component_shell::new_isolated_runtime().expect("runtime"); let root = Path::new(env!("CARGO_MANIFEST_DIR")).join("../../examples/js_story"); let loaded = runtime @@ -212,13 +242,42 @@ fn every_registered_story_example_materializes(cx: &mut TestAppContext) { ScriptRoot(view) }); let mut context = VisualTestContext::from_window(*window.deref(), cx); - context.update(|window, cx| window.draw(cx).clear(cx)); - context.run_until_parked(); - context.update(|window, cx| window.draw(cx).clear(cx)); - let view = mounted.borrow().clone().expect("mounted view"); - context.update(|_, cx| { - assert_eq!(view.read(cx).build_error(), None); - assert!(view.read(cx).snapshot().is_some()); - }); + let surfaces = surfaces.borrow().clone(); + assert!( + surfaces.len() > 1, + "fixture must enumerate registered surfaces" + ); + assert_eq!( + surfaces + .iter() + .collect::>() + .len(), + surfaces.len(), + "fixture surfaces must be unique" + ); + assert!(surfaces.iter().any(|surface| surface == "VirtualList")); + assert!(surfaces.iter().any(|surface| surface == "TabBar")); + assert!(!surfaces.iter().any(|surface| surface == "Tab")); + for surface in surfaces { + *selected.borrow_mut() = Some(surface.clone()); + context.update(|_, cx| view.update(cx, |view, cx| view.refresh(cx))); + context.update(|window, cx| window.draw(cx).clear(cx)); + context.run_until_parked(); + context.update(|window, cx| window.draw(cx).clear(cx)); + context.update(|_, cx| { + let view = view.read(cx); + assert_eq!(view.build_error(), None, "surface: {surface}"); + let tree = view.snapshot().expect("surface snapshot").debug_tree(); + if surface == "VirtualList" { + assert!(tree.contains("v_virtual_list"), "{surface}: {tree}"); + assert!(tree.contains("10,000 projects"), "{surface}: {tree}"); + } else { + assert!( + tree.contains(&format!("fixture-{surface}-")), + "{surface}: {tree}" + ); + } + }); + } } diff --git a/examples/js_story/fixtures/all-examples.js b/examples/js_story/fixtures/all-examples.js index f7ffa4041c..839dbc80e1 100644 --- a/examples/js_story/fixtures/all-examples.js +++ b/examples/js_story/fixtures/all-examples.js @@ -1,5 +1,6 @@ import { View, div } from "gpui-kit"; import { v_flex } from "gpui-base"; +import { register_surfaces, selected_surface } from "story-gallery-fixture"; import { coveredBy } from "../stories/coverage.js"; import { initializeRegisteredExamples, @@ -14,23 +15,35 @@ export default class AllRegisteredExamplesFixture extends View { init() { initializeRegisteredExamples(); this.virtualList = createVirtualListStory(); + register_surfaces([ + // Tab has no standalone examples; TabBar materializes its Tab children. + ...[...new Set(coveredBy.flatMap((entry) => entry.registrations))].filter( + (surface) => surface !== "Tab", + ), + "VirtualList", + ]); } render(cx) { - const surfaces = [...new Set(coveredBy.flatMap((entry) => entry.registrations))]; + // The host selects one surface per render so the complete inventory does + // not share a single frame's execution budget. + const surface = selected_surface(); + if (surface === null) return div(); + if (surface === "VirtualList") { + return renderVirtualListStory(this.virtualList, cx); + } + const examples = registeredExamples(surface, cx); + if (examples.length === 0) throw new Error(`No examples for ${surface}`); return v_flex() .w(900) .gap(16) .children( - surfaces.flatMap((surface) => - registeredExamples(surface, cx).map((example) => - div() - .id(`fixture-${surface}-${example.label}`) - .w_full() - .child(example.element), - ), + examples.map((example) => + div() + .id(`fixture-${surface}-${example.label}`) + .w_full() + .child(example.element), ), - ) - .child(renderVirtualListStory(this.virtualList, cx)); + ); } }