fix(VSelect): expose clickable icons to screen readers (vuetifyjs#10489)

* fix(VSelect): allow clickable append/prepend icons to be focused * refactor(VTextField): always render a named icon when clearable * refactor(VInput): use default argument instead of or operator
rachael-smith · Feb 7, 2020 · 1119810 · 1119810
1 parent 0f0c05e
commit 1119810
Show file tree

Hide file tree

Showing 7 changed files with 52 additions and 36 deletions.
diff --git a/packages/vuetify/src/components/VFileInput/__tests__/__snapshots__/VFileInput.spec.ts.snap b/packages/vuetify/src/components/VFileInput/__tests__/__snapshots__/VFileInput.spec.ts.snap
@@ -247,11 +247,14 @@ exports[`VFileInput.ts should render 1`] = `
         >
       </div>
       <div class="v-input__append-inner">
-        <div class="v-input__icon">
-          <i aria-hidden="true"
-             class="v-icon notranslate material-icons theme--light"
+        <div class="v-input__icon v-input__icon--clear">
+          <button disabled="disabled"
+                  type="button"
+                  aria-label="clear icon"
+                  class="v-icon notranslate v-icon--disabled v-icon--link material-icons theme--light"
           >
-          </i>
+            $clear
+          </button>
         </div>
       </div>
     </div>
@@ -429,11 +432,14 @@ exports[`VFileInput.ts should render without icon 1`] = `
         >
       </div>
       <div class="v-input__append-inner">
-        <div class="v-input__icon">
-          <i aria-hidden="true"
-             class="v-icon notranslate material-icons theme--light"
+        <div class="v-input__icon v-input__icon--clear">
+          <button disabled="disabled"
+                  type="button"
+                  aria-label="clear icon"
+                  class="v-icon notranslate v-icon--disabled v-icon--link material-icons theme--light"
           >
-          </i>
+            $clear
+          </button>
         </div>
       </div>
     </div>

diff --git a/packages/vuetify/src/components/VInput/VInput.sass b/packages/vuetify/src/components/VInput/VInput.sass
@@ -94,6 +94,9 @@
     &--clear
       border-radius: 50%
 
+      .v-icon--disabled
+        visibility: hidden
+
   &__slot
     align-items: center
     color: inherit

diff --git a/packages/vuetify/src/components/VInput/VInput.ts b/packages/vuetify/src/components/VInput/VInput.ts
@@ -16,6 +16,7 @@ import {
   getSlot,
   kebabCase,
 } from '../../util/helpers'
+import mergeData from '../../util/mergeData'
 
 // Types
 import { VNode, VNodeData, PropType } from 'vue'
@@ -165,17 +166,16 @@ export default baseMixins.extend<options>().extend({
     },
     genIcon (
       type: string,
-      cb?: (e: Event) => void
+      cb?: (e: Event) => void,
+      extraData: VNodeData = {}
     ) {
       const icon = (this as any)[`${type}Icon`]
       const eventName = `click:${kebabCase(type)}`
       const hasListener = !!(this.listeners$[eventName] || cb)
 
-      const data: VNodeData = {
+      const data = mergeData({
         attrs: {
           'aria-label': hasListener ? kebabCase(type).split('-')[0] + ' icon' : undefined,
-        },
-        props: {
           color: this.validationState,
           dark: this.dark,
           disabled: this.disabled,
@@ -198,7 +198,7 @@ export default baseMixins.extend<options>().extend({
               e.stopPropagation()
             },
           },
-      }
+      }, extraData)
 
       return this.$createElement('div', {
         staticClass: `v-input__icon`,

diff --git a/packages/vuetify/src/components/VSelect/VSelect.ts b/packages/vuetify/src/components/VSelect/VSelect.ts
@@ -25,7 +25,7 @@ import mergeData from '../../util/mergeData'
 
 // Types
 import mixins from '../../util/mixins'
-import { VNode, VNodeDirective, PropType } from 'vue'
+import { VNode, VNodeDirective, PropType, VNodeData } from 'vue'
 import { SelectItemKey } from 'types'
 
 export const defaultMenuProps = {
@@ -416,19 +416,21 @@ export default baseMixins.extend<options>().extend({
     },
     genIcon (
       type: string,
-      cb?: (e: Event) => void
+      cb?: (e: Event) => void,
+      extraData?: VNodeData
     ) {
-      const icon = VInput.options.methods.genIcon.call(this, type, cb)
-
-      icon.children![0].data = mergeData(icon.children![0].data!, {
-        attrs: {
-          tabindex: type !== 'append'
-            ? undefined
-            : (icon.children![0].componentOptions!.listeners && '-1'),
-          'aria-hidden': 'true',
-          'aria-label': undefined,
-        },
-      })
+      const icon = VInput.options.methods.genIcon.call(this, type, cb, extraData)
+
+      if (type === 'append') {
+        // Don't allow the dropdown icon to be focused
+        icon.children![0].data = mergeData(icon.children![0].data!, {
+          attrs: {
+            tabindex: icon.children![0].componentOptions!.listeners && '-1',
+            'aria-hidden': 'true',
+            'aria-label': undefined,
+          },
+        })
+      }
 
       return icon
     },

diff --git a/packages/vuetify/src/components/VSelect/__tests__/__snapshots__/VSelect2.spec.ts.snap b/packages/vuetify/src/components/VSelect/__tests__/__snapshots__/VSelect2.spec.ts.snap
@@ -23,8 +23,8 @@ exports[`VSelect.ts should be clearable with prop, dirty and multi select 1`] =
         </div>
         <div class="v-input__append-inner">
           <div class="v-input__icon v-input__icon--clear">
-            <button aria-hidden="true"
-                    type="button"
+            <button type="button"
+                    aria-label="clear icon"
                     class="v-icon notranslate v-icon--link material-icons theme--light"
             >
               $clear
@@ -83,8 +83,8 @@ exports[`VSelect.ts should be clearable with prop, dirty and single select 1`] =
         </div>
         <div class="v-input__append-inner">
           <div class="v-input__icon v-input__icon--clear">
-            <button aria-hidden="true"
-                    type="button"
+            <button type="button"
+                    aria-label="clear icon"
                     class="v-icon notranslate v-icon--link material-icons theme--light"
             >
               $clear

diff --git a/packages/vuetify/src/components/VTextField/VTextField.ts b/packages/vuetify/src/components/VTextField/VTextField.ts
@@ -294,14 +294,10 @@ export default baseMixins.extend<options>().extend({
     genClearIcon () {
       if (!this.clearable) return null
 
-      const icon = this.isDirty ? 'clear' : ''
-      const cb = this.isDirty ? this.clearableCallback : undefined
+      const data = this.isDirty ? undefined : { attrs: { disabled: true } }
 
       return this.genSlot('append', 'inner', [
-        this.genIcon(
-          icon,
-          cb
-        ),
+        this.genIcon('clear', this.clearableCallback, data),
       ])
     },
     genCounter () {

diff --git a/packages/vuetify/src/util/mergeData.ts b/packages/vuetify/src/util/mergeData.ts
@@ -53,6 +53,9 @@ export default function mergeData (): VNodeData {
         case 'class':
         case 'style':
         case 'directives':
+          if (!arguments[i][prop]) {
+            break
+          }
           if (!Array.isArray(mergeTarget[prop])) {
             mergeTarget[prop] = []
           }
@@ -98,6 +101,9 @@ export default function mergeData (): VNodeData {
         // uses the last given value to assign.
         case 'on':
         case 'nativeOn':
+          if (!arguments[i][prop]) {
+            break
+          }
           if (!mergeTarget[prop]) {
             mergeTarget[prop] = {}
           }
@@ -124,6 +130,9 @@ export default function mergeData (): VNodeData {
         case 'staticStyle':
         case 'hook':
         case 'transition':
+          if (!arguments[i][prop]) {
+            break
+          }
           if (!mergeTarget[prop]) {
             mergeTarget[prop] = {}
           }