Skip to content

Commit

Permalink
test: add tests and fix accessibility issues for the select
Browse files Browse the repository at this point in the history
  • Loading branch information
fbasso committed Feb 7, 2024
1 parent 2a92391 commit d54d780
Show file tree
Hide file tree
Showing 12 changed files with 83 additions and 16 deletions.
5 changes: 4 additions & 1 deletion angular/lib/src/components/select/select.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ export class SelectItemDirective<Item> {
[auUse]="controlContainerDirective"
role="combobox"
class="d-flex align-items-center flex-wrap"
[attr.aria-controls]="state.id + '-menu'"
aria-haspopup="listbox"
[attr.aria-expanded]="state.open"
>
Expand Down Expand Up @@ -75,14 +76,16 @@ export class SelectItemDirective<Item> {
</div>
@if (state.open && state.visibleItems.length) {
<ul
[id]="state.id + '-menu'"
[auUse]="menuDirective"
[class]="'dropdown-menu show ' + (menuClassName || '')"
[attr.data-popper-placement]="state.placement"
(mousedown)="$event.preventDefault()"
>
@for (itemContext of state.visibleItems; track itemCtxTrackBy($index, itemContext)) {
<li
[class]="'au-select-item dropdown-item position-relative' + (itemContext === state.highlighted ? ' bg-primary text-light' : '')"
[class]="'au-select-item dropdown-item position-relative'"
[class.bg-text-bg-primary]="itemContext === state.highlighted"
[class.selected]="itemContext.selected"
(click)="widget.api.toggleItem(itemContext.item)"
>
Expand Down
13 changes: 9 additions & 4 deletions core/src/components/select/select.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ import {createSelect, getSelectDefaultConfig} from './select';
type ExtractReadable<T> = T extends ReadableSignal<infer U> ? U : never;
type ExtractState<T> = T extends SelectWidget<infer U> ? ExtractReadable<SelectWidget<U>['state$']> : never;

const normalizeState = createTraversal((_key, value) => {
const generatedIdRegExp = /^auId-/;
const normalizeState = createTraversal((path, value) => {
const constructor = value?.constructor;
switch (constructor) {
case RegExp:
Expand All @@ -21,6 +22,10 @@ const normalizeState = createTraversal((_key, value) => {
return '(function)';
}

if (path === 'id') {
return generatedIdRegExp.test(value) ? '(generated)' : value;
}

return value;
});

Expand Down Expand Up @@ -105,7 +110,7 @@ describe(`Select model`, () => {
disabled: false,
filterText: '',
highlighted: undefined,
id: undefined,
id: '(generated)',
loading: false,
menuClassName: '',
menuItemClassName: '',
Expand All @@ -130,7 +135,7 @@ describe(`Select model`, () => {
disabled: false,
filterText: '',
highlighted: {item: 'aa', id: 'aa', selected: false},
id: undefined,
id: '(generated)',
loading: false,
menuClassName: '',
menuItemClassName: '',
Expand Down Expand Up @@ -723,7 +728,7 @@ describe(`Select model`, () => {
selectWidget.patch({items: [item1, item2]});
open();
const expectedState: ReturnType<typeof getState> = {
id: undefined,
id: '(generated)',
ariaLabel: 'Select',
visibleItems: [{item: item2, id: '1', selected: false}],
highlighted: {item: item2, id: '1', selected: false},
Expand Down
5 changes: 5 additions & 0 deletions core/src/components/select/select.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import type {NavManagerItemConfig} from '../../services/navManager';
import {createNavManager} from '../../services/navManager';
import type {Directive, PropsConfig, SlotContent, Widget, WidgetSlotContext} from '../../types';
import {bindDirective} from '../../utils/directive';
import {getGeneratedId} from '../../utils/internal/dom';
import {noop} from '../../utils/internal/func';
import {bindableDerived, stateStores, writablesForProps} from '../../utils/stores';
import type {WidgetsCommonPropsAndState} from '../commonProps';
Expand Down Expand Up @@ -343,6 +344,7 @@ export function createSelect<Item>(config?: PropsConfig<SelectProps<Item>>): Sel
// Props
const [
{
id$: _dirtyId$,
open$: _dirtyOpen$,
filterText$: _dirtyFilterText$,
items$,
Expand All @@ -358,6 +360,8 @@ export function createSelect<Item>(config?: PropsConfig<SelectProps<Item>>): Sel
] = writablesForProps<SelectProps<Item>>(defaultConfig, config);
const {selected$} = stateProps;

const id$ = computed(() => _dirtyId$() ?? getGeneratedId());

const filterText$ = bindableDerived(onFilterTextChange$, [_dirtyFilterText$]);

const {hasFocus$, directive: hasFocusDirective} = createHasFocus();
Expand Down Expand Up @@ -471,6 +475,7 @@ export function createSelect<Item>(config?: PropsConfig<SelectProps<Item>>): Sel

const widget: SelectWidget<Item> = {
...stateStores({
id$,
visibleItems$,
highlighted$,
open$,
Expand Down
8 changes: 8 additions & 0 deletions core/src/utils/internal/dom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,3 +87,11 @@ export function addEvent(element: Element, type: string, fn: EventListenerOrEven
element.removeEventListener(type, fn);
};
}

let idCount = 0;
/**
* Generates a unique ID with the format 'auId-[counter]'.
*
* @returns The generated ID.
*/
export const getGeneratedId = () => `auId-${idCount++}`;
21 changes: 21 additions & 0 deletions e2e/demo.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import type {AxeResults} from 'axe-core';
import {globSync} from 'glob';
import path from 'path';
import {normalizePath} from './utils';
import {SelectPO} from '@agnos-ui/page-objects';

const pathToFrameworkDir = normalizePath(path.join(__dirname, '../demo/src/routes'));
const allRoutes = globSync(`${pathToFrameworkDir}/**/+page.svelte`).map((route) =>
Expand Down Expand Up @@ -33,4 +34,24 @@ test.describe.parallel('Demo Website', () => {
expect((await analyze(page, svelteRoute)).violations).toEqual([]);
});
}

const frameworks = [
{name: 'Angular', url: '/angular/samples/'},
{name: 'React', url: '/react/samples/'},
{name: 'Svelte', url: '/svelte/samples/app/'},
];

test.describe.parallel('Select tests', () => {
frameworks.forEach(({name, url}) => {
test(`[${name}] Select accessibility `, async ({page}) => {
const route = `${url}#/select/default`;
await page.goto(route);
const selectPO = new SelectPO(page);
const locatorInput = selectPO.locatorInput;
await locatorInput.fill('a');
await locatorInput.press('Enter');
expect((await analyze(page, route)).violations).toEqual([]);
});
});
});
});
2 changes: 2 additions & 0 deletions e2e/samplesMarkup.e2e-spec.ts-snapshots/select-custom.html
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
class="au-select border border-1 d-block dropdown mb-3 p-1"
>
<div
aria-controls="rewritten-id-1"
aria-expanded="false"
aria-haspopup="listbox"
class="align-items-center d-flex flex-wrap"
Expand All @@ -24,6 +25,7 @@
autocomplete="off"
autocorrect="off"
class="au-select-input border-0 flex-grow-1"
id="rewritten-id-2"
type="text"
value=""
/>
Expand Down
5 changes: 4 additions & 1 deletion e2e/samplesMarkup.e2e-spec.ts-snapshots/select-default.html
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
class="au-select border border-1 d-block dropdown mb-3 p-1"
>
<div
aria-controls="rewritten-id-1"
aria-expanded="true"
aria-haspopup="listbox"
class="align-items-center d-flex flex-wrap"
Expand All @@ -33,17 +34,19 @@
autocomplete="off"
autocorrect="off"
class="au-select-input border-0 flex-grow-1"
id="rewritten-id-2"
type="text"
value="a"
/>
</div>
<ul
class="dropdown-menu show"
data-popper-placement="bottom-start"
id="rewritten-id-1"
style="left: -1px; top: 40px;"
>
<li
class="au-select-item bg-primary dropdown-item position-relative selected text-light"
class="au-select-item bg-text-bg-primary dropdown-item position-relative selected"
>
"apple"
</li>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
class="au-select border border-1 d-block dropdown mb-3 p-1"
>
<div
aria-controls="rewritten-id-1"
aria-expanded="false"
aria-haspopup="listbox"
class="align-items-center d-flex flex-wrap"
Expand All @@ -21,6 +22,7 @@
autocomplete="off"
autocorrect="off"
class="au-select-input border-0 flex-grow-1"
id="rewritten-id-2"
type="text"
value=""
/>
Expand Down
2 changes: 2 additions & 0 deletions e2e/samplesMarkup.e2e-spec.ts-snapshots/select-select.html
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ <h2>
class="au-select border border-1 d-block dropdown mb-3 p-1"
>
<div
aria-controls="rewritten-id-1"
aria-expanded="false"
aria-haspopup="listbox"
class="align-items-center d-flex flex-wrap"
Expand All @@ -32,6 +33,7 @@ <h2>
autocomplete="off"
autocorrect="off"
class="au-select-input border-0 flex-grow-1"
id="rewritten-id-2"
type="text"
value=""
/>
Expand Down
8 changes: 4 additions & 4 deletions e2e/select/select.e2e-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,22 +161,22 @@ test.describe.parallel('Select tests', () => {
await keyboard.press('Enter');
await keyboard.press('ArrowDown');

expect(await getBadgesState()).toStrictEqual(['JavaScript', 'JavaScript engine', 'JSON']);
await expect.poll(getBadgesState).toStrictEqual(['JavaScript', 'JavaScript engine', 'JSON']);

await keyboard.press('Escape');
await keyboard.press('Home');
await keyboard.press('ArrowLeft');
await keyboard.press('ArrowLeft');
await keyboard.press('ArrowLeft');
await keyboard.press('Enter');
expect(await getBadgesState(), 'Middle badge must have been removed').toStrictEqual(['JavaScript', 'JSON']);
await expect.poll(getBadgesState, 'Middle badge must have been removed').toStrictEqual(['JavaScript', 'JSON']);

await keyboard.press('Enter');
expect(await getBadgesState(), 'First badge must have been removed').toStrictEqual(['JSON']);
await expect.poll(getBadgesState, 'First badge must have been removed').toStrictEqual(['JSON']);

await keyboard.press('ArrowRight');
await keyboard.press('Enter');
expect(await getBadgesState(), 'Last badge must have been removed').toStrictEqual([]);
await expect.poll(getBadgesState, 'Last badge must have been removed').toStrictEqual([]);

Check failure on line 179 in e2e/select/select.e2e-spec.ts

View workflow job for this annotation

GitHub Actions / e2e-tests / Test (5/10)

[angular:webkit] › select/select.e2e-spec.ts:143:7 › Select tests › Custom select › Navigation

1) [angular:webkit] › select/select.e2e-spec.ts:143:7 › Select tests › Custom select › Navigation Error: Last badge must have been removed expect(received).toStrictEqual(expected) // deep equality - Expected - 1 + Received + 3 - Array [] + Array [ + "JSON", + ] Call Log: - Timeout 5000ms exceeded while waiting on the predicate 177 | await keyboard.press('ArrowRight'); 178 | await keyboard.press('Enter'); > 179 | await expect.poll(getBadgesState, 'Last badge must have been removed').toStrictEqual([]); | ^ 180 | expect(await page.evaluate(() => (document.activeElement!.tagName || '').toLowerCase())).toBe('input'); 181 | }); 182 | }); at /home/runner/work/AgnosUI/AgnosUI/e2e/select/select.e2e-spec.ts:179:75
expect(await page.evaluate(() => (document.activeElement!.tagName || '').toLowerCase())).toBe('input');
});
});
Expand Down
20 changes: 17 additions & 3 deletions react/lib/src/components/select/select.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ function Rows<Item>({slotContext}: {slotContext: SelectContext<Item>}) {
const {id} = itemContext;
const classname = ['au-select-item dropdown-item position-relative'];
if (itemContext === highlighted) {
classname.push('bg-primary text-light');
classname.push('bg-text-bg-primary');
}
if (itemContext.selected) {
classname.push('selected');
Expand All @@ -76,6 +76,7 @@ export function Select<Item>(props: Partial<SelectProps<Item>>) {
const [state, widget] = useWidgetWithConfig<SelectWidget<Item>>(createSelect, props, 'select', defaultConfig);
const slotContext: SelectContext<Item> = {state, widget: toSlotContextWidget(widget)};
const {id, ariaLabel, visibleItems, filterText, open, className, menuClassName, placement} = state;
const menuId = `${id}-menu`;

const {
directives: {floatingDirective, hasFocusDirective, referenceDirective, inputContainerDirective},
Expand All @@ -85,7 +86,14 @@ export function Select<Item>(props: Partial<SelectProps<Item>>) {
const refSetMenu = useDirectives([hasFocusDirective, floatingDirective]);
return (
<div ref={refSetContainer} className={`au-select dropdown border border-1 p-1 mb-3 d-block ${className}`}>
<div ref={refSetInputContainer} role="combobox" className="d-flex align-items-center flex-wrap" aria-haspopup="listbox" aria-expanded={open}>
<div
ref={refSetInputContainer}
role="combobox"
className="d-flex align-items-center flex-wrap"
aria-controls={menuId}
aria-haspopup="listbox"
aria-expanded={open}
>
<Badges slotContext={slotContext}></Badges>
<input
id={id}
Expand All @@ -102,7 +110,13 @@ export function Select<Item>(props: Partial<SelectProps<Item>>) {
/>
</div>
{open && visibleItems.length > 0 && (
<ul ref={refSetMenu} className={`dropdown-menu show ${menuClassName}`} data-popper-placement={placement} onMouseDown={preventDefault}>
<ul
ref={refSetMenu}
id={menuId}
className={`dropdown-menu show ${menuClassName}`}
data-popper-placement={placement}
onMouseDown={preventDefault}
>
<Rows slotContext={slotContext}></Rows>
</ul>
)}
Expand Down
8 changes: 5 additions & 3 deletions svelte/lib/src/components/select/Select.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,17 @@
directives: {floatingDirective, hasFocusDirective, referenceDirective, inputContainerDirective},
} = widget;
$: widget.patchChangedProps($$props);
$: menuId = `${$id$}-menu`;
</script>

<div use:referenceDirective class="au-select dropdown border border-1 p-1 mb-3 d-block {$className$}">
<!-- svelte-ignore a11y-role-has-required-aria-props -->
<div
use:hasFocusDirective
use:inputContainerDirective
role="combobox"
class="d-flex align-items-center flex-wrap"
aria-controls={menuId}
aria-haspopup="listbox"
aria-expanded={$open$}
>
Expand Down Expand Up @@ -96,6 +98,7 @@
{#if $open$ && $visibleItems$.length > 0}
<!-- svelte-ignore a11y-no-noninteractive-element-interactions -->
<ul
id={menuId}
use:hasFocusDirective
use:floatingDirective
class="dropdown-menu show {$menuClassName$}"
Expand All @@ -107,8 +110,7 @@
<!-- svelte-ignore a11y-click-events-have-key-events -->
<li
class={`au-select-item dropdown-item position-relative ${$menuItemClassName$}`}
class:bg-primary={isHighlighted}
class:text-light={isHighlighted}
class:bg-text-bg-primary={isHighlighted}
class:selected={itemContext.selected}
on:click={() => widget.api.toggleItem(itemContext.item)}
>
Expand Down

0 comments on commit d54d780

Please sign in to comment.