fix: set aria-selected on selected item, not the focused one (#5402) (#…

…5408) Co-authored-by: Serhii Kulykov <iamkulykov@gmail.com>
vaadin · Jan 26, 2023 · 2065a59 · 2065a59
1 parent 855bd71
commit 2065a59
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 23 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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');
     });