From 3e88650ba0ebecaf304ac39ad621a5d5fecb60fd Mon Sep 17 00:00:00 2001 From: Jason Lee Date: Sat, 26 Sep 2026 23:01:10 +0800 Subject: [PATCH] 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); +}