feat(multi-select): address accessibility tabbing issue (#1731)

* Added keydown to open, fixed a11y tabbing issue * Linting * Addressed reviews * fixup * linting * added test coverage for list-box * ran prettier
carbon-design-system · Mar 8, 2019 · f288b7c · f288b7c
1 parent b3462ea
commit f288b7c
Show file tree

Hide file tree

Showing 8 changed files with 28 additions and 9 deletions.
diff --git a/src/components/DropdownV2/__snapshots__/DropdownV2-test.js.snap b/src/components/DropdownV2/__snapshots__/DropdownV2-test.js.snap
@@ -73,7 +73,7 @@ exports[`DropdownV2 should render 1`] = `
         onClick={[Function]}
         onKeyDown={[Function]}
         role="listbox"
-        tabIndex="0"
+        tabIndex={0}
       >
         <ListBoxField
           aria-expanded={false}
@@ -246,7 +246,7 @@ exports[`DropdownV2 should render DropdownItem components 1`] = `
         onClick={[Function]}
         onKeyDown={[Function]}
         role="listbox"
-        tabIndex="0"
+        tabIndex={0}
       >
         <ListBoxField
           aria-expanded={true}
@@ -660,7 +660,7 @@ exports[`DropdownV2 should render custom item components 1`] = `
         onClick={[Function]}
         onKeyDown={[Function]}
         role="listbox"
-        tabIndex="0"
+        tabIndex={0}
       >
         <ListBoxField
           aria-expanded={true}
@@ -966,7 +966,7 @@ exports[`DropdownV2 should render with strings as items 1`] = `
         onClick={[Function]}
         onKeyDown={[Function]}
         role="listbox"
-        tabIndex="0"
+        tabIndex={0}
       >
         <ListBoxField
           aria-expanded={true}

diff --git a/src/components/ListBox/ListBox.js b/src/components/ListBox/ListBox.js
@@ -41,6 +41,7 @@ const ListBox = ({
   invalid,
   invalidText,
   light,
+  innerTabIndex,
   ...rest
 }) => {
   const className = cx({
@@ -56,7 +57,7 @@ const ListBox = ({
         {...rest}
         role="listbox"
         aria-label={ariaLabel}
-        tabIndex="0"
+        tabIndex={innerTabIndex || 0}
         className={className}
         ref={innerRef}
         onKeyDown={handleOnKeyDown}

diff --git a/src/components/ListBox/__tests__/ListBox-test.js b/src/components/ListBox/__tests__/ListBox-test.js
@@ -51,4 +51,14 @@ describe('ListBox', () => {
         .includes(mockProps.className)
     ).toBe(true);
   });
+
+  it('should pass down innerTabIndex prop to the inner divs tabIndex', () => {
+    mockProps.innerTabIndex = -1;
+    const wrapper = mount(<ListBox {...mockProps} />);
+    expect(wrapper.children().prop('tabIndex')).toBe(-1);
+  });
+  it('should default the inner divs tabIndex to 0 if no innerTabIndex prop', () => {
+    const wrapper = mount(<ListBox {...mockProps} />);
+    expect(wrapper.children().prop('tabIndex')).toBe(0);
+  });
 });
diff --git a/src/components/ListBox/__tests__/__snapshots__/ListBox-test.js.snap b/src/components/ListBox/__tests__/__snapshots__/ListBox-test.js.snap
@@ -39,7 +39,7 @@ exports[`ListBox should render 1`] = `
     onClick={[Function]}
     onKeyDown={[Function]}
     role="listbox"
-    tabIndex="0"
+    tabIndex={0}
   >
     <ListBoxField>
       <div

diff --git a/src/components/MultiSelect/FilterableMultiSelect.js b/src/components/MultiSelect/FilterableMultiSelect.js
@@ -144,11 +144,16 @@ export default class FilterableMultiSelect extends React.Component {
   handleOnStateChange = changes => {
     const { type } = changes;
     switch (type) {
-      case Downshift.stateChangeTypes.keyDownArrowDown:
       case Downshift.stateChangeTypes.keyDownArrowUp:
       case Downshift.stateChangeTypes.itemMouseEnter:
         this.setState({ highlightedIndex: changes.highlightedIndex });
         break;
+      case Downshift.stateChangeTypes.keyDownArrowDown:
+        this.setState({
+          highlightedIndex: changes.highlightedIndex,
+          isOpen: true,
+        });
+        break;
       case Downshift.stateChangeTypes.keyDownEscape:
       case Downshift.stateChangeTypes.mouseUp:
         this.setState({ isOpen: false });
@@ -254,6 +259,7 @@ export default class FilterableMultiSelect extends React.Component {
                 disabled={disabled}
                 invalid={invalid}
                 invalidText={invalidText}
+                innerTabIndex="-1"
                 {...getRootProps({ refKey: 'innerRef' })}>
                 <ListBox.Field {...getButtonProps({ disabled })}>
                   {selectedItem.length > 0 && (

diff --git a/src/components/MultiSelect/MultiSelect-story.js b/src/components/MultiSelect/MultiSelect-story.js
@@ -104,6 +104,7 @@ storiesOf('MultiSelect', module)
       } = props();
       const ComponentToUse = !filterable ? MultiSelect : MultiSelect.Filterable;
       const placeholder = !filterable ? undefined : defaultPlaceholder;
+
       return (
         <div style={{ width: 300 }}>
           <ComponentToUse

diff --git a/src/components/MultiSelect/__tests__/__snapshots__/FilterableMultiSelect-test.js.snap b/src/components/MultiSelect/__tests__/__snapshots__/FilterableMultiSelect-test.js.snap
@@ -76,6 +76,7 @@ exports[`MultiSelect.Filterable should render 1`] = `
         className="bx--multi-select bx--combo-box"
         disabled={false}
         innerRef={[Function]}
+        innerTabIndex="-1"
         type="default"
       >
         <div
@@ -84,7 +85,7 @@ exports[`MultiSelect.Filterable should render 1`] = `
           onClick={[Function]}
           onKeyDown={[Function]}
           role="listbox"
-          tabIndex="0"
+          tabIndex="-1"
         >
           <ListBoxField
             aria-expanded={false}

diff --git a/src/components/MultiSelect/__tests__/__snapshots__/MultiSelect-test.js.snap b/src/components/MultiSelect/__tests__/__snapshots__/MultiSelect-test.js.snap
@@ -84,7 +84,7 @@ exports[`MultiSelect should render 1`] = `
           onClick={[Function]}
           onKeyDown={[Function]}
           role="listbox"
-          tabIndex="0"
+          tabIndex={0}
         >
           <ListBoxField
             aria-expanded={false}