input: Revert #3246 to restore reverse tab traversal - #3253
Merged
Merged
Conversation
This was referenced Sep 26, 2026
huacnlee
added a commit
that referenced
this pull request
Sep 26, 2026
…er (#3254) ## Description Follow-up to #3246 after the full revert in #3253. This PR targets main and contains only the independent accessibility action fix. Restore the narrowly scoped accessibility action fixes without sharing the editor's FocusHandle between the semantic frame and editor: - Explicit Focus action focuses the editor unless disabled. - SetValue is advertised only when editable and rechecks editability when executed. - Replacement continues through the existing replace_all path. The original frame handle, addon hierarchy, editor rendering, tab registration, and editing logic remain as before #3246. Production changes are limited to the action handlers and their registration; the remaining diff restores regression tests from #3246. No public API or dependency changes. ## How to Test - `cargo test -p gpui-kit --features test-support --test input_focus --test input --locked` — 10 passed, including the four focus regressions retained by #3253. - `cargo test -p gpui-component --features test-support --lib input::input::tests --locked` — 12 passed. - `cargo clippy -p gpui-kit --features test-support --test input_focus --locked -- --deny warnings` passed. - Targeted rustfmt checks and `git diff --check` passed. ## Remaining acceptance work Headless tests invoke accessibility helpers and inspect advertised SetValue properties; they do not drive native accessibility requests through GPUI's private dispatcher. - Verify native Focus followed by typing, including read-only and disabled controls. - Verify native SetValue, including editability changing after an action was queued. - Accessibility focused-node reporting remains unresolved: the independent semantic frame is not the actual editor focus handle, so the native accessibility tree can report window-root focus. This PR does not claim to preserve all focused-node reporting improvements from #3246. Native screen-reader automation and Story GUI checks have not run. AI-assisted: Codex prepared this focused follow-up and description using the implementation and tests from @steipete's #3246, and executed the listed automated checks. Maintainer/native acceptance remains pending.
huacnlee
added a commit
that referenced
this pull request
Sep 26, 2026
…on (#3256) ## Summary Expand Input, Textarea and Editor interaction regressions from 82 to **159 UI cases**, with an operation matrix and a single local gate: `script/test-input`. The aim is to replace the routine manual editing tour with reproducible assertions and make this suite an example for future component UI tests. Coverage includes typing/replacement, character/word/line/document/page movement, forward/reverse selection, Unicode and CRLF, pointer/word/paragraph selection, drag autoscroll, clipboard, Undo/Redo, validation and masks, readonly/disabled transitions, focus/lifecycle, wrapping/resizing, pairs/indentation, folding, multi-cursor editing, search/replace, deterministic completion/code-action/inline/definition providers, and IME protocol transitions on all three controls. Assertions inspect text together with selections, caret geometry, focus, history or host callbacks as appropriate. ### Production fixes exposed by the tests - Preserve disabled Input pointer protection, document-edge selection, preferred columns after horizontal selection/Alt-click/offscreen navigation, and visual-row affinity through Shift-End followed by Down or Shift-Down. - Clamp drag selection to laid-out text while autoscroll reveals subsequent rows, instead of immediately selecting to EOF. - Cancel stale completion/inline requests on editing, Escape, blur, protection changes and popup acceptance. - Report the IME selection direction correctly and prevent canceled preedit from merging surrounding typing into one Undo transaction. - Preserve replacement-field focus when its mode change echoes through overlay synchronization. - Make the existing GoToDefinition action query its provider without requiring prior hover. Separate explicit navigation from hover cancellation; reject superseded responses and responses after text/caret/focus changes. No new default keybinding. The runtime changes are limited to these paths. No public API signatures change. The existing `press` helper correction pairs key-down/up and avoids simulated IME text injection for Enter/Tab; its regressions remain included. The branch starts from main after #3253 and does not depend on #3254's accessibility behavior. It also carries the isolated gallery-fixture CI correction shared with #3254: materialize registered surfaces in separate frames without changing production execution budgets. ## Verification (Linux) - `script/test-input`: **536 passed**, comprising Base **300**, Component **44**, Input UI **159**, focus **4**, keyboard helpers **16**, touch selection **13**; no failures or ignored cases in the selected runs. - All **17 Kit interaction targets: 270 passed** (includes the Input/focus/helper/touch cases above). - `cargo clippy -p gpui-kit --features test-support,assets --test input --test input_focus --test interactions --test touch_selection --locked -- --deny warnings`: passed. - Scoped rustfmt, `bash -n script/test-input`, and `git diff --check`: passed. - Five independent review/test scopes were used. Observed failing cases were corrected and rerun; tests retain explicit blur observation, distinct clipboard sentinels and controlled provider responses rather than incidental cancellation/reset steps. The existing CI matrix runs the relevant targets on Linux, macOS and Windows. Results for the new head must be checked before merging. ## Review gate and boundaries See [`crates/kit/tests/input/README.md`](crates/kit/tests/input/README.md) for the operation matrix, fixtures, exact commands and instructions for adding a failing regression for each changed interaction. For ordinary editing changes, use that regression plus the full gate and platform CI. This covers repeatable interaction contracts, not every possible document/configuration/event ordering. Composition tests call the public `ElementInputHandler` bridge for mounted controls; they do not drive a real OS input method or the platform-installed handler. Native IME/candidate windows, accessibility dispatch, clipboard/menu adapters and pixel rendering retain their relevant platform checks. The in-flight completion disposal fixture is inactive because TestPlatform retains the active window's input handler after removal; it does not certify active-window platform disposal. Unicode movement preserves the existing scalar-boundary contract; grapheme movement, regex search, duplicate/move-line commands and Shift-Page selection are not introduced by these tests. ## AI assistance Implementation, tests, documentation and review were assisted by OpenAI Codex.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Reverts #3246 (8a7e0ec) in full and adds focused UI regression coverage.
The outer semantic frame and inner editor both registered the same FocusHandle in the TabStopMap. Reverse traversal could resolve another entry for the currently focused editor, leaving Shift-Tab stuck. The regression affects Input and the reverse-focus traversal primitive used with Textarea and Editor.
The production change is the exact inverse of #3246. The tests cover adjacent inputs, repeated forward/reverse cycles and wraparound, prefix/suffix button order and keyboard activation, multiline body clicks, editing, selection, and focus departure. Button activation sends native key-down and key-up events.
This full revert also removes #3246's accessibility Focus action and read-only/disabled SetValue protections. Draft follow-up #3254 restores narrowly scoped action handling while preserving existing focus ownership and the replace_all path. No public API or dependency changes.
How to Test
cargo test -p gpui-kit --features test-support --test input_focus --test input --locked— 10 passed.cargo test -p gpui-component --features test-support --lib input::input::tests --locked— 11 passed.rustfmt --check --edition 2024 crates/kit/tests/input_focus.rsandgit diff --checkpassed.Native screen-reader automation and Story GUI checks were not run. This PR restores prior behavior; it does not claim to resolve the original accessibility defects.
AI-assisted: Codex prepared the revert, regression tests, and description; the diff was checked and the tests above were executed by the agent. Maintainer review remains pending.