Skip to content

fix: accessibility issues in keyboard navigation #3138

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
leekeh wants to merge 12 commits into
base: main
Choose a base branch
from
Open

fix: accessibility issues in keyboard navigation #3138

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
9d3bc0d
fix: better a11y for PackageSelector (use combobox pattern)
leekeh Aug 4, 2026
457ca33
fix: a11y issue in usercombobox
leekeh Aug 4, 2026
7aaa841
fix: align readmetocdropdown with standards
leekeh Aug 4, 2026
d79a99d
fix: a11y issues with VersionSelector
leekeh Aug 4, 2026
afee346
feat: use j/k navigation for search results
leekeh Aug 4, 2026
a1d64c6
fix: apply combobox aria attributes to command palatte
leekeh Aug 4, 2026
47e5784
fix: revert command palette changes
leekeh Aug 5, 2026
f6a4e82
test: update version selector tests
leekeh Aug 5, 2026
772c66b
fix: tests
leekeh Aug 6, 2026
0ac28e8
fix: remove unused import
leekeh Aug 6, 2026
20363ac
fix: correct userEvent
leekeh Aug 6, 2026
ce77199
fix: selector
leekeh Aug 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/components/AppFooter.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ const footerSections = computed<Array<{ label: string; links: FooterLink[] }>>((
</p>
<ul class="mb-6 flex flex-col gap-2">
<li class="flex gap-2 items-center">
<kbd class="kbd">↑</kbd>/<kbd class="kbd">↓</kbd>
<kbd class="kbd">j</kbd>/<kbd class="kbd">k</kbd>
<span>{{ $t('shortcuts.navigate_results') }}</span>
</li>
<li class="flex gap-2 items-center">
Expand Down
37 changes: 31 additions & 6 deletions app/components/Compare/PackageSelector.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ const props = defineProps<{

const maxPackages = computed(() => props.max ?? MAX_PACKAGE_SELECTION)

// Generate unique IDs for accessibility (prevents collisions when multiple instances mount)
const inputId = useId()
const listboxId = `${inputId}-listbox`

// Input state
const inputValue = shallowRef('')
const isInputFocused = shallowRef(false)
Expand Down Expand Up @@ -175,6 +179,14 @@ function handleKeydown(e: KeyboardEvent) {
}
}

const activeDescendantId = computed(() =>
highlightedIndex.value >= 0 ? `${listboxId}-option-${highlightedIndex.value}` : undefined,
)

const showDropdown = computed(
() => isInputFocused.value && (navigableItems.value.length > 0 || isSearching.value),
)

// Reset highlight when user types
watch(inputValue, () => {
highlightedIndex.value = -1
Expand All @@ -200,8 +212,8 @@ onClickOutside(containerRef, () => {
<template>
<div class="space-y-3">
<!-- Selected packages -->
<div v-if="packages.length > 0" class="flex flex-wrap gap-2">
<TagStatic v-for="pkg in packages" :key="pkg">
<ul v-if="packages.length > 0" class="flex flex-wrap gap-2">
<TagStatic as="li" v-for="pkg in packages" :key="pkg">
<!-- No dependency display -->
<template v-if="pkg === NO_DEPENDENCY_ID">
<span class="text-sm text-accent italic flex items-center gap-1.5">
Expand All @@ -223,12 +235,12 @@ onClickOutside(containerRef, () => {
classicon="i-lucide:x"
/>
</TagStatic>
</div>
</ul>

<!-- Add package input -->
<div v-if="packages.length < maxPackages" ref="containerRef" class="relative">
<div class="relative group flex items-center">
<label for="package-search" class="sr-only">
<label :for="inputId" class="sr-only">
{{ $t('compare.selector.search_label') }}
</label>
<span
Expand All @@ -237,7 +249,7 @@ onClickOutside(containerRef, () => {
/
</span>
<InputBase
id="package-search"
:id="inputId"
v-model="inputValue"
type="text"
:placeholder="
Expand All @@ -247,7 +259,12 @@ onClickOutside(containerRef, () => {
"
no-correct
class="w-full min-w-25 ps-7"
role="combobox"
aria-autocomplete="list"
aria-haspopup="listbox"
:aria-controls="listboxId"

@coderabbitai coderabbitai Bot Aug 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Keep combobox aria-controls references valid.

Both components retain aria-controls while v-if removes the referenced listbox. Bind aria-controls only when the dropdown is rendered, or retain a hidden listbox. ARIA combobox guidance only requires this relationship while the popup is visible. (w3.org)

  • app/components/Compare/PackageSelector.vue#L265-L265: bind aria-controls to showDropdown.
  • app/components/UserCombobox.vue#L175-L175: bind the input aria-controls to showDropdown when this listbox is mounted.
📍 Affects 2 files
  • app/components/Compare/PackageSelector.vue#L265-L265 (this comment)
  • app/components/UserCombobox.vue#L175-L175
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/components/Compare/PackageSelector.vue` at line 265, Keep combobox
aria-controls references valid by conditionally binding them to showDropdown so
they are present only while the referenced listbox is mounted. Apply this change
in app/components/Compare/PackageSelector.vue at lines 265-265 and
app/components/UserCombobox.vue at lines 175-175, preserving the existing
listbox rendering behavior.

:aria-expanded="showDropdown"
:aria-activedescendant="activeDescendantId"
ref="inputRef"
@focus="isInputFocused = true"
@keydown="handleKeydown"
Expand All @@ -263,14 +280,19 @@ onClickOutside(containerRef, () => {
leave-to-class="opacity-0"
>
<div
v-if="isInputFocused && (navigableItems.length > 0 || isSearching)"
v-if="showDropdown"
:id="listboxId"
ref="listRef"
role="listbox"
class="absolute top-full inset-x-0 mt-1 px-0.5 bg-bg-elevated border border-border rounded-lg shadow-lg z-50 max-h-64 overflow-y-auto"
>
<!-- No dependency option (easter egg with James) -->
<ButtonBase
v-if="showNoDependencyOption"
data-navigable
role="option"
:id="`${listboxId}-option-0`"
:aria-selected="highlightedIndex === 0"
class="block w-full text-start !border-transparent"
:class="highlightedIndex === 0 ? '!bg-accent/15' : ''"
:aria-label="$t('compare.no_dependency.add_column')"
Expand All @@ -296,6 +318,9 @@ onClickOutside(containerRef, () => {
v-for="(result, index) in filteredResults"
:key="result.name"
data-navigable
role="option"
:id="`${listboxId}-option-${index + resultIndexOffset}`"
:aria-selected="highlightedIndex === index + resultIndexOffset"
class="block w-full text-start my-0.5 !border-transparent"
:class="highlightedIndex === index + resultIndexOffset ? '!bg-accent/15' : ''"
@mouseenter="highlightedIndex = index + resultIndexOffset"
Expand Down
Loading
Loading