feat(b-table): better sort labeling for screen readers (closes #4487) (…

…#4488) * feat(b-table): better sort labeling for screen readers * Update mixin-thead.js * Update mixin-thead.js * Update mixin-thead.js * Update mixin-thead.js * Update mixin-thead.js * Update mixin-sorting.js * Update mixin-sorting.js * Update table-sorting.spec.js * Update package.json * Update table-sorting.spec.js * Update table-sorting.spec.js * Update mixin-thead.js * Update mixin-thead.js
bootstrap-vue · Dec 13, 2019 · d4e66fa48fdd1cd7fd4b93907fe999de3fc577f8 · d4e66fa
1 parent 69edc0c
commit d4e66fa48fdd1cd7fd4b93907fe999de3fc577f8
Unified Split

Showing with 144 additions and 72 deletions.

+29 −32 src/components/table/helpers/mixin-sorting.js

+15 −15 src/components/table/helpers/mixin-thead.js

+3 −3 src/components/table/package.json

+97 −22 src/components/table/table-sorting.spec.js
diff --git a/src/components/table/helpers/mixin-sorting.js b/src/components/table/helpers/mixin-sorting.js
@@ -1,7 +1,7 @@
 import stableSort from '../../../utils/stable-sort'
-import startCase from '../../../utils/startcase'
 import { arrayIncludes } from '../../../utils/array'
 import { isFunction, isUndefinedOrNull } from '../../../utils/inspect'
+import { trim } from '../../../utils/string'
 import defaultSortCompare from './default-sort-compare'
 
 export default {
@@ -242,55 +242,52 @@ export default {
         return {}
       }
       const sortable = field.sortable
-      let ariaLabel = ''
-      if ((!field.label || !field.label.trim()) && !field.headerTitle) {
-        // In case field's label and title are empty/blank, we need to
-        // add a hint about what the column is about for non-sighted users.
-        // This is duplicated code from tbody-row mixin, but we need it
-        // here as well, since we overwrite the original aria-label.
-        /* istanbul ignore next */
-        ariaLabel = startCase(key)
+      // Assemble the aria-sort attribute value
+      const ariaSort =
+        sortable && this.localSortBy === key
+          ? this.localSortDesc
+            ? 'descending'
+            : 'ascending'
+          : sortable
+            ? 'none'
+            : null
+      // Return the attribute
+      return {
+        'aria-sort': ariaSort
+      }
+    },
+    sortTheadThLabel(key, field, isFoot) {
+      // A label to be placed in an `.sr-only` element in the header cell
+      if (!this.isSortable || (isFoot && this.noFooterSorting)) {
+        // No label if not a sortable table
+        return null
       }
+      const sortable = field.sortable
       // The correctness of these labels is very important for screen-reader users.
-      let ariaLabelSorting = ''
+      let labelSorting = ''
       if (sortable) {
         if (this.localSortBy === key) {
           // currently sorted sortable column.
-          ariaLabelSorting = this.localSortDesc ? this.labelSortAsc : this.labelSortDesc
+          labelSorting = this.localSortDesc ? this.labelSortAsc : this.labelSortDesc
         } else {
           // Not currently sorted sortable column.
           // Not using nested ternary's here for clarity/readability
           // Default for ariaLabel
-          ariaLabelSorting = this.localSortDesc ? this.labelSortDesc : this.labelSortAsc
+          labelSorting = this.localSortDesc ? this.labelSortDesc : this.labelSortAsc
           // Handle sortDirection setting
           const sortDirection = this.sortDirection || field.sortDirection
           if (sortDirection === 'asc') {
-            ariaLabelSorting = this.labelSortAsc
+            labelSorting = this.labelSortAsc
           } else if (sortDirection === 'desc') {
-            ariaLabelSorting = this.labelSortDesc
+            labelSorting = this.labelSortDesc
           }
         }
       } else if (!this.noSortReset) {
         // Non sortable column
-        ariaLabelSorting = this.localSortBy ? this.labelSortClear : ''
-      }
-      // Assemble the aria-label attribute value
-      ariaLabel = [ariaLabel.trim(), ariaLabelSorting.trim()].filter(Boolean).join(': ')
-      // Assemble the aria-sort attribute value
-      const ariaSort =
-        sortable && this.localSortBy === key
-          ? this.localSortDesc
-            ? 'descending'
-            : 'ascending'
-          : sortable
-            ? 'none'
-            : null
-      // Return the attributes
-      // (All the above just to get these two values)
-      return {
-        'aria-label': ariaLabel || null,
-        'aria-sort': ariaSort
+        labelSorting = this.localSortBy ? this.labelSortClear : ''
       }
+      // Return the sr-only sort label or null if no label
+      return trim(labelSorting) || null
     }
   }
 }
