From 2065a5954283fdcce45528aa00776ab849ef4b3a Mon Sep 17 00:00:00 2001 From: Vaadin Bot Date: Thu, 26 Jan 2023 20:06:48 +0100 Subject: [PATCH] fix: set aria-selected on selected item, not the focused one (#5402) (#5408) Co-authored-by: Serhii Kulykov --- .../combo-box/src/vaadin-combo-box-scroller.js | 14 +++++--------- packages/combo-box/test/aria.test.js | 7 ++++--- .../src/vaadin-multi-select-combo-box-scroller.js | 13 +++++-------- packages/time-picker/test/aria.test.js | 7 ++++--- 4 files changed, 18 insertions(+), 23 deletions(-) diff --git a/packages/combo-box/src/vaadin-combo-box-scroller.js b/packages/combo-box/src/vaadin-combo-box-scroller.js index defe1a6c6c..83a1f4bae9 100644 --- a/packages/combo-box/src/vaadin-combo-box-scroller.js +++ b/packages/combo-box/src/vaadin-combo-box-scroller.js @@ -216,18 +216,13 @@ export class ComboBoxScroller extends PolymerElement { return itemIndex !== undefined ? 'option' : false; } - /** @private */ - __getAriaSelected(focusedIndex, itemIndex) { - return this.__isItemFocused(focusedIndex, itemIndex).toString(); - } - /** @private */ __isItemFocused(focusedIndex, itemIndex) { return !this.loading && focusedIndex === itemIndex; } - /** @private */ - __isItemSelected(item, selectedItem, itemIdPath) { + /** @protected */ + _isItemSelected(item, selectedItem, itemIdPath) { if (item instanceof ComboBoxPlaceholder) { return false; } else if (itemIdPath && item !== undefined && selectedItem !== undefined) { @@ -291,19 +286,20 @@ export class ComboBoxScroller extends PolymerElement { __updateElement(el, index) { const item = this.items[index]; const focusedIndex = this.focusedIndex; + const selected = this._isItemSelected(item, this.selectedItem, this.itemIdPath); el.setProperties({ item, index, label: this.getItemLabel(item), - selected: this.__isItemSelected(item, this.selectedItem, this.itemIdPath), + selected, renderer: this.renderer, focused: this.__isItemFocused(focusedIndex, index), }); el.id = `${this.__hostTagName}-item-${index}`; el.setAttribute('role', this.__getAriaRole(index)); - el.setAttribute('aria-selected', this.__getAriaSelected(focusedIndex, index)); + el.setAttribute('aria-selected', selected.toString()); el.setAttribute('aria-posinset', index + 1); el.setAttribute('aria-setsize', this.items.length); diff --git a/packages/combo-box/test/aria.test.js b/packages/combo-box/test/aria.test.js index d7bbafde87..29176e19a9 100644 --- a/packages/combo-box/test/aria.test.js +++ b/packages/combo-box/test/aria.test.js @@ -43,11 +43,12 @@ describe('ARIA', () => { expect(input.getAttribute('aria-activedescendant')).to.equal(items[1].id); }); - it('should set aria-selected on item elements depending on the focused item', () => { - arrowDownKeyDown(input); // Move focus to the 1st item. + it('should set aria-selected on item elements depending on the selected item', () => { + comboBox.value = 'foo'; expect(items[0].getAttribute('aria-selected')).to.equal('true'); expect(items[1].getAttribute('aria-selected')).to.equal('false'); - arrowDownKeyDown(input); // Move focus to the 2nd item. + + comboBox.value = 'bar'; expect(items[0].getAttribute('aria-selected')).to.equal('false'); expect(items[1].getAttribute('aria-selected')).to.equal('true'); }); diff --git a/packages/multi-select-combo-box/src/vaadin-multi-select-combo-box-scroller.js b/packages/multi-select-combo-box/src/vaadin-multi-select-combo-box-scroller.js index bf9405f478..c39891b398 100644 --- a/packages/multi-select-combo-box/src/vaadin-multi-select-combo-box-scroller.js +++ b/packages/multi-select-combo-box/src/vaadin-multi-select-combo-box-scroller.js @@ -24,14 +24,11 @@ class MultiSelectComboBoxScroller extends ComboBoxScroller { this.setAttribute('aria-multiselectable', 'true'); } - /** @private */ - __getAriaSelected(_focusedIndex, itemIndex) { - const item = this.items[itemIndex]; - return this.__isItemSelected(item, null, this.itemIdPath).toString(); - } - - /** @private */ - __isItemSelected(item, _selectedItem, itemIdPath) { + /** + * @protected + * @override + */ + _isItemSelected(item, _selectedItem, itemIdPath) { if (item instanceof ComboBoxPlaceholder) { return false; } diff --git a/packages/time-picker/test/aria.test.js b/packages/time-picker/test/aria.test.js index 4611cdc7a0..eb422c13b7 100644 --- a/packages/time-picker/test/aria.test.js +++ b/packages/time-picker/test/aria.test.js @@ -41,11 +41,12 @@ describe('ARIA', () => { expect(input.getAttribute('aria-activedescendant')).to.equal(items[1].id); }); - it('should set aria-selected on item elements depending on the focused item', () => { - arrowDownKeyDown(input); // Move focus to the 1st item. + it('should set aria-selected on item elements depending on the selected item', () => { + timePicker.value = '00:00'; expect(items[0].getAttribute('aria-selected')).to.equal('true'); expect(items[1].getAttribute('aria-selected')).to.equal('false'); - arrowDownKeyDown(input); // Move focus to the 2nd item. + + timePicker.value = '01:00'; expect(items[0].getAttribute('aria-selected')).to.equal('false'); expect(items[1].getAttribute('aria-selected')).to.equal('true'); });