From bc0d6d219d83651889fc5d31aafb8fd3f1effb41 Mon Sep 17 00:00:00 2001
From: Jeff Sagal <sagalbot@gmail.com>
Date: Wed, 4 Mar 2020 20:08:18 -0800
Subject: [PATCH] fix: Add and update WAI-ARIA 1.1 combobox propeties (#1068)

* Add aria 1.1 combobox properties

* Update dropdown test to account for v-show changes

* test: fix dropdown tests

* test: fix pointer scroll warnings

* refactor: implement uniqueId function

* fix: close #1072

remove empty button

Co-authored-by: Jeff Sagal <sagalbot@gmail.com>
---
 src/components/Select.vue           | 54 +++++++++++++++++------------
 src/mixins/pointerScroll.js         |  2 +-
 src/utility/uniqueId.js             | 12 +++++++
 tests/unit/Dropdown.spec.js         |  9 +++--
 tests/unit/utility/uniqueId.spec.js |  5 +++
 5 files changed, 55 insertions(+), 27 deletions(-)
 create mode 100644 src/utility/uniqueId.js
 create mode 100644 tests/unit/utility/uniqueId.spec.js

diff --git a/src/components/Select.vue b/src/components/Select.vue
index 62f0a22..fa0bce6 100644
--- a/src/components/Select.vue
+++ b/src/components/Select.vue
@@ -4,7 +4,7 @@
 
 <template>
   <div :dir="dir" class="v-select" :class="stateClasses">
-    <div ref="toggle" @mousedown.prevent="toggleDropdown" class="vs__dropdown-toggle">
+    <div :id="`vs${uid}__combobox`" ref="toggle" @mousedown.prevent="toggleDropdown" class="vs__dropdown-toggle" role="combobox" :aria-expanded="dropdownOpen.toString()" :aria-owns="`vs${uid}__listbox`" aria-label="Search for option">
 
       <div class="vs__selected-options" ref="selectedOptions">
         <slot v-for="option in selectedValue"
@@ -17,7 +17,7 @@
             <slot name="selected-option" v-bind="normalizeOptionForSlot(option)">
               {{ getOptionLabel(option) }}
             </slot>
-            <button v-if="multiple" :disabled="disabled" @click="deselect(option)" type="button" class="vs__deselect" aria-label="Deselect option" ref="deselectButtons">
+            <button v-if="multiple" :disabled="disabled" @click="deselect(option)" type="button" class="vs__deselect" :title="`Deselect ${getOptionLabel(option)}`" :aria-label="`Deselect ${getOptionLabel(option)}`" ref="deselectButtons">
               <component :is="childComponents.Deselect" />
             </button>
           </span>
@@ -35,7 +35,8 @@
           @click="clearSelection"
           type="button"
           class="vs__clear"
-          title="Clear selection"
+          title="Clear Selected"
+          aria-label="Clear Selected"
           ref="clearButton"
         >
           <component :is="childComponents.Deselect" />
@@ -52,23 +53,27 @@
     </div>
 
     <transition :name="transition">
-      <ul ref="dropdownMenu" v-if="dropdownOpen" class="vs__dropdown-menu" role="listbox" @mousedown.prevent="onMousedown" @mouseup="onMouseUp">
-        <li
-          role="option"
-          v-for="(option, index) in filteredOptions"
-          :key="getOptionKey(option)"
-          class="vs__dropdown-option"
-          :class="{ 'vs__dropdown-option--selected': isOptionSelected(option), 'vs__dropdown-option--highlight': index === typeAheadPointer, 'vs__dropdown-option--disabled': !selectable(option) }"
-          @mouseover="selectable(option) ? typeAheadPointer = index : null"
-          @mousedown.prevent.stop="selectable(option) ? select(option) : null"
-        >
-          <slot name="option" v-bind="normalizeOptionForSlot(option)">
-            {{ getOptionLabel(option) }}
-          </slot>
-        </li>
-        <li v-if="!filteredOptions.length" class="vs__no-options" @mousedown.stop="">
-          <slot name="no-options">Sorry, no matching options.</slot>
-        </li>
+      <ul ref="dropdownMenu" v-show="dropdownOpen" :id="`vs${uid}__listbox`" class="vs__dropdown-menu" role="listbox" @mousedown.prevent="onMousedown" @mouseup="onMouseUp">
+        <template v-if="dropdownOpen">
+          <li
+            role="option"
+            v-for="(option, index) in filteredOptions"
+            :key="getOptionKey(option)"
+            :id="`vs${uid}__option-${index}`"
+            class="vs__dropdown-option"
+            :class="{ 'vs__dropdown-option--selected': isOptionSelected(option), 'vs__dropdown-option--highlight': index === typeAheadPointer, 'vs__dropdown-option--disabled': !selectable(option) }"
+            :aria-selected="index === typeAheadPointer ? true : null"
+            @mouseover="selectable(option) ? typeAheadPointer = index : null"
+            @mousedown.prevent.stop="selectable(option) ? select(option) : null"
+          >
+            <slot name="option" v-bind="normalizeOptionForSlot(option)">
+              {{ getOptionLabel(option) }}
+            </slot>
+          </li>
+          <li v-if="!filteredOptions.length" class="vs__no-options" @mousedown.stop="">
+            <slot name="no-options">Sorry, no matching options.</slot>
+          </li>
+        </template>
       </ul>
     </transition>
   </div>
@@ -79,6 +84,7 @@
   import typeAheadPointer from '../mixins/typeAheadPointer'
   import ajax from '../mixins/ajax'
   import childComponents from './childComponents';
+  import uniqueId from '../utility/uniqueId';
 
   /**
    * @name VueSelect
@@ -516,6 +522,7 @@
 
     data() {
       return {
+        uid: uniqueId(),
         search: '',
         open: false,
         isComposing: false,
@@ -985,10 +992,11 @@
               'tabindex': this.tabindex,
               'readonly': !this.searchable,
               'id': this.inputId,
-              'aria-expanded': this.dropdownOpen,
-              'aria-label': 'Search for option',
+              'aria-autocomplete': 'list',
+              'aria-labelledby': `vs${this.uid}__combobox`,
+              'aria-controls': `vs${this.uid}__listbox`,
+              'aria-activedescendant': this.typeAheadPointer > -1 ? `vs${this.uid}__option-${this.typeAheadPointer}` : '',
               'ref': 'search',
-              'role': 'combobox',
               'type': 'search',
               'autocomplete': this.autocomplete,
               'value': this.search,
diff --git a/src/mixins/pointerScroll.js b/src/mixins/pointerScroll.js
index 160f0c0..422ee59 100644
--- a/src/mixins/pointerScroll.js
+++ b/src/mixins/pointerScroll.js
@@ -30,7 +30,7 @@ export default {
      */
     pixelsToPointerTop() {
       let pixelsToPointerTop = 0;
-      if (this.$refs.dropdownMenu) {
+      if (this.$refs.dropdownMenu && this.dropdownOpen) {
         for (let i = 0; i < this.typeAheadPointer; i++) {
           pixelsToPointerTop += this.$refs.dropdownMenu.children[i]
             .offsetHeight;
diff --git a/src/utility/uniqueId.js b/src/utility/uniqueId.js
new file mode 100644
index 0000000..8e96640
--- /dev/null
+++ b/src/utility/uniqueId.js
@@ -0,0 +1,12 @@
+let idCount = 0;
+
+/**
+ * Dead simple unique ID implementation.
+ * Thanks lodash!
+ * @return {number}
+ */
+function uniqueId() {
+  return ++idCount;
+}
+
+export default uniqueId;
diff --git a/tests/unit/Dropdown.spec.js b/tests/unit/Dropdown.spec.js
index 6ae22a4..6777410 100755
--- a/tests/unit/Dropdown.spec.js
+++ b/tests/unit/Dropdown.spec.js
@@ -129,14 +129,17 @@ describe("Toggling Dropdown", () => {
     expect(Select.vm.stateClasses['vs--open']).toEqual(true);
   });
 
-  it("should not display the dropdown if noDrop is true", () => {
+  it("should not display the dropdown if noDrop is true", async () => {
     const Select = selectWithProps({
       noDrop: true,
     });
-
     Select.vm.toggleDropdown({ target: Select.vm.$refs.search });
     expect(Select.vm.open).toEqual(true);
-    expect(Select.contains('.vs__dropdown-menu')).toBeFalsy();
+    await Select.vm.$nextTick();
+
+    expect(Select.find('.vs__dropdown-menu').element.style['display']).toEqual('none');
+    expect(Select.contains('.vs__dropdown-option')).toBeFalsy();
+    expect(Select.contains('.vs__no-options')).toBeFalsy();
     expect(Select.vm.stateClasses['vs--open']).toBeFalsy();
   });
 
diff --git a/tests/unit/utility/uniqueId.spec.js b/tests/unit/utility/uniqueId.spec.js
new file mode 100644
index 0000000..c425a7c
--- /dev/null
+++ b/tests/unit/utility/uniqueId.spec.js
@@ -0,0 +1,5 @@
+import uniqueId from '../../../src/utility/uniqueId';
+
+test('it generates a unique number', () => {
+  expect(uniqueId()).not.toEqual(uniqueId());
+});