fix: make the select popup ours to style, not the OS's - #59
Merged
Conversation
#58 unified the closed state of all four selects, and said plainly that the open <option> list stays OS-drawn because CSS cannot reach it. On macOS that left a system popup opening out of a UI that looks nothing like one -- the one part of the control a user actually looks at while choosing. So the element goes. SelectField implements the ARIA combobox pattern with aria-activedescendant: focus stays on the trigger and the active option is named by id, which avoids restoring focus on every close path. role="combobox" is unchanged, so getByRole("combobox", { name }) still finds all three call sites; only userEvent.selectOptions had to go, and tests now click the trigger and then the option, which is what a user does. What replacing a native element costs, all of it here rather than discovered later: outside-dismiss listens on pointerdown rather than click, because a click listener fires after the trigger's own handler and reopens what the user meant to close, and it bubbles from document rather than capturing, because on the capture phase it ran before React's handlers and closed the list that had just opened. Arrowing sets an active index without committing, so browsing with the keyboard does not apply each option in passing. Typeahead extends a query within a second. The list flips above the trigger when there is no room below, since .app-window is overflow: hidden and would clip it rather than scroll -- which is exactly the case for the two sidebar pickers sitting near the bottom edge. The two language selects collapse into one. They differed only in option text ("中文" vs "中"), with CSS showing one per breakpoint; now the short labels live in a list that is ours and is not clipped to the 72px rail's width. ProviderSegment's label becomes a span: htmlFor associates with form elements, and the trigger is a button, so SelectField carries the accessible name itself.
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.
Closes #55.
#55 was closed by #58, but only for the closed state. #58 said so plainly: "The open
<option>list stays OS-drawn. CSS cannot reach it, so this unifies the closed state only." On macOS that leaves a system popup opening out of a UI that looks nothing like one — and the open list is the part a user is actually looking at while choosing. So #55's premise is only half addressed, and this finishes it.What replaces it
SelectFieldimplements the ARIA combobox pattern witharia-activedescendant: focus stays on the trigger while the active option is named by id. Moving real DOM focus into the list is the other legal shape, but it means restoring focus on every close path.role="combobox"is unchanged, sogetByRole("combobox", { name })still finds all three call sites exactly as it found the native elements. OnlyuserEvent.selectOptionshad to go — tests now click the trigger and then the option, which is what a user does anyway.The cost of replacing a native element
All of it is here rather than left to be discovered in review:
pointerdown, notclick. A click listener fires after the trigger's own handler has already toggled, reopening what the user meant to close.documentrather than capturing. On the capture phase it ran before React's synthetic handlers, so thepointerdownthat opened the list closed it again on the way through. Thecontains()checks are what make the bubble phase safe..app-windowisoverflow: hidden, so a list running past the sidebar's bottom edge would be clipped, not scrolled — which is precisely the case for the two pickers docked near that edge.dereaches 深色 instead of restarting each letter.Two call-site changes worth flagging
The two language selects collapse into one. They differed only in option text (中文 vs 中), with CSS showing one and hiding the other per breakpoint. The short labels now live in the option list, which is ours and is not clipped to the 72px rail's width.
ProviderSegment's<label>becomes a<span>.htmlForonly associates with form elements and the trigger is a button, soSelectFieldcarries the accessible name itself.Verification
pnpm run test— 25 files, 176 tests pass (10 new forSelectField, covering pointer commit, keyboard commit, arrow-without-commit, outside dismiss, Escape, and the preserved combobox role)pnpm run buildandtsc --noEmitpassgo test ./...andgo vet ./...pass.language-select-wide/.language-select-compactremainThe e2e suite is updated for the two-step interaction, and its coverage-collision check now targets the new trigger selectors.
🤖 Generated with Claude Code