select: expose committed values and accessible activation - #4
Closed
grishy wants to merge 3 commits into
Closed
Conversation
Expose the committed value rather than the temporary search cursor. Handle accessible activation on the semantic root because native accessibility can flatten the painted trigger child.
Owner
Author
|
Continued upstream in longbridge#2973. |
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
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.
On macOS, Select's
AXPopUpButtonhas no current value orAXPressaction. Its trigger child can be flattened out of the accessibility tree, so the child's click handler is not enough.This exposes activation on the enabled root and supplies the committed selection as its accessible value. It reuses the existing open-state callback and focus handles. The value includes the item's title and prefix, or the placeholder when unselected. Filtering does not change it. Base Select exposes
.accessibility_value(...)for application-owned values.Screenshots
These screenshots are only for context. They show two states of the same example, not a visual before/after change.
Accessibility check
While searching for Rust, the root still reports the committed value Go. Captured from the macOS accessibility tree:
{ "role": "AXPopUpButton", "title": "Programming language", "value": "Language: Go", "actions": ["AXPress"] }How to Test
Scroll to Search and find Programming language in macOS Accessibility Inspector.
AXPress, typeGo, and press Enter. ExpectLanguage: Go, a closed popup, and focus on the trigger.Rust. The accessible value must remainLanguage: Go.AXPressto close. Check focus returns. Also check Down opens and Escape closes.AXPressaction.Automated checks
cargo test -p gpui-base -p gpui-component --lib --locked cargo clippy -p gpui-base -p gpui-component -p gpui-component-story --all-targets --locked -- -D warnings771 base and 419 component tests passed, along with strict Clippy and formatting. Regression coverage includes accessible metadata, disabled activation, filtering, prefixes, and placeholder fallback after clearing.
Windows and Linux runtime were not tested. Visual search preview and the existing disabled-root
AXEnabled=1reporting are outside this patch.Checklist
AI assistance
I tested the original change in my application, then asked OpenAI's
gpt-6-astrato extract it into a PR. The agent prepared the code, tests, verification scripts, and description, and ran the automated checks. I reviewed the extracted patch and manually tested the scenarios above on macOS using Accessibility Inspector.