Conversation
Options were keyed by position, so typing in the search input made Vue reuse the same ComboboxItem instance and rewrite its text rather than move it. The item id belongs to that instance, and reka reads aria-activedescendant from the highlighted element's id - so the attribute kept pointing at an id that never changed and a screen reader was never told the active option moved. It kept announcing the first match however much the member typed, while the highlight tracked the filter correctly for sighted users. Keys each item by where it sits in the unfiltered items instead, so the key follows the item through the re-sort. filterGroups drops groups that filter away entirely, so the group key has to come from an item too; and identical primitives cannot be told apart by value, so each occurrence takes one of that value's original positions in turn and the keys stay unique. Resolves nuxt#6910 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthrough
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This localized change preserves option identity during filtering so assistive technology follows the highlighted option correctly, with regression coverage for reordered and duplicate items. No actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Linked Issues checkExplanation The implementation satisfies issue Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files. (1 skipped: 1 unsupported.) ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Some tools did not complete. Review the errors below. 🔧 ESLint
src/runtime/components/SelectMenu.vueParsing error: Unexpected token ) test/components/SelectMenu.spec.tsParsing error: Unexpected token { Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
commit: |
This branch was successfully deployed
1 participant
Resolves #6910.
The problem
SelectMenukeys its options by position, so typing in the search input — which re-sorts the list throughfilterGroups— makes Vue reuse the sameComboboxIteminstance and rewrite its text rather than move or replace it.The item's
idbelongs to that instance (ComboboxItem:useId(undefined, 'reka-combobox-item')), andaria-activedescendantis derived from the highlighted element's id (ListboxFilter:watchSyncEffect(() => activedescendant.value = rootContext.highlightedElement.value?.id)). So the attribute keeps pointing at an id that never changes, thewatchSyncEffectnever fires, and assistive technology is never told the active option moved — it keeps announcing the option that was active before the filter ran.Sighted users see nothing wrong, because
data-highlightedlands on that same reused node.This is not a reka bug: a stable id per item instance is exactly what
aria-activedescendantneeds, and reka is handed the same element with new text. Item identity is information onlySelectMenuhas, since it owns thev-for.The fix
Key each rendered item by where it sits in the unfiltered items, so the key follows the item through any re-sort. Two details the obvious fix gets wrong, and both are covered:
filterGroupsends with.filter(group => group.some(...)), so groups that filter away entirely are dropped andfilteredGroupsindices no longer align withgroups. A group key built fromgroupIndexis wrong as soon as one group empties, so the group key comes from its first item instead.['A', 'A']) cannot be told apart by value, so each occurrence consumes one of that value's original positions in turn. A naive value-derived key collides and Vue warns.Tests
Two regression tests in
test/components/SelectMenu.spec.ts:moves aria-activedescendant when filtering changes the highlighted item— the property that matters: when the highlighted option's text changes, itsidmust change too, and the search input'saria-activedescendantmust follow. Onv4it fails withexpected 'reka-combobox-item-v-3' not to be 'reka-combobox-item-v-3'— the text went fromAlphatoGammaand the id did not move.keeps keys unique when items repeat— pins the duplicate-primitive case, asserting no Vue duplicate-key warning.It types into the search input rather than setting
search-termas a prop, because it is reka's own search-term watcher that moves the highlight, and typing is the path a user takes.Locally, on this branch:
test/components/SelectMenu.spec.ts194 passed (both thenuxtandvueprojects, no snapshot churn — keys are not rendered),eslintclean on both changed files,vue-tsc --noEmitclean.Beyond the suite, measured by hand in Chromium across the item shapes, asserting the same property:
labelonly, novalue-keyvalue-keytype: label/separatorvirtualizeSelecting the highlighted option after filtering still yields the right value for every shape.
Scope, and what this deliberately leaves alone
virtualizeis not fixed, and looks like a separate problem: that branch renders throughComboboxVirtualizer, which recycles its own nodes and is passed no key at all, so the same id is reused by construction. reka excludes the virtual path from its highlight handling too (!rootContext.isVirtual.value).InputMenu.vue#L803-L804,Listbox.vue#L410-L411andDropdownMenuContent.vue#L186-L187, which all filter throughfilterGroupsand whose reka counterparts bindaria-activedescendantthe same way. I have left them alone on purpose:SelectMenuis the one I could measure against a real screen-reader user, and doing all four properly wants the keying extracted intouseFilter(which already ownsfilterGroups) rather than copied four times. That is a shape question for you — happy to extend this PR either way.Why it was reported
A blind member of our church could not use a venue picker in our app: it read out the first option and then went silent while he typed. He has been running exactly this change as a local patch and confirms it fixes it for him.