fix: Adjust aria labels to reference selections(#275)

ellinge · mrchief · commit d341b75eba7e · 2019-07-22T22:48:35.000-04:00
and ensure "Remove" label is read by Screen readers
diff --git a/__snapshots__/src/index.test.js.md b/__snapshots__/src/index.test.js.md
@@ -16,11 +16,14 @@ Generated by [AVA](https://ava.li).
         className="dropdown"
       >
         <Trigger
+          clientId="rdts"
           onTrigger={Function {}}
           showDropdown={true}
+          tags={[]}
           texts={{}}
         >
           <Input
+            clientId="rdts"
             inputRef={Function inputRef {}}
             onBlur={Function {}}
             onFocus={Function {}}
@@ -177,12 +180,15 @@ Generated by [AVA](https://ava.li).
         className="dropdown"
       >
         <Trigger
+          clientId="rdts"
           disabled={true}
           onTrigger={Function {}}
           showDropdown={false}
+          tags={[]}
           texts={{}}
         >
           <Input
+            clientId="rdts"
             disabled={true}
             inputRef={Function inputRef {}}
             onBlur={Function {}}
@@ -287,21 +293,25 @@ Generated by [AVA](https://ava.li).
           className="dropdown radio-select"
         >
           <Trigger
+            clientId="rdts"
             mode="radioSelect"
             onTrigger={Function {}}
             showDropdown={false}
+            tags={[]}
             texts={{}}
           >
             <a
               "aria-expanded"="false"
               "aria-haspopup"="tree"
               className="dropdown-trigger arrow bottom"
+              id="rdts_trigger"
               onClick={Function {}}
               onKeyDown={Function {}}
               role="button"
               tabIndex={0}
             >
               <Input
+                clientId="rdts"
                 inputRef={Function inputRef {}}
                 mode="radioSelect"
                 onBlur={Function {}}
@@ -426,20 +436,24 @@ Generated by [AVA](https://ava.li).
           className="dropdown"
         >
           <Trigger
+            clientId="rdts"
             onTrigger={Function {}}
             showDropdown={false}
+            tags={[]}
             texts={{}}
           >
             <a
               "aria-expanded"="false"
               "aria-haspopup"="tree"
               className="dropdown-trigger arrow bottom"
+              id="rdts_trigger"
               onClick={Function {}}
               onKeyDown={Function {}}
               role="button"
               tabIndex={0}
             >
               <Input
+                clientId="rdts"
                 inputRef={Function inputRef {}}
                 onBlur={Function {}}
                 onFocus={Function {}}
@@ -486,11 +500,14 @@ Generated by [AVA](https://ava.li).
         className="dropdown"
       >
         <Trigger
+          clientId="rdts"
           onTrigger={Function {}}
           showDropdown={true}
+          tags={[]}
           texts={{}}
         >
           <Input
+            clientId="rdts"
             inputRef={Function inputRef {}}
             onBlur={Function {}}
             onFocus={Function {}}
diff --git a/__snapshots__/src/index.test.js.snap b/__snapshots__/src/index.test.js.snap
diff --git a/__snapshots__/src/tag/index.test.js.md b/__snapshots__/src/tag/index.test.js.md
@@ -9,14 +9,16 @@ Generated by [AVA](https://ava.li).
 > Snapshot 1
 
     <span
+      "aria-label"="hello"
       className="tag"
       id="abc_tag"
     >
       hello
       <button
         "aria-label"="Remove"
-        "aria-labelledby"="abc_tag"
+        "aria-labelledby"="abc_button abc_tag"
         className="tag-remove"
+        id="abc_button"
         onClick={Function {}}
         onKeyDown={Function {}}
         onKeyUp={Function {}}
diff --git a/__snapshots__/src/tag/index.test.js.snap b/__snapshots__/src/tag/index.test.js.snap
diff --git a/__snapshots__/src/trigger/index.test.js.md b/__snapshots__/src/trigger/index.test.js.md
@@ -12,6 +12,7 @@ Generated by [AVA](https://ava.li).
       "aria-expanded"="false"
       "aria-haspopup"="tree"
       className="dropdown-trigger arrow bottom"
+      id="rtds_trigger"
       onClick={Function {}}
       onKeyDown={Function {}}
       role="button"
diff --git a/__snapshots__/src/trigger/index.test.js.snap b/__snapshots__/src/trigger/index.test.js.snap
diff --git a/docs/src/stories/Options/index.js b/docs/src/stories/Options/index.js
@@ -70,7 +70,7 @@ class WithOptions extends PureComponent {
             </select>
           </div>
           <div style={{ marginBottom: '10px' }}>
-            <label htmlFor={showDropdown}>ShowDropdown: </label>
+            <label htmlFor={showDropdown}>Show dropdown: </label>
             <select
               id="showDropdown"
               value={showDropdown}
@@ -123,6 +123,7 @@ class WithOptions extends PureComponent {
             disabled={disabled}
             readOnly={readOnly}
             showDropdown={showDropdown}
+            texts={{ label: 'Demo Dropdown' }}
           />
         </div>
       </div>
diff --git a/src/a11y/index.js b/src/a11y/index.js
@@ -1,10 +1,20 @@
-export function getAriaLabel(label) {
-  if (!label) return undefined
+export function getAriaLabel(label, additionalLabelledBy) {
+  const attributes = getAriaAttributeForLabel(label)
 
-  if (label && label[0] === '#') {
-    /* See readme for label. When label starts with # it references ids of dom nodes instead.
-      When used on aria-labelledby, they should be referenced without a starting hash/# */
-    return { 'aria-labelledby': label.replace(/#/g, '') }
+  if (additionalLabelledBy) {
+    attributes['aria-labelledby'] = `${attributes['aria-labelledby'] || ''} ${additionalLabelledBy}`.trim()
+  }
+
+  return attributes
+}
+
+function getAriaAttributeForLabel(label) {
+  if (!label) return {}
+
+  /* See readme for label. When label starts with # it references ids of dom nodes instead.
+    When used on aria-labelledby, they should be referenced without a starting hash/# */
+  if (label[0] === '#') {
+    return { 'aria-labelledby': label.substring(1).replace(/ #/g, ' ') }
   }
   return { 'aria-label': label }
 }
diff --git a/src/index.js b/src/index.js
@@ -277,11 +277,11 @@ class DropdownTreeSelect extends Component {
 
   render() {
     const { disabled, readOnly, mode, texts } = this.props
-    const { showDropdown, currentFocus } = this.state
+    const { showDropdown, currentFocus, tags } = this.state
 
     const activeDescendant = currentFocus ? `${currentFocus}_li` : undefined
 
-    const commonProps = { disabled, readOnly, activeDescendant, texts, mode }
+    const commonProps = { disabled, readOnly, activeDescendant, texts, mode, clientId: this.clientId }
 
     return (
       <div
@@ -298,12 +298,12 @@ class DropdownTreeSelect extends Component {
             { 'radio-select': mode === 'radioSelect' }
           )}
         >
-          <Trigger onTrigger={this.onTrigger} showDropdown={showDropdown} {...commonProps}>
+          <Trigger onTrigger={this.onTrigger} showDropdown={showDropdown} {...commonProps} tags={tags}>
             <Input
               inputRef={el => {
                 this.searchInput = el
               }}
-              tags={this.state.tags}
+              tags={tags}
               onInputChange={this.onInputChange}
               onFocus={this.onInputFocus}
               onBlur={this.onInputBlur}
@@ -327,7 +327,6 @@ class DropdownTreeSelect extends Component {
                   onNodeToggle={this.onNodeToggle}
                   mode={mode}
                   showPartiallySelected={this.props.showPartiallySelected}
-                  clientId={this.clientId}
                   {...commonProps}
                 />
               )}
diff --git a/src/index.test.js b/src/index.test.js
@@ -290,3 +290,19 @@ test('adds aria-label when having label on search input', t => {
   t.deepEqual(wrapper.find('.search').prop('aria-labelledby'), undefined)
   t.deepEqual(wrapper.find('.search').prop('aria-label'), 'hello world')
 })
+
+test('appends selected tags to aria-labelledby with provided aria-labelledby', t => {
+  const { tree } = t.context
+  tree[0].checked = true
+  const wrapper = mount(<DropdownTreeSelect id="rdts" data={tree} texts={{ label: '#hello #world' }} />)
+  t.deepEqual(wrapper.find('.dropdown-trigger').prop('aria-labelledby'), 'hello world rdts-0_tag')
+  t.deepEqual(wrapper.find('.dropdown-trigger').prop('aria-label'), undefined)
+})
+
+test('appends selected tags to aria-labelledby with text label', t => {
+  const { tree } = t.context
+  tree[0].checked = true
+  const wrapper = mount(<DropdownTreeSelect id="rdts" data={tree} texts={{ label: 'hello world' }} />)
+  t.deepEqual(wrapper.find('.dropdown-trigger').prop('aria-labelledby'), 'rdts_trigger rdts-0_tag')
+  t.deepEqual(wrapper.find('.dropdown-trigger').prop('aria-label'), 'hello world')
+})
diff --git a/src/tag/index.js b/src/tag/index.js
@@ -6,6 +6,8 @@ import styles from './index.css'
 
 const cx = cn.bind(styles)
 
+export const getTagId = id => `${id}_tag`
+
 class Tag extends PureComponent {
   static propTypes = {
     id: PropTypes.string.isRequired,
@@ -40,24 +42,23 @@ class Tag extends PureComponent {
   render() {
     const { id, label, labelRemove = 'Remove', readOnly, disabled } = this.props
 
-    const tagId = `${id}_tag`
+    const tagId = getTagId(id)
+    const buttonId = `${id}_button`
     const className = cx('tag-remove', { readOnly }, { disabled })
     const isDisabled = readOnly || disabled
-    const onClick = !isDisabled ? this.handleClick : undefined
-    const onKeyDown = !isDisabled ? this.onKeyDown : undefined
-    const onKeyUp = !isDisabled ? this.onKeyUp : undefined
 
     return (
-      <span className={cx('tag')} id={tagId}>
+      <span className={cx('tag')} id={tagId} aria-label={label}>
         {label}
         <button
-          onClick={onClick}
-          onKeyDown={onKeyDown}
-          onKeyUp={onKeyUp}
+          id={buttonId}
+          onClick={!isDisabled ? this.handleClick : undefined}
+          onKeyDown={!isDisabled ? this.onKeyDown : undefined}
+          onKeyUp={!isDisabled ? this.onKeyUp : undefined}
           className={className}
           type="button"
           aria-label={labelRemove}
-          aria-labelledby={tagId}
+          aria-labelledby={`${buttonId} ${tagId}`}
           aria-disabled={isDisabled}
         >
           x
diff --git a/src/tree-manager/keyboardNavigation.js b/src/tree-manager/keyboardNavigation.js
@@ -1,4 +1,5 @@
 import nodeVisitor from './nodeVisitor'
+import { getTagId } from '../tag'
 
 const Keys = {
   Up: 'ArrowUp',
@@ -150,7 +151,7 @@ const getNextFocusAfterTagDelete = (deletedId, prevTags, tags, fallback) => {
 
   index = tags.length > index ? index : tags.length - 1
   const newFocusId = tags[index]._id
-  const focusNode = document.getElementById(`${newFocusId}_tag`)
+  const focusNode = document.getElementById(getTagId(newFocusId))
   if (focusNode) {
     return focusNode.firstElementChild || fallback
   }
diff --git a/src/trigger/index.js b/src/trigger/index.js
@@ -3,6 +3,7 @@ import PropTypes from 'prop-types'
 import cn from 'classnames/bind'
 
 import { getAriaLabel } from '../a11y'
+import { getTagId } from '../tag'
 
 import styles from '../index.css'
 
@@ -16,17 +17,34 @@ class Trigger extends PureComponent {
     showDropdown: PropTypes.bool,
     mode: PropTypes.oneOf(['multiSelect', 'simpleSelect', 'radioSelect', 'hierarchical']),
     texts: PropTypes.object,
+    clientId: PropTypes.string,
+    tags: PropTypes.array,
   }
 
   getAriaAttributes = () => {
-    const { mode, texts = {}, showDropdown } = this.props
+    const { mode, texts = {}, showDropdown, clientId, tags } = this.props
+
+    const triggerId = `${clientId}_trigger`
+    const labelledBy = []
+    let labelAttributes = getAriaLabel(texts.label)
+    if (tags && tags.length) {
+      if (labelAttributes['aria-label']) {
+        // Adds reference to self when having aria-label
+        labelledBy.push(triggerId)
+      }
+      tags.forEach(t => {
+        labelledBy.push(getTagId(t._id))
+      })
+      labelAttributes = getAriaLabel(texts.label, labelledBy.join(' '))
+    }
 
     const attributes = {
+      id: triggerId,
       role: 'button',
       tabIndex: 0,
       'aria-haspopup': mode === 'simpleSelect' ? 'listbox' : 'tree',
       'aria-expanded': showDropdown ? 'true' : 'false',
-      ...getAriaLabel(texts.label),
+      ...labelAttributes,
     }
 
     return attributes
diff --git a/src/trigger/index.test.js b/src/trigger/index.test.js
@@ -6,6 +6,6 @@ import toJson from 'enzyme-to-json'
 import Trigger from './index'
 
 test('Trigger component', t => {
-  const input = toJson(shallow(<Trigger />))
+  const input = toJson(shallow(<Trigger clientId={'rtds'} />))
   t.snapshot(input)
 })