diff --git a/src/components/table/helpers/mixin-thead.js b/src/components/table/helpers/mixin-thead.js
@@ -89,6 +89,7 @@ export default {
         }
         const sortAttrs = this.isSortable ? this.sortTheadThAttrs(field.key, field, isFoot) : {}
         const sortClass = this.isSortable ? this.sortTheadThClasses(field.key, field, isFoot) : null
+        const sortLabel = this.isSortable ? this.sortTheadThLabel(field.key, field, isFoot) : null
         const data = {
           key: field.key,
           class: [this.fieldClasses(field), sortClass],
@@ -124,22 +125,21 @@ export default {
             ...slotNames
           ]
         }
-        const hasSlot = this.hasNormalizedSlot(slotNames)
-        let slot = field.label
-        if (hasSlot) {
-          slot = this.normalizeSlot(slotNames, {
-            label: field.label,
-            column: field.key,
-            field,
-            isFoot,
-            // Add in row select methods
-            selectAllRows,
-            clearSelected
-          })
-        } else {
-          data.domProps = htmlOrText(field.labelHtml)
+        const scope = {
+          label: field.label,
+          column: field.key,
+          field,
+          isFoot,
+          // Add in row select methods
+          selectAllRows,
+          clearSelected
         }
-        return h(BTh, data, slot)
+        const content =
+          this.normalizeSlot(slotNames, scope) ||
+          (field.labelHtml ? h('div', { domProps: htmlOrText(field.labelHtml) }) : field.label)
+        const srLabel = sortLabel ? h('span', { staticClass: 'sr-only' }, ` (${sortLabel})`) : null
+        // Return the header cell
+        return h(BTh, data, [content, srLabel].filter(identity))
       }
 
       // Generate the array of <th> cells

