Skip to content

Commit cf680a5

Browse files
committed
refactor(a11y): replace radio controls with action buttons for all/none
1 parent e5b0ab2 commit cf680a5

File tree

2 files changed

+63
-76
lines changed

2 files changed

+63
-76
lines changed

app/components/Compare/FacetSelector.vue

Lines changed: 10 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -22,52 +22,6 @@ function isCategoryNoneSelected(category: string): boolean {
2222
const selectableFacets = facets.filter(f => !f.comingSoon)
2323
return selectableFacets.length > 0 && selectableFacets.every(f => !isFacetSelected(f.id))
2424
}
25-
26-
function getCategoryActiveControl(category: string): 'all' | 'none' {
27-
if (isCategoryAllSelected(category)) return 'all'
28-
if (isCategoryNoneSelected(category)) return 'none'
29-
return 'all'
30-
}
31-
32-
function handleCategoryControlKeydown(category: string, event: KeyboardEvent): void {
33-
const { key } = event
34-
35-
if (!['ArrowLeft', 'ArrowRight', 'ArrowUp', 'ArrowDown'].includes(key)) return
36-
37-
event.preventDefault()
38-
39-
const target = event.currentTarget as HTMLElement | null
40-
if (!target) return
41-
42-
const group = target.closest('[data-facet-category-radiogroup]') as HTMLElement | null
43-
if (!group) return
44-
45-
const radios = Array.from(group.querySelectorAll<HTMLElement>('[role="radio"]'))
46-
if (!radios.length) return
47-
48-
const currentIndex = radios.indexOf(target)
49-
if (currentIndex === -1) return
50-
51-
let nextIndex = currentIndex
52-
53-
if (key === 'ArrowLeft' || key === 'ArrowUp') {
54-
nextIndex = (currentIndex - 1 + radios.length) % radios.length
55-
} else if (key === 'ArrowRight' || key === 'ArrowDown') {
56-
nextIndex = (currentIndex + 1) % radios.length
57-
}
58-
59-
const nextRadio = radios[nextIndex]
60-
if (!nextRadio) return
61-
const radioType = nextRadio.dataset.radioType
62-
63-
if (radioType === 'all') {
64-
selectCategory(category)
65-
} else if (radioType === 'none') {
66-
deselectCategory(category)
67-
}
68-
69-
nextRadio.focus()
70-
}
7125
</script>
7226

7327
<template>
@@ -76,9 +30,7 @@ function handleCategoryControlKeydown(category: string, event: KeyboardEvent): v
7630
<!-- Category header with all/none buttons -->
7731
<div
7832
class="flex items-center gap-2 mb-2"
79-
role="radiogroup"
8033
:aria-labelledby="`facet-category-label-${category}`"
81-
data-facet-category-radiogroup
8234
>
8335
<span
8436
:id="`facet-category-label-${category}`"
@@ -87,24 +39,22 @@ function handleCategoryControlKeydown(category: string, event: KeyboardEvent): v
8739
{{ getCategoryLabel(category) }}
8840
</span>
8941
<ButtonBase
90-
role="radio"
91-
:aria-checked="isCategoryAllSelected(category)"
92-
:tabindex="getCategoryActiveControl(category) === 'all' ? 0 : -1"
93-
data-radio-type="all"
94-
@keydown="handleCategoryControlKeydown(category, $event)"
95-
@click="selectCategory(category)"
42+
:aria-disabled="isCategoryAllSelected(category)"
43+
:aria-label="`${$t('compare.facets.all')} ${getCategoryLabel(category)} facets`"
44+
:data-facet-category="category"
45+
data-facet-category-action="all"
46+
@click="!isCategoryAllSelected(category) && selectCategory(category)"
9647
size="small"
9748
>
9849
{{ $t('compare.facets.all') }}
9950
</ButtonBase>
10051
<span class="text-2xs text-fg-muted/40" aria-hidden="true">/</span>
10152
<ButtonBase
102-
role="radio"
103-
:aria-checked="isCategoryNoneSelected(category)"
104-
:tabindex="getCategoryActiveControl(category) === 'none' ? 0 : -1"
105-
data-radio-type="none"
106-
@keydown="handleCategoryControlKeydown(category, $event)"
107-
@click="deselectCategory(category)"
53+
:aria-disabled="isCategoryNoneSelected(category)"
54+
:aria-label="`${$t('compare.facets.none')} ${getCategoryLabel(category)} facets`"
55+
:data-facet-category="category"
56+
data-facet-category-action="none"
57+
@click="!isCategoryNoneSelected(category) && deselectCategory(category)"
10858
size="small"
10959
>
11060
{{ $t('compare.facets.none') }}

test/nuxt/components/compare/FacetSelector.spec.ts

