fix(dropdowns): pass user ID's to Downshift (#6326)

* fix(dropdowns): pass user supplied IDs to Downshift * test(Dropdown): update snappies * Apply suggestions from code review * test(MultiSelect): update test to match new id shape * fix(dropdown): fix id locations for each dropdown variant * chore(listbox): remove unused snapshot Co-authored-by: TJ Egan <tw15egan@gmail.com> Co-authored-by: Josh Black <josh@josh.black>
carbon-design-system · Jun 24, 2020 · aa8060f · aa8060f
1 parent 1d0d51a
commit aa8060f
Show file tree

Hide file tree

Showing 9 changed files with 39 additions and 111 deletions.
diff --git a/packages/react/src/components/ComboBox/ComboBox.js b/packages/react/src/components/ComboBox/ComboBox.js
@@ -337,6 +337,7 @@ export default class ComboBox extends React.Component {
         inputValue={this.state.inputValue || ''}
         itemToString={itemToString}
         defaultSelectedItem={initialSelectedItem}
+        inputId={id}
         selectedItem={selectedItem}>
         {({
           getToggleButtonProps,
@@ -367,7 +368,6 @@ export default class ComboBox extends React.Component {
               light={light}
               size={size}>
               <ListBox.Field
-                id={id}
                 {...getToggleButtonProps({
                   disabled,
                   onClick: this.onToggleClick(isOpen),
@@ -413,9 +413,7 @@ export default class ComboBox extends React.Component {
                 />
               </ListBox.Field>
               {isOpen && (
-                <ListBox.Menu
-                  id={id}
-                  {...getMenuProps({ 'aria-label': ariaLabel })}>
+                <ListBox.Menu {...getMenuProps({ 'aria-label': ariaLabel })}>
                   {this.filterItems(items, itemToString, inputValue).map(
                     (item, index) => {
                       const itemProps = getItemProps({ item, index });

diff --git a/packages/react/src/components/Dropdown/Dropdown.js b/packages/react/src/components/Dropdown/Dropdown.js
@@ -125,12 +125,12 @@ function Dropdown({
         invalid={invalid}
         invalidText={invalidText}
         light={light}
-        isOpen={isOpen}>
+        isOpen={isOpen}
+        id={id}>
         {invalid && (
           <WarningFilled16 className={`${prefix}--list-box__invalid-icon`} />
         )}
         <button
-          id={id}
           className={`${prefix}--list-box__field`}
           disabled={disabled}
           aria-disabled={disabled}

diff --git a/packages/react/src/components/Dropdown/__snapshots__/Dropdown-test.js.snap b/packages/react/src/components/Dropdown/__snapshots__/Dropdown-test.js.snap
@@ -50,12 +50,14 @@ exports[`Dropdown should render 1`] = `
     <ListBox
       className="bx--dropdown"
       disabled={false}
+      id="test-dropdown"
       isOpen={false}
       light={false}
       type="default"
     >
       <div
         className="bx--dropdown bx--list-box"
+        id="test-dropdown"
         onClick={[Function]}
         onKeyDown={[Function]}
       >
@@ -196,12 +198,14 @@ exports[`Dropdown should render custom item components 1`] = `
     <ListBox
       className="bx--dropdown bx--dropdown--open"
       disabled={false}
+      id="test-dropdown"
       isOpen={true}
       light={false}
       type="default"
     >
       <div
         className="bx--dropdown bx--dropdown--open bx--list-box bx--list-box--expanded"
+        id="test-dropdown"
         onClick={[Function]}
         onKeyDown={[Function]}
       >
@@ -500,12 +504,14 @@ exports[`Dropdown should render with strings as items 1`] = `
     <ListBox
       className="bx--dropdown bx--dropdown--open"
       disabled={false}
+      id="test-dropdown"
       isOpen={true}
       light={false}
       type="default"
     >
       <div
         className="bx--dropdown bx--dropdown--open bx--list-box bx--list-box--expanded"
+        id="test-dropdown"
         onClick={[Function]}
         onKeyDown={[Function]}
       >

diff --git a/packages/react/src/components/ListBox/ListBoxField.js b/packages/react/src/components/ListBox/ListBoxField.js
@@ -11,36 +11,42 @@ import PropTypes from 'prop-types';
 
 const { prefix } = settings;
 
-export const translationIds = {}; // No longer used, left export for backward-compatibility
+// No longer used, left export for backward-compatibility
+export const translationIds = {};
 
 /**
  * `ListBoxField` is responsible for creating the containing node for valid
  * elements inside of a field. It also provides a11y-related attributes like
  * `role` to make sure a user can focus the given field.
  */
-const ListBoxField = ({ children, id, disabled, tabIndex, ...rest }) => (
-  <div
-    role={rest['aria-expanded'] ? 'combobox' : rest.role || 'combobox'}
-    aria-owns={(rest['aria-expanded'] && `${id}__menu`) || null}
-    aria-controls={(rest['aria-expanded'] && `${id}__menu`) || null}
-    aria-haspopup="listbox"
-    className={`${prefix}--list-box__field`}
-    tabIndex={(!disabled && tabIndex) || -1}
-    {...rest}>
-    {children}
-  </div>
-);
+function ListBoxField({ children, disabled, tabIndex, ...rest }) {
+  return (
+    <div
+      className={`${prefix}--list-box__field`}
+      tabIndex={(!disabled && tabIndex) || -1}
+      {...rest}>
+      {children}
+    </div>
+  );
+}
 
 ListBoxField.propTypes = {
   /**
-   * Provide the contents of your ListBoxField
+   * Typically set by `getToggleButtonProps`, this should specify whether the
+   * field has a popup.
    */
-  children: PropTypes.node,
+  'aria-haspopup': PropTypes.oneOfType([PropTypes.string, PropTypes.bool]),
 
   /**
-   * Specify a custom `id`
+   * The role for the component, should be set by `getToggleButtonProps` coming
+   * from Downshift
    */
-  id: PropTypes.string.isRequired,
+  role: PropTypes.string,
+
+  /**
+   * Provide the contents of your ListBoxField
+   */
+  children: PropTypes.node,
 
   /**
    * Specify if the parent <ListBox> is disabled

diff --git a/packages/react/src/components/ListBox/__tests__/ListBoxField-test.js b/packages/react/src/components/ListBox/__tests__/ListBoxField-test.js
@@ -32,19 +32,4 @@ describe('ListBoxField', () => {
     const wrapper = mount(<ListBox.Field id="test-listbox" disabled />);
     expect(wrapper.children().prop('tabIndex')).toBe(-1);
   });
-
-  it('should set `aria-owns` based when expanded', () => {
-    const wrapper = mount(
-      <ListBox.Field id="test-listbox" aria-expanded>
-        <ListBox.Selection clearSelection={jest.fn()} />
-      </ListBox.Field>
-    );
-    expect(wrapper.find('div[aria-expanded]').props()['aria-owns']).toBe(
-      'test-listbox__menu'
-    );
-    expect(wrapper.find('div[aria-expanded]').props()['aria-controls']).toBe(
-      'test-listbox__menu'
-    );
-    expect(wrapper).toMatchSnapshot();
-  });
 });
diff --git a/packages/react/src/components/ListBox/__tests__/__snapshots__/ListBox-test.js.snap b/packages/react/src/components/ListBox/__tests__/__snapshots__/ListBox-test.js.snap
@@ -17,11 +17,8 @@ exports[`ListBox should render 1`] = `
       id="test-listbox"
     >
       <div
-        aria-controls={null}
-        aria-haspopup="listbox"
-        aria-owns={null}
         className="bx--list-box__field"
-        role="combobox"
+        id="test-listbox"
         tabIndex={-1}
       />
     </ListBoxField>

diff --git a/packages/react/src/components/ListBox/__tests__/__snapshots__/ListBoxField-test.js.snap b/packages/react/src/components/ListBox/__tests__/__snapshots__/ListBoxField-test.js.snap
@@ -5,69 +5,8 @@ exports[`ListBoxField should render 1`] = `
   id="test-listbox"
 >
   <div
-    aria-controls={null}
-    aria-haspopup="listbox"
-    aria-owns={null}
     className="bx--list-box__field"
-    role="combobox"
-    tabIndex={-1}
-  >
-    <ListBoxSelection
-      clearSelection={[MockFunction]}
-      translateWithId={[Function]}
-    >
-      <div
-        aria-label="Clear Selection"
-        className="bx--list-box__selection"
-        onClick={[Function]}
-        onKeyDown={[Function]}
-        role="button"
-        tabIndex={0}
-        title="Clear selected item"
-      >
-        <ForwardRef(Close16)>
-          <Icon
-            fill="currentColor"
-            height={16}
-            preserveAspectRatio="xMidYMid meet"
-            viewBox="0 0 32 32"
-            width={16}
-            xmlns="http://www.w3.org/2000/svg"
-          >
-            <svg
-              aria-hidden={true}
-              fill="currentColor"
-              focusable="false"
-              height={16}
-              preserveAspectRatio="xMidYMid meet"
-              viewBox="0 0 32 32"
-              width={16}
-              xmlns="http://www.w3.org/2000/svg"
-            >
-              <path
-                d="M24 9.4L22.6 8 16 14.6 9.4 8 8 9.4 14.6 16 8 22.6 9.4 24 16 17.4 22.6 24 24 22.6 17.4 16 24 9.4z"
-              />
-            </svg>
-          </Icon>
-        </ForwardRef(Close16)>
-      </div>
-    </ListBoxSelection>
-  </div>
-</ListBoxField>
-`;
-
-exports[`ListBoxField should set \`aria-owns\` based when expanded 1`] = `
-<ListBoxField
-  aria-expanded={true}
-  id="test-listbox"
->
-  <div
-    aria-controls="test-listbox__menu"
-    aria-expanded={true}
-    aria-haspopup="listbox"
-    aria-owns="test-listbox__menu"
-    className="bx--list-box__field"
-    role="combobox"
+    id="test-listbox"
     tabIndex={-1}
   >
     <ListBoxSelection

diff --git a/packages/react/src/components/MultiSelect/MultiSelect.js b/packages/react/src/components/MultiSelect/MultiSelect.js
@@ -114,7 +114,6 @@ function MultiSelect({
   const helperId = !helperText
     ? undefined
     : `multiselect-helper-text-${multiSelectInstanceId}`;
-  const labelId = `multiselect-label-${multiSelectInstanceId}`;
   const fieldLabelId = `multiselect-field-label-${multiSelectInstanceId}`;
   const helperClasses = cx(`${prefix}--form__helper-text`, {
     [`${prefix}--form__helper-text--disabled`]: disabled,
@@ -172,25 +171,24 @@ function MultiSelect({
   return (
     <div className={wrapperClasses}>
       {titleText && (
-        <label id={labelId} className={titleClasses} {...getLabelProps()}>
+        <label className={titleClasses} {...getLabelProps()}>
           {titleText}
         </label>
       )}
       <ListBox
-        id={id}
         type={type}
         size={size}
         className={className}
         disabled={disabled}
         light={light}
         invalid={invalid}
         invalidText={invalidText}
-        isOpen={isOpen}>
+        isOpen={isOpen}
+        id={id}>
         {invalid && (
           <WarningFilled16 className={`${prefix}--list-box__invalid-icon`} />
         )}
         <button
-          id={id}
           className={`${prefix}--list-box__field`}
           disabled={disabled}
           aria-disabled={disabled}

diff --git a/...act/src/components/MultiSelect/__tests__/__snapshots__/FilterableMultiSelect-test.js.snap b/...act/src/components/MultiSelect/__tests__/__snapshots__/FilterableMultiSelect-test.js.snap
@@ -112,12 +112,11 @@ exports[`MultiSelect.Filterable should render 1`] = `
               type="button"
             >
               <div
-                aria-controls={null}
                 aria-haspopup={true}
                 aria-labelledby="test-filterable-multiselect-label"
-                aria-owns={null}
                 className="bx--list-box__field"
                 data-toggle={true}
+                id="test-filterable-multiselect"
                 onBlur={[Function]}
                 onClick={[Function]}
                 onKeyDown={[Function]}