diff --git a/src/components/table/package.json b/src/components/table/package.json
@@ -235,15 +235,15 @@
           },
           {
             "prop": "labelSortAsc",
-            "description": "String to place in the header cell's 'aria-label' when clicking the cell will change the sort direction to ascending"
+            "description": "Hidden string to place in the header cell when clicking the cell will change the sort direction to ascending"
           },
           {
             "prop": "labelSortDesc",
-            "description": "String to place in the header cell's 'aria-label' when clicking the cell will change the sort direction to descending"
+            "description": "Hidden string to place in the header cell when clicking the cell will change the sort direction to descending"
           },
           {
             "prop": "labelSortClear",
-            "description": "String to place in the header cell's 'aria-label' when clicking the cell will clear the current sorting direction"
+            "description": "Hidden string to place in the header cell when clicking the cell will clear the current sorting direction"
           },
           {
             "prop": "selectable",

diff --git a/src/components/table/table-sorting.spec.js b/src/components/table/table-sorting.spec.js
@@ -77,17 +77,32 @@ describe('table > sorting', () => {
     // Currently sorted as ascending
     expect($ths.at(0).attributes('aria-sort')).toBe('ascending')
     // For switching to descending
-    expect($ths.at(0).attributes('aria-label')).toBe(wrapper.vm.labelSortDesc)
+    expect(
+      $ths
+        .at(0)
+        .find('.sr-only')
+        .text()
+    ).toContain(wrapper.vm.labelSortDesc)
 
     // Not sorted by this column
     expect($ths.at(1).attributes('aria-sort')).toBe('none')
     // For sorting by ascending
-    expect($ths.at(1).attributes('aria-label')).toBe(wrapper.vm.labelSortAsc)
+    expect(
+      $ths
+        .at(1)
+        .find('.sr-only')
+        .text()
+    ).toContain(wrapper.vm.labelSortAsc)
 
     // Not a sortable column
     expect($ths.at(2).attributes('aria-sort')).not.toBeDefined()
     // For clearing sorting
-    expect($ths.at(2).attributes('aria-label')).toBe(wrapper.vm.labelSortClear)
+    expect(
+      $ths
+        .at(2)
+        .find('.sr-only')
+        .text()
+    ).toContain(wrapper.vm.labelSortClear)
 
     // Change sort direction
     wrapper.setProps({
@@ -113,17 +128,32 @@ describe('table > sorting', () => {
     // Currently sorted as descending
     expect($ths.at(0).attributes('aria-sort')).toBe('descending')
     // For switching to ascending
-    expect($ths.at(0).attributes('aria-label')).toBe(wrapper.vm.labelSortAsc)
+    expect(
+      $ths
+        .at(0)
+        .find('.sr-only')
+        .text()
+    ).toContain(wrapper.vm.labelSortAsc)
 
     // Not sorted by this column
     expect($ths.at(1).attributes('aria-sort')).toBe('none')
     // For sorting by ascending
-    expect($ths.at(1).attributes('aria-label')).toBe(wrapper.vm.labelSortAsc)
+    expect(
+      $ths
+        .at(1)
+        .find('.sr-only')
+        .text()
+    ).toContain(wrapper.vm.labelSortAsc)
 
     // Not a sortable column
     expect($ths.at(2).attributes('aria-sort')).not.toBeDefined()
     // For clearing sorting
-    expect($ths.at(2).attributes('aria-label')).toBe(wrapper.vm.labelSortClear)
+    expect(
+      $ths
+        .at(2)
+        .find('.sr-only')
+        .text()
+    ).toContain(wrapper.vm.labelSortClear)
 
     // Clear sort
     wrapper.setProps({
@@ -150,17 +180,32 @@ describe('table > sorting', () => {
     // Currently not sorted
     expect($ths.at(0).attributes('aria-sort')).toBe('none')
     // For sorting by ascending
-    expect($ths.at(0).attributes('aria-label')).toBe(wrapper.vm.labelSortAsc)
+    expect(
+      $ths
+        .at(0)
+        .find('.sr-only')
+        .text()
+    ).toContain(wrapper.vm.labelSortAsc)
 
     // Not sorted by this column
     expect($ths.at(1).attributes('aria-sort')).toBe('none')
     // For sorting by ascending
-    expect($ths.at(1).attributes('aria-label')).toBe(wrapper.vm.labelSortAsc)
+    expect(
+      $ths
+        .at(1)
+        .find('.sr-only')
+        .text()
+    ).toContain(wrapper.vm.labelSortAsc)
 
     // Not a sortable column
     expect($ths.at(2).attributes('aria-sort')).not.toBeDefined()
     // For clearing sorting
-    expect($ths.at(2).attributes('aria-label')).not.toBeDefined()
+    expect(
+      $ths
+        .at(2)
+        .find('.sr-only')
+        .exists()
+    ).toBe(false)
 
     wrapper.destroy()
   })
@@ -351,7 +396,7 @@ describe('table > sorting', () => {
     expect(columnA[2]).toBe('2')
     // Should have aria-* labels
     expect(wrapper.findAll('tfoot > tr > th[aria-sort]').length).toBe(2)
-    expect(wrapper.findAll('tfoot > tr > th[aria-label]').length).toBe(2)
+    expect(wrapper.findAll('tfoot > tr > th > .sr-only').length).toBe(2)
 
     // Sort by first column
     wrapper
@@ -376,7 +421,7 @@ describe('table > sorting', () => {
     expect(columnA[2]).toBe('3')
     // Should have aria-* labels
     expect(wrapper.findAll('tfoot > tr > th[aria-sort]').length).toBe(2)
-    expect(wrapper.findAll('tfoot > tr > th[aria-label]').length).toBe(3)
+    expect(wrapper.findAll('tfoot > tr > th > .sr-only').length).toBe(3)
 
     // Click first column header again to reverse sort
     wrapper
@@ -400,7 +445,7 @@ describe('table > sorting', () => {
     expect(columnA[2]).toBe('1')
     // Should have aria-* labels
     expect(wrapper.findAll('tfoot > tr > th[aria-sort]').length).toBe(2)
-    expect(wrapper.findAll('tfoot > tr > th[aria-label]').length).toBe(3)
+    expect(wrapper.findAll('tfoot > tr > th > .sr-only').length).toBe(3)
 
     // Click second column header to sort by it (by using keydown.enter)
     wrapper
@@ -445,7 +490,7 @@ describe('table > sorting', () => {
     expect(columnA[2]).toBe('2')
     // Should have aria-* labels
     expect(wrapper.findAll('tfoot > tr > th[aria-sort]').length).toBe(2)
-    expect(wrapper.findAll('tfoot > tr > th[aria-label]').length).toBe(2)
+    expect(wrapper.findAll('tfoot > tr > th > .sr-only').length).toBe(2)
 
     wrapper.destroy()
   })
@@ -483,7 +528,7 @@ describe('table > sorting', () => {
     expect(columnA[2]).toBe('2')
     // Shouldn't have aria-* labels
     expect(wrapper.findAll('tfoot > tr > th[aria-sort]').length).toBe(0)
-    expect(wrapper.findAll('tfoot > tr > th[aria-label]').length).toBe(0)
+    expect(wrapper.findAll('tfoot > tr > th > .sr-only').length).toBe(0)
 
     // Click first column
     wrapper
@@ -506,7 +551,7 @@ describe('table > sorting', () => {
     expect(columnA[2]).toBe('2')
     // Shouldn't have aria-* labels
     expect(wrapper.findAll('tfoot > tr > th[aria-sort]').length).toBe(0)
-    expect(wrapper.findAll('tfoot > tr > th[aria-label]').length).toBe(0)
+    expect(wrapper.findAll('tfoot > tr > th > .sr-only').length).toBe(0)
 
     // Click third column header
     wrapper
@@ -529,7 +574,7 @@ describe('table > sorting', () => {
     expect(columnA[2]).toBe('2')
     // Shouldn't have aria-* labels
     expect(wrapper.findAll('tfoot > tr > th[aria-sort]').length).toBe(0)
-    expect(wrapper.findAll('tfoot > tr > th[aria-label]').length).toBe(0)
+    expect(wrapper.findAll('tfoot > tr > th > .sr-only').length).toBe(0)
 
     wrapper.destroy()
   })
@@ -568,17 +613,32 @@ describe('table > sorting', () => {
     // Currently not sorted
     expect($ths.at(0).attributes('aria-sort')).toBe('none')
     // For switching to descending
-    expect($ths.at(0).attributes('aria-label')).toBe(wrapper.vm.labelSortDesc)
+    expect(
+      $ths
+        .at(0)
+        .find('.sr-only')
+        .text()
+    ).toContain(wrapper.vm.labelSortDesc)
 
     // Not sorted by this column
     expect($ths.at(1).attributes('aria-sort')).toBe('none')
     // For sorting by ascending
-    expect($ths.at(1).attributes('aria-label')).toBe(wrapper.vm.labelSortDesc)
+    expect(
+      $ths
+        .at(1)
+        .find('.sr-only')
+        .text()
+    ).toContain(wrapper.vm.labelSortDesc)
 
     // Not a sortable column
     expect($ths.at(2).attributes('aria-sort')).not.toBeDefined()
     // For clearing sorting
-    expect($ths.at(2).attributes('aria-label')).not.toBeDefined()
+    expect(
+      $ths
+        .at(2)
+        .find('.sr-only')
+        .exists()
+    ).toBe(false)
 
     // Change sort direction (should be descending first)
     wrapper
@@ -605,17 +665,32 @@ describe('table > sorting', () => {
     // Currently sorted as descending
     expect($ths.at(0).attributes('aria-sort')).toBe('descending')
     // For switching to ascending
-    expect($ths.at(0).attributes('aria-label')).toBe(wrapper.vm.labelSortAsc)
+    expect(
+      $ths
+        .at(0)
+        .find('.sr-only')
+        .text()
+    ).toContain(wrapper.vm.labelSortAsc)
 
     // Not sorted by this column
     expect($ths.at(1).attributes('aria-sort')).toBe('none')
     // For sorting by ascending
-    expect($ths.at(1).attributes('aria-label')).toBe(wrapper.vm.labelSortDesc)
+    expect(
+      $ths
+        .at(1)
+        .find('.sr-only')
+        .text()
+    ).toContain(wrapper.vm.labelSortDesc)
 
     // Not a sortable column
     expect($ths.at(2).attributes('aria-sort')).not.toBeDefined()
     // For clearing sorting
-    expect($ths.at(2).attributes('aria-label')).toBe(wrapper.vm.labelSortClear)
+    expect(
+      $ths
+        .at(2)
+        .find('.sr-only')
+        .text()
+    ).toContain(wrapper.vm.labelSortClear)
 
     wrapper.destroy()
   })