Lines changed: 53 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,9 @@ vi.mock('~/composables/useFacetSelection', () => ({
8585
facetsByCategory: computed(() => {
8686
const result: Record<string, ReturnType<typeof buildFacetInfo>[]> = {}
8787
for (const category of CATEGORY_ORDER) {
88-
result[category] = FACETS_BY_CATEGORY[category].map(facet => buildFacetInfo(facet))
88+
result[category] = FACETS_BY_CATEGORY[category].map((facet: ComparisonFacet) =>
89+
buildFacetInfo(facet),
90+
)
8991
}
9092
return result
9193
}),
@@ -230,12 +232,21 @@ describe('FacetSelector', () => {
230232
})
231233

232234
describe('category all/none buttons', () => {
235+
function findCategoryActionButton(
236+
component: Awaited<ReturnType<typeof mountSuspended>>,
237+
category: string,
238+
action: 'all' | 'none',
239+
) {
240+
return component.find(
241+
`button[data-facet-category="${category}"][data-facet-category-action="${action}"]`,
242+
)
243+
}
244+
233245
it('calls selectCategory when all button is clicked', async () => {
234246
const component = await mountSuspended(FacetSelector)
235247

236-
// Find the first 'all' radio (for performance category)
237-
const allButton = component.findAll('button[role="radio"]').find(b => b.text() === 'all')
238-
await allButton!.trigger('click')
248+
const allButton = findCategoryActionButton(component, 'performance', 'all')
249+
await allButton.trigger('click')
239250

240251
expect(mockSelectCategory).toHaveBeenCalledWith('performance')
241252
})
@@ -247,38 +258,64 @@ describe('FacetSelector', () => {
247258

248259
const component = await mountSuspended(FacetSelector)
249260

250-
// Find the first 'none' radio (for performance category)
251-
const noneButton = component.findAll('button[role="radio"]').find(b => b.text() === 'none')
252-
await noneButton!.trigger('click')
261+
const noneButton = findCategoryActionButton(component, 'performance', 'none')
262+
await noneButton.trigger('click')
253263

254264
expect(mockDeselectCategory).toHaveBeenCalledWith('performance')
255265
})
256266

257-
it('marks all button as checked when all facets in category are selected', async () => {
267+
it('marks all button as aria-disabled when all facets in category are selected', async () => {
258268
// Select all performance facets
259269
const performanceFacets: (string | ComparisonFacet)[] = FACETS_BY_CATEGORY.performance.filter(
260-
f => !FACET_INFO[f].comingSoon,
270+
(f: ComparisonFacet) => !FACET_INFO[f].comingSoon,
261271
)
262272
mockSelectedFacets.value = performanceFacets
263273
mockIsFacetSelected.mockImplementation((f: string) => performanceFacets.includes(f))
264274

265275
const component = await mountSuspended(FacetSelector)
266276

267-
const allButton = component.findAll('button[role="radio"]').find(b => b.text() === 'all')
268-
// First all button (performance) should be checked
269-
expect(allButton!.attributes('aria-checked')).toBe('true')
277+
const allButton = findCategoryActionButton(component, 'performance', 'all')
278+
279+
expect(allButton.attributes('aria-disabled')).toBe('true')
270280
})
271281

272-
it('marks none button as checked when no facets in category are selected', async () => {
282+
it('marks none button as aria-disabled when no facets in category are selected', async () => {
273283
// Deselect all performance facets
274284
mockSelectedFacets.value = ['downloads'] // only health facet selected
275285
mockIsFacetSelected.mockImplementation((f: string) => f === 'downloads')
276286

277287
const component = await mountSuspended(FacetSelector)
278288

279-
const noneButton = component.findAll('button[role="radio"]').find(b => b.text() === 'none')
280-
// First none button (performance) should be checked
281-
expect(noneButton!.attributes('aria-checked')).toBe('true')
289+
const noneButton = findCategoryActionButton(component, 'performance', 'none')
290+
291+
expect(noneButton.attributes('aria-disabled')).toBe('true')
292+
})
293+
294+
it('does not call selectCategory when all button action is already fulfilled', async () => {
295+
const performanceFacets: (string | ComparisonFacet)[] = FACETS_BY_CATEGORY.performance.filter(
296+
(f: ComparisonFacet) => !FACET_INFO[f].comingSoon,
297+
)
298+
mockSelectedFacets.value = performanceFacets
299+
mockIsFacetSelected.mockImplementation((f: string) => performanceFacets.includes(f))
300+
301+
const component = await mountSuspended(FacetSelector)
302+
303+
const allButton = findCategoryActionButton(component, 'performance', 'all')
304+
await allButton.trigger('click')
305+
306+
expect(mockSelectCategory).not.toHaveBeenCalled()
307+
})
308+
309+
it('does not call deselectCategory when none button action is already fulfilled', async () => {
310+
mockSelectedFacets.value = ['downloads']
311+
mockIsFacetSelected.mockImplementation((f: string) => f === 'downloads')
312+
313+
const component = await mountSuspended(FacetSelector)
314+
315+
const noneButton = findCategoryActionButton(component, 'performance', 'none')
316+
await noneButton.trigger('click')
317+
318+
expect(mockDeselectCategory).not.toHaveBeenCalled()
282319
})
283320
})
284321

0 commit comments

Comments
 (0)