Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
|
/approve-spec 7da075c |
There was a problem hiding this comment.
Requesting changes: the list-lifetime decision is owner-approved, but one sentence in PR3 needs fixing before this lands.
- It says the trigger's
aria-activedescendantnames the active option on every open, but with search on, AR2 andspec:AST-058FR6 put it on the search input, which holds focus. - Its start rule (selected, else first enabled) changes where ArrowDown, ArrowUp, searchable and disabled-first opens start today, while the compatibility line says documented keyboard behavior stays the same. Either say only that the first open starts like a later one, or record the change and reconcile it with #8868.
[Automated review]
| the exit plays on the list. No list stays mounted while closed. Every open | ||
| starts the same way, the first included: the highlight starts on the | ||
| selected option, or on the first enabled option when none is selected; the | ||
| trigger's `aria-activedescendant` names it; and FR2 alignment applies. |
There was a problem hiding this comment.
With hasSearch, AR2 keeps the combobox relationship at the search control and spec:AST-058 FR6 puts aria-activedescendant on the element that holds focus, which is the search input; the trigger is not a combobox then. Suggest: "the element AR2 gives the combobox relationship (the trigger, or the search input when search is on) names it in aria-activedescendant".
| the exit plays on the list. No list stays mounted while closed. Every open | ||
| starts the same way, the first included: the highlight starts on the | ||
| selected option, or on the first enabled option when none is selected; the |
There was a problem hiding this comment.
This sets a new start rule. Today ArrowDown opens on the first option and ArrowUp on the last, whatever the value; a searchable Selector opened by a press or Enter has no active option; and a disabled first option is highlighted when nothing is selected (useCombobox docs say so too). If the goal is only that the first open matches later opens, suggest: "the first open starts its highlight where a later open by the same path starts it". Otherwise this change needs a Current-state impact line and a compatibility line, and it has to agree with the FR8 that #8868 proposes.
| `astryx upgrade --from 0.6.3 --apply --path .`. | ||
| - A closed Selector renders no option list (PR3). Undocumented markup is | ||
| not a released contract (`spec:AST-017` FR3), and the documented selection, | ||
| keyboard, accessibility, and form behavior stays the same, so no caller must |
There was a problem hiding this comment.
While PR3 keeps its start rule, documented keyboard behavior does not stay the same: the ArrowDown, ArrowUp and search open paths change. Either narrow PR3 or name the keyboard change here and recheck the [fix] class against spec:AST-017 FR5.
…n-open # Conflicts: # docs/specs/components/Selector/Selector.spec.md
There was a problem hiding this comment.
Approval waits on an engineering owner's decision on the Selector record changes. The /approve-spec on this pull request named the previous head, and the latest commit changed PR3, so PR3, DEC-2, the compatibility line, Current-state impact and the new verification rows need a fresh decision: an engineering owner records it by commenting /approve-spec <full head SHA> on the pull request, and the author may do so as an engineering owner.
- Miss: the PR3, FR2 verification row (
Selector.spec.md:396) still fails an implementation whose open does not start on the selected or first enabled option, while PR3 now keeps today's start (ArrowDown on the first option, ArrowUp on the last, no active option when a search list opens by a press or Enter), so reword the row to test that the first open starts where a later open by the same path starts it and that AR2's combobox element names it.
Fixing the row is a new commit, so approving the head that carries it avoids a second approval. The earlier aria-activedescendant and PR3 start-rule findings are fixed, though the description's Decision list still describes the old start rule.
[Automated review]
| | FR7 | `Selector.test.tsx` press model suite | finger release over another option, finger/mouse release outside, listbox marker | The press option is selected, the listbox takes focus, a finger release outside closes the list, or the stray click selects | `audit:Selector/behavior` | | ||
| | AR5 | `Selector.activeOption.test.tsx`: the ring after keyboard input, no ring after pointer input, the ring again when keyboard input follows a pointer highlight; the active option's indicator measured against the listbox in light and dark | keyboard-active option, pointer-active option | Removing the ring, or drawing it after pointer input, fails; an indicator under 3:1 against the listbox in either mode fails (the fill alone measures 1.09:1 light and 1.15:1 dark) | `audit:Selector/accessibility` | | ||
| | PR3 | `Selector.test.tsx` mount suite and a Chromium story that counts elements | never opened, opened then closed, popover and bottom sheet | A closed Selector has any listbox, option, or search element besides its trigger, form value, and inert marker | `audit:Selector/behavior` | | ||
| | PR3, FR2 | `Selector.test.tsx` mount suite and placement tests | first open and a later open, by pointer and by keyboard, with and without a selected value | The first or a later open does not start on the selected option (or the first enabled option when none is selected), `aria-activedescendant` does not name it, or the selected row is not aligned over the trigger | `audit:Selector/behavior` | |
There was a problem hiding this comment.
This row still fails an implementation whose open does not start on the selected option (or the first enabled one), but PR3 (:226-227) now keeps today's start: ArrowDown opens on the first option and ArrowUp on the last whatever the value, a disabled first option is highlighted when nothing is selected, and a search list opened by a press or Enter has no active option (hooks.ts, useCombobox.doc.mjs). Suggest: "The first open does not start its highlight where a later open by the same path starts it, the element AR2 gives the combobox relationship does not name it in aria-activedescendant, or FR2 alignment fails", with states "first and later opens by press, Enter, ArrowDown and ArrowUp, with and without a selected value, search on and off".
Maintainer impact
A page with many Selectors becomes usable sooner and holds far less. Today every closed Selector keeps its whole option list mounted. Measured on a page with 20 Selectors of 249 options each (production build, popover presentation, 4× CPU slowdown, median of 7 loads), building the list on every open and removing it on close gives:
Each open costs a little more (about 50 ms on a 249-option reopen in this setup). The roughly 200 ms that every open of a long list costs is separate work; this record sets no rule for it and notes it under Current-state impact.
Exact decision needed
None open. The owner decided when a Selector builds its option list.
Existing authority searched
component:SelectorFR2, FR6, FR7, PR2, AR1, the bottom-sheet presentation, and the verification map. No claim sets the list's lifetime.component:Popover(side effects and lifetime): the default context layer keeps its host mounted while closed; lazy mounting is a Layer option. Selector opts out of that default.architecture:layer-runtime(Layer lazy mount, INV14 exit copy) andspec:AST-090(Selector is a layer surface whose exit plays when a theme turns exits on).component:ContextMenuPR1 keeps its surface after the first open; this record deliberately differs for Selector (DEC-2).Decision
The owner, on 2026-11-02: "I think best is we do B and we improve open time for large selector options".
Recorded as
component:SelectorPR3 and DEC-2:aria-activedescendantnaming it; FR2 alignment holds.aria-controlsnames the list only while it exists.Release class: patch
[fix]. Undocumented markup is not a released contract (spec:AST-017FR3); documented selection, keyboard, accessibility, and form behavior stays the same, so no caller must change source (FR1); a victim-free correction is[fix]with a patch bump (FR5, FR7).Rejected alternatives
Claim scope and non-goals
Evidence and implementation relationship
Changed records
component:Selector: new PR3, DEC-2, a Current-state impact section (PR3 gap, open time for long lists), a compatibility line with the release class, and six verification rows (closed element count; first and later opens' active option and alignment; typeahead while closed; form submission; exit animation;aria-controlspresence).Writing rubric results
Scope
Validation
pnpm install --frozen-lockfile: passedpnpm check:knowledge: "Knowledge templates and records are aligned."git diff --check: clean