Skip to content

Commit

Permalink
Merge pull request #1481 from calvin-fb/accessibility-improvements
Browse files Browse the repository at this point in the history
Improve ember-power-select screen reader accessibility
  • Loading branch information
cibernox authored Nov 22, 2021
2 parents 00498e6 + 11b6a89 commit 742200b
Show file tree
Hide file tree
Showing 8 changed files with 136 additions and 42 deletions.
3 changes: 1 addition & 2 deletions addon-test-support/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,11 @@ import { click, fillIn, settled } from '@ember/test-helpers';
import { warn } from '@ember/debug';

async function openIfClosedAndGetContentId(trigger) {
let contentId = trigger.attributes['aria-owns'] && `${trigger.attributes['aria-owns'].value}`;
let contentId = `ember-basic-dropdown-content-${trigger.getAttribute('data-ebd-id').replace('-trigger', '')}`
let content = contentId ? document.querySelector(`#${contentId}`) : undefined;
// If the dropdown is closed, open it
if (!content || content.classList.contains('ember-basic-dropdown-content-placeholder')) {
await click(trigger);
contentId = `${trigger.attributes['aria-owns'].value}`;
}
return contentId;
}
Expand Down
5 changes: 5 additions & 0 deletions addon/components/power-select.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,14 @@
{{on "focus" this.handleFocus}}
{{on "blur" this.handleBlur}}
class="ember-power-select-trigger {{@triggerClass}}{{if publicAPI.isActive " ember-power-select-trigger--active"}}"
aria-activedescendant={{unless @searchEnabled (concat publicAPI.uniqueId "-" this.highlightedIndex)}}
aria-controls={{unless @searchEnabled listboxId}}
aria-describedby={{@ariaDescribedBy}}
aria-haspopup={{unless @searchEnabled "listbox"}}
aria-invalid={{@ariaInvalid}}
aria-label={{@ariaLabel}}
aria-labelledby={{@ariaLabelledBy}}
aria-owns=""
aria-required={{@required}}
role={{or @triggerRole "button"}}
title={{@title}}
Expand Down Expand Up @@ -84,6 +88,7 @@
@placeholderComponent={{this.placeholderComponent}}
@extra={{@extra}}
@listboxId={{listboxId}}
@ariaActiveDescendant={{concat publicAPI.uniqueId "-" this.highlightedIndex}}
@selectedItemComponent={{@selectedItemComponent}}
@searchPlaceholder={{@searchPlaceholder}}
/>
Expand Down
10 changes: 9 additions & 1 deletion addon/components/power-select.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
advanceSelectableOption,
defaultMatcher,
defaultTypeAheadMatcher,
pathForOption,
MatcherFn
} from '../utils/group-utils';
import { restartableTask } from 'ember-concurrency-decorators';
Expand Down Expand Up @@ -147,6 +148,13 @@ export default class PowerSelect extends Component<PowerSelectArgs> {
get highlightOnHover(): boolean {
return this.args.highlightOnHover === undefined ? true : this.args.highlightOnHover
}

get highlightedIndex(): string {
let results = this.results;
let highlighted = this.highlighted;
return pathForOption(results, highlighted);
}

get placeholderComponent(): string {
return this.args.placeholderComponent || 'power-select/placeholder';
}
Expand Down Expand Up @@ -413,7 +421,7 @@ export default class PowerSelect extends Component<PowerSelectArgs> {
if (this.args.scrollTo) {
return this.args.scrollTo(option, select);
}
let optionsList = document.querySelector(`[aria-controls="ember-power-select-trigger-${select.uniqueId}"]`) as HTMLElement;
let optionsList = document.getElementById(`ember-power-select-options-${select.uniqueId}`) as HTMLElement;
if (!optionsList) {
return;
}
Expand Down
4 changes: 3 additions & 1 deletion addon/components/power-select/before-options.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
spellcheck={{false}} role="combobox"
class="ember-power-select-search-input"
value={{@select.searchText}}
aria-activedescendant={{@ariaActiveDescendant}}
aria-controls={{@listboxId}}
aria-haspopup="listbox"
placeholder={{@searchPlaceholder}}
{{on "input" @onInput}}
{{on "focus" @onFocus}}
Expand All @@ -14,4 +16,4 @@
{{did-insert this.focusInput}}
{{will-destroy this.clearSearch}}>
</div>
{{/if}}
{{/if}}
3 changes: 2 additions & 1 deletion addon/components/power-select/options.hbs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<ul role="listbox" aria-controls="ember-power-select-trigger-{{@select.uniqueId}}" {{did-insert this.addHandlers}} {{will-destroy this.removeHandlers}} ...attributes>
<ul role="listbox" {{did-insert this.addHandlers}} {{will-destroy this.removeHandlers}} ...attributes>
{{! template-lint-disable no-unnecessary-concat }}
{{#if @select.loading}}
{{#if @loadingMessage}}
Expand All @@ -23,6 +23,7 @@
</Group>
{{else}}
<li class="ember-power-select-option"
id="{{@select.uniqueId}}-{{@groupIndex}}{{index}}"
aria-selected="{{ember-power-select-is-selected opt @select.selected}}"
aria-disabled={{if opt.disabled "true"}}
aria-current="{{eq opt @select.highlighted}}"
Expand Down
20 changes: 20 additions & 0 deletions addon/utils/group-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,26 @@ export function indexOfOption(collection: any, option: any): number {
})(collection);
}

export function pathForOption(collection: any, option: any): string {
return (function walk(collection): string {
if (!collection) {
return '';
}
for (let i = 0; i < get(collection, 'length'); i++) {
let entry = collection.objectAt ? collection.objectAt(i) : collection[i];
if (isGroup(entry)) {
let result = walk(get(entry, 'options'));
if (result.length > 0) {
return i + '.' + result;
}
} else if (entry === option) {
return i + '';
}
}
return '';
})(collection);
}

export function optionAtIndex(originalCollection: any, index: number): { disabled: boolean, option: any } {
let counter = 0;
return (function walk(collection, ancestorIsDisabled): { disabled: boolean, option: any } | void {
Expand Down
112 changes: 76 additions & 36 deletions tests/integration/components/power-select/a11y-test.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import { module, test } from 'qunit';
import { setupRenderingTest } from 'ember-qunit';
import { render, triggerKeyEvent } from '@ember/test-helpers';
import { render, triggerEvent, triggerKeyEvent } from '@ember/test-helpers';
import { hbs } from 'ember-cli-htmlbars';
import { numbers, groupedNumbers, countriesWithDisabled } from '../constants';
import { clickTrigger, findContains } from 'ember-power-select/test-support/helpers';

module('Integration | Component | Ember Power Select (Accesibility)', function(hooks) {
module('Integration | Component | Ember Power Select (Accessibility)', function(hooks) {
setupRenderingTest(hooks);

test('Single-select: The top-level options list have `role=listbox` and nested lists have `role=group`', async function(assert) {
Expand Down Expand Up @@ -215,8 +215,8 @@ module('Integration | Component | Ember Power Select (Accesibility)', function(h
assert.dom('.ember-power-select-option[aria-disabled=true]').exists({ count: 3 }, 'Three of them are disabled');
});

test('Single-select: The trigger has `role=button` and `aria-owns=<id-of-dropdown>`', async function(assert) {
assert.expect(2);
test('Single-select: The trigger has `role=button`', async function(assert) {
assert.expect(1);

this.numbers = numbers;
await render(hbs`
Expand All @@ -227,11 +227,10 @@ module('Integration | Component | Ember Power Select (Accesibility)', function(h

await clickTrigger();
assert.dom('.ember-power-select-trigger').hasAttribute('role', 'button', 'The trigger has role button');
assert.dom('.ember-power-select-trigger').hasAttribute('aria-owns', /^ember-basic-dropdown-content-ember\d+$/, 'aria-owns points to the dropdown');
});

test('Multiple-select: The trigger has `role=button` and `aria-owns=<id-of-dropdown>`', async function(assert) {
assert.expect(2);
test('Multiple-select: The trigger has `role=button`', async function(assert) {
assert.expect(1);

this.numbers = numbers;
await render(hbs`
Expand All @@ -242,7 +241,6 @@ module('Integration | Component | Ember Power Select (Accesibility)', function(h

await clickTrigger();
assert.dom('.ember-power-select-trigger').hasAttribute('role', 'button', 'The trigger has role button');
assert.dom('.ember-power-select-trigger').hasAttribute('aria-owns', /^ember-basic-dropdown-content-ember\d+$/, 'aria-owns points to the dropdown');
});

test('Single-select: The trigger attribute `aria-expanded` is true when the dropdown is opened', async function(assert) {
Expand Down Expand Up @@ -333,34 +331,6 @@ module('Integration | Component | Ember Power Select (Accesibility)', function(h
assert.dom('.ember-power-select-trigger-multiple-input').hasAttribute('aria-controls', /^ember-power-select-options-ember\d+$/, 'The `aria-controls` points to the id of the listbox');
});

test('Single-select: The listbox has `aria-controls=<id-of-the-trigger>`', async function(assert) {
assert.expect(1);

this.numbers = numbers;
await render(hbs`
<PowerSelect @options={{this.numbers}} @selected={{this.selected}} @onChange={{action (mut this.foo)}} as |option|>
{{option}}
</PowerSelect>
`);

await clickTrigger();
assert.dom('.ember-power-select-options').hasAttribute('aria-controls', /^ember-power-select-trigger-ember\d+$/, 'The listbox controls the trigger');
});

test('Multiple-select: The listbox has `aria-controls=<id-of-the-trigger>`', async function(assert) {
assert.expect(1);

this.numbers = numbers;
await render(hbs`
<PowerSelectMultiple @options={{this.numbers}} @selected={{this.selected}} @onChange={{action (mut this.foo)}} as |option|>
{{option}}
</PowerSelectMultiple>
`);

await clickTrigger();
assert.dom('.ember-power-select-options').hasAttribute('aria-controls', /^ember-power-select-trigger-ember\d+$/, 'The listbox controls the trigger');
});

test('Multiple-select: The selected elements are <li>s inside an <ul>, and have an item with `role=button` with `aria-label="remove element"`', async function(assert) {
assert.expect(12);

Expand Down Expand Up @@ -478,4 +448,74 @@ module('Integration | Component | Ember Power Select (Accesibility)', function(h
this.set('role', undefined);
assert.dom('.ember-power-select-trigger').hasAttribute('role', 'button', 'The `role` was defaults to `button`.');
});

test('Dropdown with search disabled has proper aria attributes to associate trigger with the options', async function(assert) {
assert.expect(3);
this.numbers = numbers;

await render(hbs`
<PowerSelect
@options={{this.numbers}}
@selected={{this.selected}}
@searchEnabled={{false}}
@onChange={{action (mut this.selected)}}
as |number|
>
{{number}}
</PowerSelect>
`);

await clickTrigger();

assert.dom('.ember-power-select-trigger').hasAttribute('aria-haspopup', 'listbox');
assert.dom('.ember-power-select-trigger').hasAttribute('aria-owns', '');
assert.dom('.ember-power-select-trigger').hasAttribute('aria-controls', document.querySelector('.ember-power-select-options').id);
});

test('Dropdown with search enabled has proper aria attributes to associate search box with the options', async function(assert) {
assert.expect(5);
this.numbers = numbers;

await render(hbs`
<PowerSelect
@options={{this.numbers}}
@selected={{this.selected}}
@searchEnabled={{true}}
@onChange={{action (mut this.selected)}}
as |number|
>
{{number}}
</PowerSelect>
`);

await clickTrigger();

assert.dom('.ember-power-select-trigger').hasNoAttribute('aria-haspopup');
assert.dom('.ember-power-select-trigger').hasNoAttribute('aria-controls');
assert.dom('.ember-power-select-search-input').hasAttribute('role', 'combobox');
assert.dom('.ember-power-select-search-input').hasAttribute('aria-haspopup', 'listbox');
assert.dom('.ember-power-select-search-input').hasAttribute('aria-controls', document.querySelector('.ember-power-select-options').id);
});

test('Trigger has aria-activedescendant attribute for the highlighted option', async function(assert) {
assert.expect(4);
this.numbers = numbers;

await render(hbs`
<PowerSelect @options={{this.numbers}} @selected={{this.selected}} @onChange={{action (mut this.selected)}} as |number|>
{{number}}
</PowerSelect>
`);

await clickTrigger();

// by default, the first option is highlighted and marked as aria-activedescendant
assert.dom('.ember-power-select-option').hasAttribute('aria-current', 'true', 'The first element is highlighted');
assert.dom('.ember-power-select-trigger').hasAttribute('aria-activedescendant', document.querySelector('.ember-power-select-option:nth-child(1)').id, 'The first element is the aria-activedescendant');

await triggerEvent('.ember-power-select-option:nth-child(4)', 'mouseover');

assert.dom('.ember-power-select-option:nth-child(4)').hasAttribute('aria-current', 'true', 'The 4th element is highlighted');
assert.dom('.ember-power-select-trigger').hasAttribute('aria-activedescendant', document.querySelector('.ember-power-select-option:nth-child(4)').id, 'The 4th element is the aria-activedescendant');
});
});
21 changes: 20 additions & 1 deletion tests/unit/utils/group-utils-test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { isGroup, indexOfOption, optionAtIndex, filterOptions, stripDiacritics, countOptions, defaultTypeAheadMatcher } from 'ember-power-select/utils/group-utils';
import { isGroup, indexOfOption, pathForOption, optionAtIndex, filterOptions, stripDiacritics, countOptions, defaultTypeAheadMatcher } from 'ember-power-select/utils/group-utils';
import { module, test } from 'qunit';

const groupedOptions = [
Expand Down Expand Up @@ -58,6 +58,25 @@ module('Unit | Utility | Group utils', function() {
assert.equal(indexOfOption(groupedOptions, null), -1);
});

test('#pathForOption works for simple lists with no nesting', function(assert) {
assert.equal(pathForOption(null, null), '');
assert.equal(pathForOption(basicOptions, null), '');
assert.equal(pathForOption(basicOptions, ''), '');
assert.equal(pathForOption(basicOptions, 'non-existant option'), '');
assert.equal(pathForOption(basicOptions, 'zero'), '0');
assert.equal(pathForOption(basicOptions, 'one'), '1');
assert.equal(pathForOption(basicOptions, 'five'), '5');
});

test('#pathForOption works for nested lists', function(assert) {
assert.equal(pathForOption(groupedOptions, 'zero'), '0.0');
assert.equal(pathForOption(groupedOptions, 'four'), '1.0');
assert.equal(pathForOption(groupedOptions, 'seven'), '2.0.0');
assert.equal(pathForOption(groupedOptions, 'ten'), '2.1.0');
assert.equal(pathForOption(groupedOptions, 'one hundred'), '3');
assert.equal(pathForOption(groupedOptions, 'one thousand'), '4');
});

test('#optionAtIndex returns an object `{ disabled, option }`, disabled being true if that option or any ancestor is disabled, and the option will be undefined if the index is out of range', function(assert) {
assert.deepEqual(optionAtIndex(basicOptions, 0), { disabled: false, option: 'zero' });
assert.deepEqual(optionAtIndex(basicOptions, 5), { disabled: false, option: 'five' });
Expand Down

0 comments on commit 742200b

Please sign in to comment.