Skip to content

Commit

Permalink
fix(docs-infra): improve accessibility of aio-select component (#46013)
Browse files Browse the repository at this point in the history
improve the accessibility of the aio-select component so that it is
clear for screen reader users its functionality (currently it is
presented as a simple button), following the WAI-ARIA authoring
practices (see: https://www.w3.org/TR/wai-aria-practices/#combobox)

A first attempt in improving the accessibility of the component has been
tried in PR #45937 by replacing it with the material select component,
such implementation has however been scrapped since the increase of
payload sizes has proven prohibitively large

(also note that given native select elements haven't been used given the lack
of syling options for such elements)

PR Close #46013
  • Loading branch information
dario-piotrowicz authored and jessicajaniuk committed Jun 10, 2022
1 parent cac6ef7 commit b60d15c
Show file tree
Hide file tree
Showing 7 changed files with 204 additions and 50 deletions.
2 changes: 1 addition & 1 deletion aio/src/app/app.component.html
Expand Up @@ -64,7 +64,7 @@
<aio-nav-menu [nodes]="sideNavNodes" [currentNode]="currentNodes?.SideNav" [isWide]="dockSideNav" navLabel="guides and docs"></aio-nav-menu>

<div class="doc-version">
<aio-select (change)="onDocVersionChange($event.index)" [options]="docVersions" [selected]="currentDocVersion"></aio-select>
<aio-select (optionsToggled)="scrollSelectIntoView()" (change)="onDocVersionChange($event.index)" [options]="docVersions" [selected]="currentDocVersion"></aio-select>
</div>
</mat-sidenav>

Expand Down
12 changes: 12 additions & 0 deletions aio/src/app/app.component.ts
Expand Up @@ -22,6 +22,7 @@ import { TocService } from 'app/shared/toc.service';
import { SwUpdatesService } from 'app/sw-updates/sw-updates.service';
import { BehaviorSubject, combineLatest, Observable } from 'rxjs';
import { first, map } from 'rxjs/operators';
import { SelectComponent } from './shared/select/select.component';

const sideNavView = 'SideNav';
export const showTopMenuWidth = 1150;
Expand Down Expand Up @@ -116,6 +117,9 @@ export class AppComponent implements OnInit {

@ViewChildren('themeToggle, externalIcons', { read: ElementRef }) toolbarIcons: QueryList<ElementRef>;

@ViewChild(SelectComponent, { read: ElementRef })
docVersionSelectElement: ElementRef;

constructor(
public deployment: Deployment,
private documentService: DocumentService,
Expand Down Expand Up @@ -477,4 +481,12 @@ export class AppComponent implements OnInit {
}
}
}

scrollSelectIntoView() {
// this method is used to scroll the select component into view when it is clicked/opened
// the setTimeout is needed so that the scroll happens after the component has expanded
setTimeout(() =>
this.docVersionSelectElement.nativeElement?.scrollIntoView({behavior: 'smooth'})
);
}
}
37 changes: 27 additions & 10 deletions aio/src/app/shared/select/select.component.html
@@ -1,16 +1,33 @@
<div class="form-select-menu">
<button class="form-select-button" (click)="toggleOptions()" [disabled]="disabled">
<span><strong>{{label}}</strong></span><span *ngIf="showSymbol" class="symbol {{selected?.value}}"></span><span>{{selected?.title}}</span>
</button>
<ul class="form-select-dropdown" *ngIf="showOptions">
<div class="form-select-button"
role="combobox"
[attr.aria-controls]="listBoxId"
aria-haspopup="listbox"
(click)="toggleOptions()"
(keydown)="handleKeydown($event)"
[attr.aria-expanded]="showOptions"
[attr.aria-activedescendant]="currentOptionIdx > -1 && showOptions ? listBoxId + '-option-' + currentOptionIdx : null"
[attr.aria-label]="label + (selected?.title ?? '')"
[class.disabled]="disabled"
tabindex="0"
>
<div aria-hidden="true">
<ng-container *ngTemplateOutlet="optionTemplate; context: { showLabel: true, value: selected?.value, title: selected?.title }"></ng-container>
</div>
</div>
<ul class="form-select-dropdown" *ngIf="showOptions" [id]="listBoxId" role="listbox" tabIndex="-1" #listBox>
<!-- eslint-disable-next-line @angular-eslint/template/click-events-have-key-events -- the key events are handled in the ts class -->
<li *ngFor="let option of options; index as i"
role="option"
[class.selected]="option === selected"
role="button"
tabindex="0"
(click)="select(option, i)"
(keydown.enter)="select(option, i)"
(keydown.space)="select(option, i); $event.preventDefault()">
<span *ngIf="showSymbol" class="symbol {{option.value}}"></span><span>{{option.title}}</span>
[attr.aria-selected]="option === selected"
[class.current]="currentOptionIdx === i"
[id]="listBoxId + '-option-' + i"
(click)="select(i)">
<ng-container *ngTemplateOutlet="optionTemplate; context: { showLabel: false, value: option.value, title: option.title }"></ng-container>
</li>
</ul>
</div>
<ng-template #optionTemplate let-showLabel="showLabel" let-value="value" let-title="title">
<span *ngIf="showLabel"><strong>{{label}}</strong></span><span *ngIf="showSymbol" class="symbol {{value}}"></span><span>{{title}}</span>
</ng-template>
124 changes: 96 additions & 28 deletions aio/src/app/shared/select/select.component.spec.ts
Expand Up @@ -5,12 +5,14 @@ import { SelectComponent, Option } from './select.component';

const options = [
{ title: 'Option A', value: 'option-a' },
{ title: 'Option B', value: 'option-b' }
{ title: 'Option B', value: 'option-b' },
{ title: 'Option C', value: 'option-c' },
];

let host: HostComponent;
let fixture: ComponentFixture<HostComponent>;
let element: DebugElement;
let component: SelectComponent;

describe('SelectComponent', () => {

Expand All @@ -24,6 +26,7 @@ describe('SelectComponent', () => {
fixture = TestBed.createComponent(HostComponent);
host = fixture.componentInstance;
element = fixture.debugElement.query(By.directive(SelectComponent));
component = element.componentInstance;
});

describe('(initially)', () => {
Expand Down Expand Up @@ -69,13 +72,13 @@ describe('SelectComponent', () => {
it('should be disabled if the component is disabled', () => {
host.options = options;
fixture.detectChanges();
expect(getButton().disabled).toBe(false);
expect(getButton().getAttribute('disabled')).toBe(null);
expect(component.disabled).toBeFalsy();
expect(getButton().classList).not.toContain('disabled');

host.disabled = true;
fixture.detectChanges();
expect(getButton().disabled).toBe(true);
expect(getButton().getAttribute('disabled')).toBeDefined();
expect(component.disabled).toBeTruthy();
expect(getButton().classList).toContain('disabled');
});

it('should not toggle the visibility of the options list if disabled', () => {
Expand Down Expand Up @@ -111,24 +114,6 @@ describe('SelectComponent', () => {
expect(getButtonSymbol()?.className).toContain(options[0].value);
});

it('should select the current option when enter is pressed', () => {
const e = new KeyboardEvent('keydown', {bubbles: true, cancelable: true, key: 'Enter'});
getOptions()[0].dispatchEvent(e);
fixture.detectChanges();
expect(host.onChange).toHaveBeenCalledWith({ option: options[0], index: 0 });
expect(getButton().textContent).toContain(options[0].title);
expect(getButtonSymbol()?.className).toContain(options[0].value);
});

it('should select the current option when space is pressed', () => {
const e = new KeyboardEvent('keydown', {bubbles: true, cancelable: true, key: ' '});
getOptions()[0].dispatchEvent(e);
fixture.detectChanges();
expect(host.onChange).toHaveBeenCalledWith({ option: options[0], index: 0 });
expect(getButton().textContent).toContain(options[0].title);
expect(getButtonSymbol()?.className).toContain(options[0].value);
});

it('should hide when an option is clicked', () => {
getOptions()[0].click();
fixture.detectChanges();
Expand All @@ -140,12 +125,91 @@ describe('SelectComponent', () => {
fixture.detectChanges();
expect(getOptionContainer()).toEqual(null);
});
});

it('should hide if the escape button is pressed', () => {
const e = new KeyboardEvent('keydown', { bubbles: true, cancelable: true, key: 'Escape' });
document.dispatchEvent(e);
describe('keyboard navigation', () => {
const openOptions = () => {
component.showOptions = true;
fixture.detectChanges();
expect(getOptionContainer()).toEqual(null);
};

const pressKey = (key: string) => {
const debugBtnElement = fixture.debugElement.query(By.css('.form-select-button'));
debugBtnElement.triggerEventHandler('keydown', { bubbles: true, cancelable: true, key, preventDefault(){} });
fixture.detectChanges();
};

const printKey = (key: string) => key === ' ' ? "' '" : key;

['ArrowDown', 'ArrowUp', 'Enter', 'Space', ' '].forEach(key =>
it(`should open the options list when the ${printKey(key)} key is pressed`, () => {
expect(getOptionContainer()).toBeFalsy();
pressKey(key);
expect(getOptionContainer()).toBeTruthy();
})
);

['Escape', 'Tab'].forEach(key =>
it(`should close the options list when the ${printKey(key)} key is pressed`, () => {
host.options = options;
openOptions();
expect(getOptionContainer()).toBeTruthy();
pressKey(key);
expect(getOptionContainer()).toBeFalsy();
})
);

['Enter', 'Space', ' '].forEach(key =>
it(`should select the current option when the ${printKey(key)} key is pressed`, () => {
component.currentOptionIdx = 0;
host.showSymbol = true;
host.options = options;
openOptions();
expect(getButton().textContent).not.toContain(options[0].title);
expect(getButtonSymbol()?.className).not.toContain(options[0].value);
pressKey(key);
expect(host.onChange).toHaveBeenCalledWith({ option: options[0], index: 0 });
expect(getButton().textContent).toContain(options[0].title);
expect(getButtonSymbol()?.className).toContain(options[0].value);
})
);

it('should move to the next option when the ArrowDown key is pressed', () => {
component.currentOptionIdx = 1;
host.options = options;
openOptions();
pressKey('ArrowDown');
expect(component.currentOptionIdx).toEqual(2);
expect(getCurrentOption().textContent).toEqual('Option C');
});

it('should move to the previous option when the ArrowUp key is pressed', () => {
component.currentOptionIdx = 1;
host.options = options;
openOptions();
pressKey('ArrowUp');
expect(component.currentOptionIdx).toEqual(0);
expect(getCurrentOption().textContent).toEqual('Option A');
});

it('should do nothing when the ArrowDown key is pressed and the current option is the last', () => {
component.currentOptionIdx = 2;
host.options = options;
openOptions();
pressKey('ArrowDown');
expect(component.currentOptionIdx).toEqual(2);
expect(getCurrentOption().textContent).toEqual('Option C');
expect(getOptionContainer()).toBeTruthy();
});

it('should do nothing when the ArrowUp key is pressed and the current option is the first', () => {
component.currentOptionIdx = 0;
host.options = options;
openOptions();
pressKey('ArrowUp');
expect(component.currentOptionIdx).toEqual(0);
expect(getCurrentOption().textContent).toEqual('Option A');
expect(getOptionContainer()).toBeTruthy();
});
});
});
Expand All @@ -172,7 +236,7 @@ class HostComponent {
}

function getButton(): HTMLButtonElement {
return element.query(By.css('button')).nativeElement;
return element.query(By.css('.form-select-button')).nativeElement;
}

function getButtonSymbol(): HTMLElement | null {
Expand All @@ -187,3 +251,7 @@ function getOptionContainer(): HTMLUListElement|null {
function getOptions(): HTMLLIElement[] {
return element.queryAll(By.css('li')).map(de => de.nativeElement);
}

function getCurrentOption(): HTMLElement {
return element.query(By.css('[role=option].current')).nativeElement;
}
71 changes: 64 additions & 7 deletions aio/src/app/shared/select/select.component.ts
@@ -1,4 +1,4 @@
import { Component, ElementRef, EventEmitter, HostListener, Input, Output, OnInit } from '@angular/core';
import { Component, ElementRef, EventEmitter, HostListener, Input, Output, OnInit, ViewChild } from '@angular/core';

export interface Option {
title: string;
Expand All @@ -10,20 +10,43 @@ export interface Option {
templateUrl: 'select.component.html'
})
export class SelectComponent implements OnInit {
static instancesCounter = 0;

@Input() selected: Option;

@Input() options: Option[];

// eslint-disable-next-line @angular-eslint/no-output-native
@Output() change = new EventEmitter<{option: Option, index: number}>();

@Output() optionsToggled = new EventEmitter<boolean>();

@Input() showSymbol = false;

@Input() label: string;
@Input() label!: string;

@Input() disabled: boolean;

showOptions = false;
@ViewChild('listBox', { read: ElementRef }) listBox: ElementRef;

private _showOptions = false;

get showOptions() {
return this._showOptions;
}

set showOptions(showOptions: boolean) {
if(!this.disabled) {
if(this.showOptions !== showOptions) {
this.optionsToggled.emit(showOptions);
}
this._showOptions = showOptions;
}
}

listBoxId = `aio-select-list-box-${SelectComponent.instancesCounter++}`;

currentOptionIdx = 0;

constructor(private hostElement: ElementRef) {}

Expand All @@ -39,7 +62,8 @@ export class SelectComponent implements OnInit {
this.showOptions = false;
}

select(option: Option, index: number) {
select(index: number) {
const option = this.options[index];
this.selected = option;
this.change.emit({option, index});
this.hideOptions();
Expand All @@ -53,8 +77,41 @@ export class SelectComponent implements OnInit {
}
}

@HostListener('document:keydown.escape')
onKeyDown() {
this.hideOptions();
handleKeydown(event: KeyboardEvent) {
const runOrOpenOptions = (fn: () => void): void => {
if(!this.showOptions) {
this.showOptions = true;
const indexOfSelected = (this.options ?? []).indexOf(this.selected);
this.currentOptionIdx = indexOfSelected > 0 ? indexOfSelected : 0;
} else {
fn();
}
};
switch(event.key) {
case 'ArrowDown':
runOrOpenOptions(() =>
this.currentOptionIdx = Math.min(this.currentOptionIdx + 1, (this.options?.length ?? 0) - 1)
);
break;
case 'ArrowUp':
runOrOpenOptions(() => this.currentOptionIdx = Math.max(this.currentOptionIdx - 1, 0));
break;
case 'Escape':
this.hideOptions();
break;
case 'Tab':
if(this.showOptions) {
this.select(this.currentOptionIdx);
}
break;
case 'Enter':
case 'Space':
case ' ':
runOrOpenOptions(() => this.select(this.currentOptionIdx));
break;
}
if(event.key !== 'Tab') {
event.preventDefault();
}
}
}
4 changes: 2 additions & 2 deletions aio/src/styles/2-modules/select-menu/_select-menu-theme.scss
Expand Up @@ -18,7 +18,7 @@
box-shadow: 0 2px 2px rgba(constants.$blue-400, 0.24), 0 0 2px rgba(constants.$blue-400, 0.12);
}

&[disabled] {
&.disabled {
color: lightgrey;
}
}
Expand All @@ -28,7 +28,7 @@
box-shadow: 0 16px 16px rgba(constants.$black, 0.24), 0 0 16px rgba(constants.$black, 0.12);

li {
&:hover {
&:hover, &.current {
background-color: if($is-dark-theme, constants.$darkgray, constants.$blue-grey-50);
color: f($is-dark-theme, constants.$blue-grey-400, constants.$blue-grey-500);
}
Expand Down

0 comments on commit b60d15c

Please sign in to comment.