From bb1b8e9e10dfa23dd2b362461cb5be4da3a6b7df Mon Sep 17 00:00:00 2001 From: Calvin Lough Date: Wed, 17 Nov 2021 08:17:28 -0700 Subject: [PATCH 1/5] Update markup to make the component usable by screen readers --- addon/components/power-select.hbs | 5 ++++- addon/components/power-select.ts | 12 ++++++++++++ addon/components/power-select/options.hbs | 11 ++++++----- 3 files changed, 22 insertions(+), 6 deletions(-) diff --git a/addon/components/power-select.hbs b/addon/components/power-select.hbs index a4f1a862c..6ee3bfad2 100644 --- a/addon/components/power-select.hbs +++ b/addon/components/power-select.hbs @@ -38,12 +38,15 @@ {{on "focus" this.handleFocus}} {{on "blur" this.handleBlur}} class="ember-power-select-trigger {{@triggerClass}}{{if publicAPI.isActive " ember-power-select-trigger--active"}}" + aria-activedescendant={{concat publicAPI.uniqueId "-" this.highlightedIndex}} + aria-controls={{listboxId}} aria-describedby={{@ariaDescribedBy}} + aria-haspopup="listbox" aria-invalid={{@ariaInvalid}} aria-label={{@ariaLabel}} aria-labelledby={{@ariaLabelledBy}} aria-required={{@required}} - role={{or @triggerRole "button"}} + role={{or @triggerRole "combobox"}} title={{@title}} id={{@triggerId}} tabindex={{and (not @disabled) (or @tabindex "0")}} diff --git a/addon/components/power-select.ts b/addon/components/power-select.ts index 12cc926fb..6dab40e5a 100644 --- a/addon/components/power-select.ts +++ b/addon/components/power-select.ts @@ -147,6 +147,18 @@ export default class PowerSelect extends Component { get highlightOnHover(): boolean { return this.args.highlightOnHover === undefined ? true : this.args.highlightOnHover } + + get highlightedIndex(): number { + let results = this.results; + let highlighted = this.highlighted; + + if (results) { + return indexOfOption(results, highlighted); + } + + return 0; + } + get placeholderComponent(): string { return this.args.placeholderComponent || 'power-select/placeholder'; } diff --git a/addon/components/power-select/options.hbs b/addon/components/power-select/options.hbs index d8641a343..62b170cea 100644 --- a/addon/components/power-select/options.hbs +++ b/addon/components/power-select/options.hbs @@ -1,4 +1,4 @@ - + From 1e9524a12b24963f1ce26fa7ca9754aba909ab94 Mon Sep 17 00:00:00 2001 From: Calvin Lough Date: Wed, 17 Nov 2021 10:55:16 -0700 Subject: [PATCH 2/5] Add pathForOption function to compute paths of nested options --- addon/components/power-select.ts | 10 +++------- addon/utils/group-utils.ts | 20 ++++++++++++++++++++ 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/addon/components/power-select.ts b/addon/components/power-select.ts index 6dab40e5a..9ea224bd3 100644 --- a/addon/components/power-select.ts +++ b/addon/components/power-select.ts @@ -15,6 +15,7 @@ import { advanceSelectableOption, defaultMatcher, defaultTypeAheadMatcher, + pathForOption, MatcherFn } from '../utils/group-utils'; import { restartableTask } from 'ember-concurrency-decorators'; @@ -148,15 +149,10 @@ export default class PowerSelect extends Component { return this.args.highlightOnHover === undefined ? true : this.args.highlightOnHover } - get highlightedIndex(): number { + get highlightedIndex(): string { let results = this.results; let highlighted = this.highlighted; - - if (results) { - return indexOfOption(results, highlighted); - } - - return 0; + return pathForOption(results, highlighted); } get placeholderComponent(): string { diff --git a/addon/utils/group-utils.ts b/addon/utils/group-utils.ts index 6eb8a0d6e..34deefdd1 100644 --- a/addon/utils/group-utils.ts +++ b/addon/utils/group-utils.ts @@ -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 { From 0d443cfbb44c08eeb040e6844c7fe79a217184ab Mon Sep 17 00:00:00 2001 From: Calvin Lough Date: Wed, 17 Nov 2021 13:05:01 -0700 Subject: [PATCH 3/5] Update accessibility tests --- addon/components/power-select.hbs | 2 +- addon/components/power-select.ts | 2 +- addon/components/power-select/options.hbs | 2 +- .../components/power-select/a11y-test.js | 70 +++++++++++-------- .../power-select/mouse-control-test.js | 2 +- tests/unit/utils/group-utils-test.js | 21 +++++- 6 files changed, 64 insertions(+), 35 deletions(-) diff --git a/addon/components/power-select.hbs b/addon/components/power-select.hbs index 6ee3bfad2..8157ee1ea 100644 --- a/addon/components/power-select.hbs +++ b/addon/components/power-select.hbs @@ -46,7 +46,7 @@ aria-label={{@ariaLabel}} aria-labelledby={{@ariaLabelledBy}} aria-required={{@required}} - role={{or @triggerRole "combobox"}} + role={{or @triggerRole "button"}} title={{@title}} id={{@triggerId}} tabindex={{and (not @disabled) (or @tabindex "0")}} diff --git a/addon/components/power-select.ts b/addon/components/power-select.ts index 9ea224bd3..3fa5d6ba8 100644 --- a/addon/components/power-select.ts +++ b/addon/components/power-select.ts @@ -421,7 +421,7 @@ export default class PowerSelect extends Component { 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.querySelector(`#ember-power-select-options-${select.uniqueId}`) as HTMLElement; if (!optionsList) { return; } diff --git a/addon/components/power-select/options.hbs b/addon/components/power-select/options.hbs index 62b170cea..4b3b0b08c 100644 --- a/addon/components/power-select/options.hbs +++ b/addon/components/power-select/options.hbs @@ -28,7 +28,7 @@ aria-disabled={{if opt.disabled "true"}} aria-current="{{eq opt @select.highlighted}}" data-option-index="{{@groupIndex}}{{index}}" - role="option"> + role={{if (eq opt @select.highlighted) "alert" "option"}}> {{yield opt @select}} {{/if}} diff --git a/tests/integration/components/power-select/a11y-test.js b/tests/integration/components/power-select/a11y-test.js index dadb27192..8506d90b8 100644 --- a/tests/integration/components/power-select/a11y-test.js +++ b/tests/integration/components/power-select/a11y-test.js @@ -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) { @@ -333,34 +333,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=`', async function(assert) { - assert.expect(1); - - this.numbers = numbers; - await render(hbs` - - {{option}} - - `); - - 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=`', async function(assert) { - assert.expect(1); - - this.numbers = numbers; - await render(hbs` - - {{option}} - - `); - - 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
  • s inside an
      , and have an item with `role=button` with `aria-label="remove element"`', async function(assert) { assert.expect(12); @@ -478,4 +450,42 @@ 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('Trigger has proper aria attributes to associate it with the options', async function(assert) { + assert.expect(2); + this.numbers = numbers; + + await render(hbs` + + {{number}} + + `); + + await clickTrigger(); + + assert.dom('.ember-power-select-trigger').hasAttribute('aria-haspopup', 'listbox'); + assert.dom('.ember-power-select-trigger').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` + + {{number}} + + `); + + 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'); + }); }); diff --git a/tests/integration/components/power-select/mouse-control-test.js b/tests/integration/components/power-select/mouse-control-test.js index 16c05a7c1..d7c021847 100644 --- a/tests/integration/components/power-select/mouse-control-test.js +++ b/tests/integration/components/power-select/mouse-control-test.js @@ -193,7 +193,7 @@ module('Integration | Component | Ember Power Select (Mouse control)', function( `); await clickTrigger(); - await triggerEvent('ul', 'mouseover'); + await triggerEvent('.ember-power-select-options', 'mouseover'); }); }); diff --git a/tests/unit/utils/group-utils-test.js b/tests/unit/utils/group-utils-test.js index b2eab0566..96b862cb5 100644 --- a/tests/unit/utils/group-utils-test.js +++ b/tests/unit/utils/group-utils-test.js @@ -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 = [ @@ -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' }); From 0d9689088a238112b3749ebbb2b965ab15b9bac1 Mon Sep 17 00:00:00 2001 From: Calvin Lough Date: Thu, 18 Nov 2021 13:02:02 -0700 Subject: [PATCH 4/5] Pass empty aria-owns property to override the one from ember-basic-dropdown --- addon-test-support/index.js | 3 +-- addon/components/power-select.hbs | 1 + addon/components/power-select.ts | 2 +- addon/components/power-select/options.hbs | 8 ++++---- .../components/power-select/a11y-test.js | 13 ++++++------- .../components/power-select/mouse-control-test.js | 2 +- 6 files changed, 14 insertions(+), 15 deletions(-) diff --git a/addon-test-support/index.js b/addon-test-support/index.js index a558eaa01..b9c93132f 100644 --- a/addon-test-support/index.js +++ b/addon-test-support/index.js @@ -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; } diff --git a/addon/components/power-select.hbs b/addon/components/power-select.hbs index 8157ee1ea..095b7210c 100644 --- a/addon/components/power-select.hbs +++ b/addon/components/power-select.hbs @@ -45,6 +45,7 @@ aria-invalid={{@ariaInvalid}} aria-label={{@ariaLabel}} aria-labelledby={{@ariaLabelledBy}} + aria-owns="" aria-required={{@required}} role={{or @triggerRole "button"}} title={{@title}} diff --git a/addon/components/power-select.ts b/addon/components/power-select.ts index 3fa5d6ba8..ec4246f1a 100644 --- a/addon/components/power-select.ts +++ b/addon/components/power-select.ts @@ -421,7 +421,7 @@ export default class PowerSelect extends Component { if (this.args.scrollTo) { return this.args.scrollTo(option, select); } - let optionsList = document.querySelector(`#ember-power-select-options-${select.uniqueId}`) as HTMLElement; + let optionsList = document.getElementById(`ember-power-select-options-${select.uniqueId}`) as HTMLElement; if (!optionsList) { return; } diff --git a/addon/components/power-select/options.hbs b/addon/components/power-select/options.hbs index 4b3b0b08c..ff0f36890 100644 --- a/addon/components/power-select/options.hbs +++ b/addon/components/power-select/options.hbs @@ -1,4 +1,4 @@ -
      +
        {{! template-lint-disable no-unnecessary-concat }} {{#if @select.loading}} {{#if @loadingMessage}} @@ -22,7 +22,7 @@ {{else}} -
        {{yield opt @select}} -
        + {{/if}} {{/each}} {{/let}} -
      +
    diff --git a/tests/integration/components/power-select/a11y-test.js b/tests/integration/components/power-select/a11y-test.js index 8506d90b8..6de9ef329 100644 --- a/tests/integration/components/power-select/a11y-test.js +++ b/tests/integration/components/power-select/a11y-test.js @@ -215,8 +215,8 @@ module('Integration | Component | Ember Power Select (Accessibility)', function( 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=`', 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` @@ -227,11 +227,10 @@ module('Integration | Component | Ember Power Select (Accessibility)', function( 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=`', 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` @@ -242,7 +241,6 @@ module('Integration | Component | Ember Power Select (Accessibility)', function( 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) { @@ -452,7 +450,7 @@ module('Integration | Component | Ember Power Select (Accessibility)', function( }); test('Trigger has proper aria attributes to associate it with the options', async function(assert) { - assert.expect(2); + assert.expect(3); this.numbers = numbers; await render(hbs` @@ -464,6 +462,7 @@ module('Integration | Component | Ember Power Select (Accessibility)', function( 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); }); diff --git a/tests/integration/components/power-select/mouse-control-test.js b/tests/integration/components/power-select/mouse-control-test.js index d7c021847..16c05a7c1 100644 --- a/tests/integration/components/power-select/mouse-control-test.js +++ b/tests/integration/components/power-select/mouse-control-test.js @@ -193,7 +193,7 @@ module('Integration | Component | Ember Power Select (Mouse control)', function( `); await clickTrigger(); - await triggerEvent('.ember-power-select-options', 'mouseover'); + await triggerEvent('ul', 'mouseover'); }); }); From 11b6a8960293e83f4cab8a2cdd374f61d8423de5 Mon Sep 17 00:00:00 2001 From: Calvin Lough Date: Mon, 22 Nov 2021 09:57:22 -0700 Subject: [PATCH 5/5] When dropdown has a search box, aria properties should be on the search box instead of the trigger --- addon/components/power-select.hbs | 7 ++-- .../power-select/before-options.hbs | 4 ++- .../components/power-select/a11y-test.js | 35 +++++++++++++++++-- 3 files changed, 40 insertions(+), 6 deletions(-) diff --git a/addon/components/power-select.hbs b/addon/components/power-select.hbs index 095b7210c..91761b6e7 100644 --- a/addon/components/power-select.hbs +++ b/addon/components/power-select.hbs @@ -38,10 +38,10 @@ {{on "focus" this.handleFocus}} {{on "blur" this.handleBlur}} class="ember-power-select-trigger {{@triggerClass}}{{if publicAPI.isActive " ember-power-select-trigger--active"}}" - aria-activedescendant={{concat publicAPI.uniqueId "-" this.highlightedIndex}} - aria-controls={{listboxId}} + aria-activedescendant={{unless @searchEnabled (concat publicAPI.uniqueId "-" this.highlightedIndex)}} + aria-controls={{unless @searchEnabled listboxId}} aria-describedby={{@ariaDescribedBy}} - aria-haspopup="listbox" + aria-haspopup={{unless @searchEnabled "listbox"}} aria-invalid={{@ariaInvalid}} aria-label={{@ariaLabel}} aria-labelledby={{@ariaLabelledBy}} @@ -88,6 +88,7 @@ @placeholderComponent={{this.placeholderComponent}} @extra={{@extra}} @listboxId={{listboxId}} + @ariaActiveDescendant={{concat publicAPI.uniqueId "-" this.highlightedIndex}} @selectedItemComponent={{@selectedItemComponent}} @searchPlaceholder={{@searchPlaceholder}} /> diff --git a/addon/components/power-select/before-options.hbs b/addon/components/power-select/before-options.hbs index 14be9f0b9..9c9c5b25b 100644 --- a/addon/components/power-select/before-options.hbs +++ b/addon/components/power-select/before-options.hbs @@ -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}} @@ -14,4 +16,4 @@ {{did-insert this.focusInput}} {{will-destroy this.clearSearch}}> -{{/if}} \ No newline at end of file +{{/if}} diff --git a/tests/integration/components/power-select/a11y-test.js b/tests/integration/components/power-select/a11y-test.js index 6de9ef329..a7c3f2f2f 100644 --- a/tests/integration/components/power-select/a11y-test.js +++ b/tests/integration/components/power-select/a11y-test.js @@ -449,12 +449,18 @@ module('Integration | Component | Ember Power Select (Accessibility)', function( assert.dom('.ember-power-select-trigger').hasAttribute('role', 'button', 'The `role` was defaults to `button`.'); }); - test('Trigger has proper aria attributes to associate it with the options', async function(assert) { + 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` - + {{number}} `); @@ -466,6 +472,31 @@ module('Integration | Component | Ember Power Select (Accessibility)', function( 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` + + {{number}} + + `); + + 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;