From 9878f7eb912d731a5b5230e7d537618d1c41051b Mon Sep 17 00:00:00 2001
From: Dylan Kilgore <dkilgore@eightfold.ai>
Date: Thu, 4 May 2023 13:03:26 -0700
Subject: [PATCH 1/3] fix: select: ensure aria attributes target the intended
 element

---
 src/components/Dropdown/Dropdown.tsx          | 22 +++++
 src/components/Dropdown/Dropdown.types.ts     |  6 ++
 src/components/Select/Select.tsx              |  2 +-
 .../Select/__snapshots__/Select.test.tsx.snap | 90 ++++++++-----------
 4 files changed, 65 insertions(+), 55 deletions(-)

diff --git a/src/components/Dropdown/Dropdown.tsx b/src/components/Dropdown/Dropdown.tsx
index cd16629b4..18fbb2936 100644
--- a/src/components/Dropdown/Dropdown.tsx
+++ b/src/components/Dropdown/Dropdown.tsx
@@ -40,6 +40,7 @@ export const Dropdown: FC<DropdownProps> = React.memo(
   React.forwardRef<DropdownRef, DropdownProps>(
     (
       {
+        ariaRef,
         children,
         classNames,
         closeOnDropdownClick = true,
@@ -243,6 +244,27 @@ export const Dropdown: FC<DropdownProps> = React.memo(
         if (child.props.id && dropdownReferenceId !== child.props.id) {
           setReferenceElementId(child.props.id);
         }
+        // If there's an ariaRef, apply the a11y attributes to it, rather than the immediate child.
+        if (ariaRef && ariaRef.current) {
+          ariaRef.current.setAttribute('aria-controls', dropdownId);
+          ariaRef.current.setAttribute('aria-expanded', `${mergedVisible}`);
+          ariaRef.current.setAttribute('aria-haspopup', 'true');
+
+          if (!ariaRef.current.role) {
+            ariaRef.current.setAttribute('role', 'button');
+          }
+
+          return cloneElement(child, {
+            ...{
+              [TRIGGER_TO_HANDLER_MAP_ON_ENTER[trigger]]: toggle(true),
+            },
+            id: dropdownReferenceId,
+            onClick: handleReferenceClick,
+            onKeyDown: handleReferenceKeyDown,
+            className: referenceWrapperClassNames,
+          });
+        }
+
         return cloneElement(child, {
           ...{
             [TRIGGER_TO_HANDLER_MAP_ON_ENTER[trigger]]: toggle(true),
diff --git a/src/components/Dropdown/Dropdown.types.ts b/src/components/Dropdown/Dropdown.types.ts
index 276f2e28f..74cf16c91 100644
--- a/src/components/Dropdown/Dropdown.types.ts
+++ b/src/components/Dropdown/Dropdown.types.ts
@@ -16,6 +16,12 @@ export const TRIGGER_TO_HANDLER_MAP_ON_LEAVE = {
 };
 
 export interface DropdownProps {
+  /**
+   * The ref of element that should implement the following props:
+   * 'aria-controls', 'aria-expanded', 'aria-haspopup', 'role'
+   * @default child
+   */
+  ariaRef?: React.MutableRefObject<HTMLElement>;
   /**
    * Class names of the main wrapper
    */
diff --git a/src/components/Select/Select.tsx b/src/components/Select/Select.tsx
index 50a5f312b..e0b4dde59 100644
--- a/src/components/Select/Select.tsx
+++ b/src/components/Select/Select.tsx
@@ -722,6 +722,7 @@ export const Select: FC<SelectProps> = React.forwardRef(
           {/* When Dropdown is hidden, place Pills outside the reference element */}
           {!dropdownVisible && showPills() ? getPills() : null}
           <Dropdown
+            ariaRef={inputRef}
             width={dropdownWidth}
             closeOnReferenceClick={closeOnReferenceClick}
             {...dropdownProps}
@@ -731,7 +732,6 @@ export const Select: FC<SelectProps> = React.forwardRef(
             onVisibleChange={(isVisible) => setDropdownVisibility(isVisible)}
             overlay={isLoading ? spinner : <OptionMenu options={options} />}
             showDropdown={showDropdown}
-            tabIndex={-1} // Defer focus to the TextInput
             visible={
               dropdownVisible &&
               (showEmptyDropdown ||
diff --git a/src/components/Select/__snapshots__/Select.test.tsx.snap b/src/components/Select/__snapshots__/Select.test.tsx.snap
index 346ddc830..29a681bcc 100644
--- a/src/components/Select/__snapshots__/Select.test.tsx.snap
+++ b/src/components/Select/__snapshots__/Select.test.tsx.snap
@@ -9,13 +9,8 @@ exports[`Select Renders with default value 1`] = `
       class="select-dropdown-main-wrapper main-wrapper"
     >
       <div
-        aria-controls="dropdown-"
-        aria-expanded="false"
-        aria-haspopup="true"
         class="reference-wrapper select-input-wrapper"
         id="dropdown-reference"
-        role="button"
-        tabindex="-1"
       >
         <div
           class="input-wrapper input-medium input-stretch right-icon"
@@ -27,12 +22,15 @@ exports[`Select Renders with default value 1`] = `
               class="select-input-group input-group right-icon"
             >
               <input
+                aria-controls="dropdown-"
                 aria-disabled="false"
+                aria-expanded="false"
+                aria-haspopup="true"
                 class="select-input input-medium with-icon-button input-stretch right-icon clear-disabled clear-not-visible"
                 id="input-"
                 placeholder="Select"
                 readonly=""
-                role="combobox"
+                role="button"
                 tabindex="0"
                 type="text"
                 value="Option 2"
@@ -152,13 +150,8 @@ exports[`Select Renders with default values when multiple 1`] = `
       class="select-dropdown-main-wrapper main-wrapper"
     >
       <div
-        aria-controls="dropdown-"
-        aria-expanded="false"
-        aria-haspopup="true"
         class="reference-wrapper select-input-wrapper"
         id="dropdown-reference"
-        role="button"
-        tabindex="-1"
       >
         <div
           class="input-wrapper input-medium input-stretch right-icon"
@@ -170,12 +163,15 @@ exports[`Select Renders with default values when multiple 1`] = `
               class="select-input-group input-group right-icon"
             >
               <input
+                aria-controls="dropdown-"
                 aria-disabled="false"
+                aria-expanded="false"
+                aria-haspopup="true"
                 class="select-input input-medium with-icon-button input-stretch right-icon clear-disabled clear-not-visible"
                 id="input-"
                 placeholder=""
                 readonly=""
-                role="combobox"
+                role="button"
                 tabindex="0"
                 type="text"
                 value=""
@@ -227,13 +223,8 @@ exports[`Select Renders without crashing 1`] = `
       class="select-dropdown-main-wrapper main-wrapper"
     >
       <div
-        aria-controls="dropdown-"
-        aria-expanded="false"
-        aria-haspopup="true"
         class="reference-wrapper select-input-wrapper"
         id="dropdown-reference"
-        role="button"
-        tabindex="-1"
       >
         <div
           class="input-wrapper input-medium input-stretch right-icon"
@@ -245,12 +236,15 @@ exports[`Select Renders without crashing 1`] = `
               class="select-input-group input-group right-icon"
             >
               <input
+                aria-controls="dropdown-"
                 aria-disabled="false"
+                aria-expanded="false"
+                aria-haspopup="true"
                 class="select-input input-medium with-icon-button input-stretch right-icon clear-disabled clear-not-visible"
                 id="input-"
                 placeholder="Select test"
                 readonly=""
-                role="combobox"
+                role="button"
                 tabindex="0"
                 type="text"
                 value=""
@@ -301,13 +295,8 @@ exports[`Select Select is large 1`] = `
     class="select-dropdown-main-wrapper main-wrapper"
   >
     <div
-      aria-controls="dropdown-"
-      aria-expanded="false"
-      aria-haspopup="true"
       class="reference-wrapper select-input-wrapper"
       id="dropdown-reference"
-      role="button"
-      tabindex="-1"
     >
       <div
         class="input-wrapper input-large input-stretch right-icon"
@@ -319,12 +308,15 @@ exports[`Select Select is large 1`] = `
             class="select-input-group input-group right-icon"
           >
             <input
+              aria-controls="dropdown-"
               aria-disabled="false"
+              aria-expanded="false"
+              aria-haspopup="true"
               class="select-input input-large with-icon-button input-stretch right-icon clear-disabled clear-not-visible"
               id="input-"
               placeholder="Select"
               readonly=""
-              role="combobox"
+              role="button"
               tabindex="0"
               type="text"
               value=""
@@ -374,13 +366,8 @@ exports[`Select Select is medium 1`] = `
     class="select-dropdown-main-wrapper main-wrapper"
   >
     <div
-      aria-controls="dropdown-"
-      aria-expanded="false"
-      aria-haspopup="true"
       class="reference-wrapper select-input-wrapper"
       id="dropdown-reference"
-      role="button"
-      tabindex="-1"
     >
       <div
         class="input-wrapper input-medium input-stretch right-icon"
@@ -392,12 +379,15 @@ exports[`Select Select is medium 1`] = `
             class="select-input-group input-group right-icon"
           >
             <input
+              aria-controls="dropdown-"
               aria-disabled="false"
+              aria-expanded="false"
+              aria-haspopup="true"
               class="select-input input-medium with-icon-button input-stretch right-icon clear-disabled clear-not-visible"
               id="input-"
               placeholder="Select"
               readonly=""
-              role="combobox"
+              role="button"
               tabindex="0"
               type="text"
               value=""
@@ -447,13 +437,8 @@ exports[`Select Select is pill shaped 1`] = `
     class="select-dropdown-main-wrapper main-wrapper"
   >
     <div
-      aria-controls="dropdown-"
-      aria-expanded="false"
-      aria-haspopup="true"
       class="reference-wrapper select-input-wrapper"
       id="dropdown-reference"
-      role="button"
-      tabindex="-1"
     >
       <div
         class="input-wrapper input-medium input-stretch right-icon"
@@ -465,12 +450,15 @@ exports[`Select Select is pill shaped 1`] = `
             class="select-input-group input-group right-icon"
           >
             <input
+              aria-controls="dropdown-"
               aria-disabled="false"
+              aria-expanded="false"
+              aria-haspopup="true"
               class="select-input input-medium pill-shape pill-shape-with-icon-button input-stretch right-icon clear-disabled clear-not-visible"
               id="input-"
               placeholder="Select"
               readonly=""
-              role="combobox"
+              role="button"
               tabindex="0"
               type="text"
               value=""
@@ -520,13 +508,8 @@ exports[`Select Select is rectangle shaped 1`] = `
     class="select-dropdown-main-wrapper main-wrapper"
   >
     <div
-      aria-controls="dropdown-"
-      aria-expanded="false"
-      aria-haspopup="true"
       class="reference-wrapper select-input-wrapper"
       id="dropdown-reference"
-      role="button"
-      tabindex="-1"
     >
       <div
         class="input-wrapper input-medium input-stretch right-icon"
@@ -538,12 +521,15 @@ exports[`Select Select is rectangle shaped 1`] = `
             class="select-input-group input-group right-icon"
           >
             <input
+              aria-controls="dropdown-"
               aria-disabled="false"
+              aria-expanded="false"
+              aria-haspopup="true"
               class="select-input input-medium with-icon-button input-stretch right-icon clear-disabled clear-not-visible"
               id="input-"
               placeholder="Select"
               readonly=""
-              role="combobox"
+              role="button"
               tabindex="0"
               type="text"
               value=""
@@ -593,13 +579,8 @@ exports[`Select Select is small 1`] = `
     class="select-dropdown-main-wrapper main-wrapper"
   >
     <div
-      aria-controls="dropdown-"
-      aria-expanded="false"
-      aria-haspopup="true"
       class="reference-wrapper select-input-wrapper"
       id="dropdown-reference"
-      role="button"
-      tabindex="-1"
     >
       <div
         class="input-wrapper input-small input-stretch right-icon"
@@ -611,12 +592,15 @@ exports[`Select Select is small 1`] = `
             class="select-input-group input-group right-icon"
           >
             <input
+              aria-controls="dropdown-"
               aria-disabled="false"
+              aria-expanded="false"
+              aria-haspopup="true"
               class="select-input input-small with-icon-button input-stretch right-icon clear-disabled clear-not-visible"
               id="input-"
               placeholder="Select"
               readonly=""
-              role="combobox"
+              role="button"
               tabindex="0"
               type="text"
               value=""
@@ -666,13 +650,8 @@ exports[`Select Select is underline shaped 1`] = `
     class="select-dropdown-main-wrapper main-wrapper"
   >
     <div
-      aria-controls="dropdown-"
-      aria-expanded="false"
-      aria-haspopup="true"
       class="reference-wrapper select-input-wrapper"
       id="dropdown-reference"
-      role="button"
-      tabindex="-1"
     >
       <div
         class="input-wrapper underline input-medium input-stretch right-icon"
@@ -684,12 +663,15 @@ exports[`Select Select is underline shaped 1`] = `
             class="select-input-group input-group right-icon"
           >
             <input
+              aria-controls="dropdown-"
               aria-disabled="false"
+              aria-expanded="false"
+              aria-haspopup="true"
               class="select-input input-medium with-icon-button underline input-stretch right-icon clear-disabled clear-not-visible"
               id="input-"
               placeholder="Select"
               readonly=""
-              role="combobox"
+              role="button"
               tabindex="0"
               type="text"
               value=""

From a60ac5019939f7d1637380eeb6f9d85f439a634f Mon Sep 17 00:00:00 2001
From: Dylan Kilgore <dkilgore@eightfold.ai>
Date: Thu, 4 May 2023 13:32:39 -0700
Subject: [PATCH 2/3] chore: dropdown: add ariaref ut and refine role null
 check

---
 src/components/Dropdown/Dropdown.test.tsx     | 87 +++++++++++++++----
 src/components/Dropdown/Dropdown.tsx          |  2 +-
 .../Select/__snapshots__/Select.test.tsx.snap | 18 ++--
 3 files changed, 78 insertions(+), 29 deletions(-)

diff --git a/src/components/Dropdown/Dropdown.test.tsx b/src/components/Dropdown/Dropdown.test.tsx
index 563fe4fd9..910189657 100644
--- a/src/components/Dropdown/Dropdown.test.tsx
+++ b/src/components/Dropdown/Dropdown.test.tsx
@@ -1,4 +1,4 @@
-import React, { useState } from 'react';
+import React, { useRef, useState } from 'react';
 import Enzyme from 'enzyme';
 import Adapter from '@wojtekmaj/enzyme-adapter-react-17';
 import MatchMediaMock from 'jest-matchmedia-mock';
@@ -9,8 +9,10 @@ import {
   ButtonWidth,
   DefaultButton,
 } from '../Button';
-import { IconName } from '../Icon';
+import { Icon, IconName } from '../Icon';
 import { List } from '../List';
+import { Stack } from '../Stack';
+import { TextInput } from '../Inputs';
 import { render, screen, waitFor } from '@testing-library/react';
 
 Enzyme.configure({ adapter: new Adapter() });
@@ -46,26 +48,26 @@ const Overlay = () => (
   />
 );
 
+const dropdownProps: object = {
+  trigger: 'click',
+  classNames: 'my-dropdown-class',
+  style: {},
+  dropdownClassNames: 'my-dropdown-class',
+  dropdownStyle: {
+    color: 'red',
+  },
+  placement: 'bottom-start',
+  overlay: Overlay(),
+  offset: 0,
+  positionStrategy: 'absolute',
+  disabled: false,
+  closeOnDropdownClick: true,
+  portal: false,
+};
+
 const DropdownComponent = (): JSX.Element => {
   const [visible, setVisibility] = useState(false);
 
-  const dropdownProps: object = {
-    trigger: 'click',
-    classNames: 'my-dropdown-class',
-    style: {},
-    dropdownClassNames: 'my-dropdown-class',
-    dropdownStyle: {
-      color: 'red',
-    },
-    placement: 'bottom-start',
-    overlay: Overlay(),
-    offset: 0,
-    positionStrategy: 'absolute',
-    disabled: false,
-    closeOnDropdownClick: true,
-    portal: false,
-  };
-
   return (
     <Dropdown
       {...dropdownProps}
@@ -84,6 +86,36 @@ const DropdownComponent = (): JSX.Element => {
   );
 };
 
+const ComplexDropdownComponent = (): JSX.Element => {
+  const inputRef: React.MutableRefObject<HTMLInputElement> =
+    useRef<HTMLInputElement>(null);
+  const [visible, setVisibility] = useState(false);
+
+  return (
+    <Dropdown
+      {...dropdownProps}
+      ariaRef={inputRef}
+      onVisibleChange={(isVisible) => setVisibility(isVisible)}
+    >
+      <div id="wrapper">
+        <Stack direction="horizontal" flexGap="xl">
+          <Icon path={IconName.mdiAccount} />
+          <TextInput
+            ref={inputRef}
+            placeholder={'Select'}
+            iconProps={{
+              path: IconName.mdiChevronDown,
+              rotate: visible ? 180 : 0,
+            }}
+            role="combobox"
+            data-testid="test-input-id"
+          />
+        </Stack>
+      </div>
+    </Dropdown>
+  );
+};
+
 describe('Dropdown', () => {
   beforeAll(() => {
     matchMedia = new MatchMediaMock();
@@ -132,4 +164,21 @@ describe('Dropdown', () => {
     const dropdownButton = screen.getByRole('button');
     expect(dropdownButton.id).toBe('test-button-id');
   });
+
+  test('Should support ariaRef prop for complex dropdown references', async () => {
+    const { container } = render(<ComplexDropdownComponent />);
+    const dropdownAriaRef = screen.getByTestId('test-input-id');
+    expect(dropdownAriaRef.getAttribute('aria-controls')).toBeTruthy();
+    expect(dropdownAriaRef.getAttribute('aria-expanded')).toBe('false');
+    expect(dropdownAriaRef.getAttribute('aria-haspopup')).toBe('true');
+    expect(dropdownAriaRef.getAttribute('role')).toBe('combobox');
+    dropdownAriaRef.click();
+    await waitFor(() => screen.getByText('User profile 1'));
+    const option1 = screen.getByText('User profile 1');
+    expect(option1).toBeTruthy();
+    expect(container.querySelector('.dropdown-wrapper')?.classList).toContain(
+      'my-dropdown-class'
+    );
+    expect(dropdownAriaRef.getAttribute('aria-expanded')).toBe('true');
+  });
 });
diff --git a/src/components/Dropdown/Dropdown.tsx b/src/components/Dropdown/Dropdown.tsx
index 18fbb2936..ecdaa950e 100644
--- a/src/components/Dropdown/Dropdown.tsx
+++ b/src/components/Dropdown/Dropdown.tsx
@@ -250,7 +250,7 @@ export const Dropdown: FC<DropdownProps> = React.memo(
           ariaRef.current.setAttribute('aria-expanded', `${mergedVisible}`);
           ariaRef.current.setAttribute('aria-haspopup', 'true');
 
-          if (!ariaRef.current.role) {
+          if (ariaRef.current.role === null) {
             ariaRef.current.setAttribute('role', 'button');
           }
 
diff --git a/src/components/Select/__snapshots__/Select.test.tsx.snap b/src/components/Select/__snapshots__/Select.test.tsx.snap
index 29a681bcc..0e5699393 100644
--- a/src/components/Select/__snapshots__/Select.test.tsx.snap
+++ b/src/components/Select/__snapshots__/Select.test.tsx.snap
@@ -30,7 +30,7 @@ exports[`Select Renders with default value 1`] = `
                 id="input-"
                 placeholder="Select"
                 readonly=""
-                role="button"
+                role="combobox"
                 tabindex="0"
                 type="text"
                 value="Option 2"
@@ -171,7 +171,7 @@ exports[`Select Renders with default values when multiple 1`] = `
                 id="input-"
                 placeholder=""
                 readonly=""
-                role="button"
+                role="combobox"
                 tabindex="0"
                 type="text"
                 value=""
@@ -244,7 +244,7 @@ exports[`Select Renders without crashing 1`] = `
                 id="input-"
                 placeholder="Select test"
                 readonly=""
-                role="button"
+                role="combobox"
                 tabindex="0"
                 type="text"
                 value=""
@@ -316,7 +316,7 @@ exports[`Select Select is large 1`] = `
               id="input-"
               placeholder="Select"
               readonly=""
-              role="button"
+              role="combobox"
               tabindex="0"
               type="text"
               value=""
@@ -387,7 +387,7 @@ exports[`Select Select is medium 1`] = `
               id="input-"
               placeholder="Select"
               readonly=""
-              role="button"
+              role="combobox"
               tabindex="0"
               type="text"
               value=""
@@ -458,7 +458,7 @@ exports[`Select Select is pill shaped 1`] = `
               id="input-"
               placeholder="Select"
               readonly=""
-              role="button"
+              role="combobox"
               tabindex="0"
               type="text"
               value=""
@@ -529,7 +529,7 @@ exports[`Select Select is rectangle shaped 1`] = `
               id="input-"
               placeholder="Select"
               readonly=""
-              role="button"
+              role="combobox"
               tabindex="0"
               type="text"
               value=""
@@ -600,7 +600,7 @@ exports[`Select Select is small 1`] = `
               id="input-"
               placeholder="Select"
               readonly=""
-              role="button"
+              role="combobox"
               tabindex="0"
               type="text"
               value=""
@@ -671,7 +671,7 @@ exports[`Select Select is underline shaped 1`] = `
               id="input-"
               placeholder="Select"
               readonly=""
-              role="button"
+              role="combobox"
               tabindex="0"
               type="text"
               value=""

From 283c1098fdd80e008054f21e57face8eaf28588a Mon Sep 17 00:00:00 2001
From: Dylan Kilgore <dkilgore@eightfold.ai>
Date: Thu, 4 May 2023 14:53:22 -0700
Subject: [PATCH 3/3] chore: dropdown: update role null check to handle all
 elements

---
 src/components/Dropdown/Dropdown.tsx | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/components/Dropdown/Dropdown.tsx b/src/components/Dropdown/Dropdown.tsx
index ecdaa950e..efa8b2073 100644
--- a/src/components/Dropdown/Dropdown.tsx
+++ b/src/components/Dropdown/Dropdown.tsx
@@ -250,7 +250,7 @@ export const Dropdown: FC<DropdownProps> = React.memo(
           ariaRef.current.setAttribute('aria-expanded', `${mergedVisible}`);
           ariaRef.current.setAttribute('aria-haspopup', 'true');
 
-          if (ariaRef.current.role === null) {
+          if (!ariaRef.current.hasAttribute('role')) {
             ariaRef.current.setAttribute('role', 'button');
           }