From 20314b5f560921b8e762790dfa45386bf7916a54 Mon Sep 17 00:00:00 2001
From: Silviu Alexandru Avram <silviu.avram91@gmail.com>
Date: Mon, 20 Mar 2023 09:46:15 +0200
Subject: [PATCH] feat: getter prop with aria-label will not add
 aria-labelledby (#1488)

---
 README.md                                         |  6 ++++++
 src/__tests__/downshift.aria.js                   | 15 ++++++++++++---
 src/downshift.js                                  |  2 +-
 src/hooks/useCombobox/README.md                   |  6 ++++++
 .../useCombobox/__tests__/getInputProps.test.js   |  9 +++++++++
 .../useCombobox/__tests__/getMenuProps.test.js    |  9 +++++++++
 src/hooks/useCombobox/index.js                    |  6 ++++--
 src/hooks/useSelect/README.md                     |  6 ++++++
 .../useSelect/__tests__/getMenuProps.test.js      |  8 +++++---
 .../__tests__/getToggleButtonProps.test.js        |  9 +++++++++
 src/hooks/useSelect/index.js                      |  7 ++++---
 11 files changed, 71 insertions(+), 12 deletions(-)

diff --git a/README.md b/README.md
index 746419fca..7fe38f39b 100644
--- a/README.md
+++ b/README.md
@@ -747,6 +747,12 @@ Optional properties:
   from `getInputProps` and a `disabled` prop will be returned (effectively
   disabling the input).
 
+- `aria-label`: By default the menu will add an `aria-labelledby` that refers to
+  the `<label>` rendered with `getLabelProps`. However, if you provide
+  `aria-label` to give a more specific label that describes the options
+  available, then `aria-labelledby` will not be provided and screen readers can
+  use your `aria-label` instead.
+
 #### `getLabelProps`
 
 This method should be applied to the `label` you render. It is useful for
diff --git a/src/__tests__/downshift.aria.js b/src/__tests__/downshift.aria.js
index 5df26f747..3d8f19ccd 100644
--- a/src/__tests__/downshift.aria.js
+++ b/src/__tests__/downshift.aria.js
@@ -24,7 +24,7 @@ test('can override the ids', () => {
   expect(container.firstChild).toMatchSnapshot()
 })
 
-test('if aria-label is provided to the menu then aria-labelledby is not applied to the label', () => {
+test('if aria-label is provided to the menu then aria-labelledby is not applied to the menu', () => {
   const customLabel = 'custom menu label'
   const {menu} = renderDownshift({
     menuProps: {'aria-label': customLabel},
@@ -33,7 +33,16 @@ test('if aria-label is provided to the menu then aria-labelledby is not applied
   expect(menu).toHaveAttribute('aria-label', customLabel)
 })
 
-function renderDownshift({renderFn, props, menuProps} = {}) {
+test('if aria-label is provided to the input then aria-labelledby is not applied to the input', () => {
+  const customLabel = 'custom menu label'
+  const {input} = renderDownshift({
+    inputProps: {'aria-label': customLabel},
+  })
+  expect(input).not.toHaveAttribute('aria-labelledby')
+  expect(input).toHaveAttribute('aria-label', customLabel)
+})
+
+function renderDownshift({renderFn, props, menuProps, inputProps} = {}) {
   function defaultRenderFn({
     getInputProps,
     getToggleButtonProps,
@@ -46,7 +55,7 @@ function renderDownshift({renderFn, props, menuProps} = {}) {
         <label data-testid="label" {...getLabelProps()}>
           label
         </label>
-        <input data-testid="input" {...getInputProps()} />
+        <input data-testid="input" {...getInputProps(inputProps)} />
         <button data-testid="button" {...getToggleButtonProps()} />
         <ul data-testid="menu" {...getMenuProps(menuProps)}>
           <li data-testid="item-0" {...getItemProps({item: 'item', index: 0})}>
diff --git a/src/downshift.js b/src/downshift.js
index 03166bd29..edc4e1052 100644
--- a/src/downshift.js
+++ b/src/downshift.js
@@ -826,7 +826,7 @@ class Downshift extends Component {
           ? this.getItemId(highlightedIndex)
           : null,
       'aria-controls': isOpen ? this.menuId : null,
-      'aria-labelledby': this.labelId,
+      'aria-labelledby': rest && rest['aria-label'] ? undefined : this.labelId,
       // https://developer.mozilla.org/en-US/docs/Web/Security/Securing_your_site/Turning_off_form_autocompletion
       // revert back since autocomplete="nope" is ignored on latest Chrome and Opera
       autoComplete: 'off',
diff --git a/src/hooks/useCombobox/README.md b/src/hooks/useCombobox/README.md
index 7b46f65ff..775c7b414 100644
--- a/src/hooks/useCombobox/README.md
+++ b/src/hooks/useCombobox/README.md
@@ -891,6 +891,12 @@ Optional properties:
   However, if you are just rendering a primitive component like `<div>`, there
   is no need to specify this property. It defaults to `ref`.
 
+- `aria-label`: By default the input will add an `aria-labelledby` that refers to
+  the `<label>` rendered with `getLabelProps`. However, if you provide
+  `aria-label` to give a more specific label that describes the options
+  available, then `aria-labelledby` will not be provided and screen readers can
+  use your `aria-label` instead.
+
 In some cases, you might want to completely bypass the `refKey` check. Then you
 can provide the object `{suppressRefError : true}` as the second argument to
 `getInput`. **Please use it with extreme care and only if you are absolutely
diff --git a/src/hooks/useCombobox/__tests__/getInputProps.test.js b/src/hooks/useCombobox/__tests__/getInputProps.test.js
index 3bdce8696..24987d14c 100644
--- a/src/hooks/useCombobox/__tests__/getInputProps.test.js
+++ b/src/hooks/useCombobox/__tests__/getInputProps.test.js
@@ -157,6 +157,15 @@ describe('getInputProps', () => {
       expect(inputProps.onFocus).toBeUndefined()
       expect(inputProps.disabled).toBe(true)
     })
+
+    test("do not assign 'aria-labelledby' if it has aria-label", () => {
+      const ariaLabel = 'not so fast'
+      const {result} = renderUseCombobox()
+      const inputProps = result.current.getInputProps({'aria-label': ariaLabel})
+
+      expect(inputProps['aria-labelledby']).toBeUndefined()
+      expect(inputProps['aria-label']).toBe(ariaLabel)
+    })
   })
 
   describe('user props', () => {
diff --git a/src/hooks/useCombobox/__tests__/getMenuProps.test.js b/src/hooks/useCombobox/__tests__/getMenuProps.test.js
index 7e23a99be..357b32282 100644
--- a/src/hooks/useCombobox/__tests__/getMenuProps.test.js
+++ b/src/hooks/useCombobox/__tests__/getMenuProps.test.js
@@ -52,6 +52,15 @@ describe('getMenuProps', () => {
 
       expect(menuProps.role).toEqual('listbox')
     })
+
+    test("do not assign 'aria-labelledby' if it has aria-label", () => {
+      const ariaLabel = 'not so fast'
+      const {result} = renderUseCombobox()
+      const menuProps = result.current.getMenuProps({'aria-label': ariaLabel})
+
+      expect(menuProps['aria-labelledby']).toBeUndefined()
+      expect(menuProps['aria-label']).toBe(ariaLabel)
+    })
   })
 
   describe('user props', () => {
diff --git a/src/hooks/useCombobox/index.js b/src/hooks/useCombobox/index.js
index b08ebd2dd..3e75b54c0 100644
--- a/src/hooks/useCombobox/index.js
+++ b/src/hooks/useCombobox/index.js
@@ -270,7 +270,8 @@ function useCombobox(userProps = {}) {
         }),
         id: elementIds.menuId,
         role: 'listbox',
-        'aria-labelledby': elementIds.labelId,
+        'aria-labelledby':
+          rest && rest['aria-label'] ? undefined : `${elementIds.labelId}`,
         onMouseLeave: callAllEventHandlers(onMouseLeave, () => {
           dispatch({
             type: stateChangeTypes.MenuMouseLeave,
@@ -480,7 +481,8 @@ function useCombobox(userProps = {}) {
         'aria-autocomplete': 'list',
         'aria-controls': elementIds.menuId,
         'aria-expanded': latestState.isOpen,
-        'aria-labelledby': elementIds.labelId,
+        'aria-labelledby':
+          rest && rest['aria-label'] ? undefined : `${elementIds.labelId}`,
         // https://developer.mozilla.org/en-US/docs/Web/Security/Securing_your_site/Turning_off_form_autocompletion
         // revert back since autocomplete="nope" is ignored on latest Chrome and Opera
         autoComplete: 'off',
diff --git a/src/hooks/useSelect/README.md b/src/hooks/useSelect/README.md
index c288a3229..51d8dc487 100644
--- a/src/hooks/useSelect/README.md
+++ b/src/hooks/useSelect/README.md
@@ -753,6 +753,12 @@ Optional properties:
   primitive component like `<div>`, there is no need to specify this property.
   It defaults to `ref`.
 
+- `aria-label`: By default the toggle element will add an `aria-labelledby` that
+  refers to the `<label>` rendered with `getLabelProps`. However, if you provide
+  `aria-label` to give a more specific label that describes the options
+  available, then `aria-labelledby` will not be provided and screen readers can
+  use your `aria-label` instead.
+
 In some cases, you might want to completely bypass the `refKey` check. Then you
 can provide the object `{suppressRefError : true}` as the second argument to
 `getToggleButtonProps`. **Please use it with extreme care and only if you are
diff --git a/src/hooks/useSelect/__tests__/getMenuProps.test.js b/src/hooks/useSelect/__tests__/getMenuProps.test.js
index e23844c7d..d6112578b 100644
--- a/src/hooks/useSelect/__tests__/getMenuProps.test.js
+++ b/src/hooks/useSelect/__tests__/getMenuProps.test.js
@@ -53,11 +53,13 @@ describe('getMenuProps', () => {
       expect(menuProps.role).toEqual('listbox')
     })
 
-    test("assign '-1' to tabindex", () => {
+    test("do not assign 'aria-labelledby' if it has aria-label", () => {
+      const ariaLabel = 'not so fast'
       const {result} = renderUseSelect()
-      const menuProps = result.current.getMenuProps()
+      const menuProps = result.current.getMenuProps({'aria-label': ariaLabel})
 
-      expect(menuProps.tabIndex).toEqual(-1)
+      expect(menuProps['aria-labelledby']).toBeUndefined()
+      expect(menuProps['aria-label']).toBe(ariaLabel)
     })
   })
 
diff --git a/src/hooks/useSelect/__tests__/getToggleButtonProps.test.js b/src/hooks/useSelect/__tests__/getToggleButtonProps.test.js
index 76b038445..fb3f49be4 100644
--- a/src/hooks/useSelect/__tests__/getToggleButtonProps.test.js
+++ b/src/hooks/useSelect/__tests__/getToggleButtonProps.test.js
@@ -127,6 +127,15 @@ describe('getToggleButtonProps', () => {
 
       expect(toggleButtonProps['aria-activedescendant']).toEqual('')
     })
+
+    test("do not assign 'aria-labelledby' if it has aria-label", () => {
+      const ariaLabel = 'not so fast'
+      const {result} = renderUseSelect()
+      const menuProps = result.current.getToggleButtonProps({'aria-label': ariaLabel})
+
+      expect(menuProps['aria-labelledby']).toBeUndefined()
+      expect(menuProps['aria-label']).toBe(ariaLabel)
+    })
   })
 
   describe('user props', () => {
diff --git a/src/hooks/useSelect/index.js b/src/hooks/useSelect/index.js
index 1abe269b1..d9eae77be 100644
--- a/src/hooks/useSelect/index.js
+++ b/src/hooks/useSelect/index.js
@@ -345,8 +345,8 @@ function useSelect(userProps = {}) {
         }),
         id: elementIds.menuId,
         role: 'listbox',
-        'aria-labelledby': elementIds.labelId,
-        tabIndex: -1,
+        'aria-labelledby':
+          rest && rest['aria-label'] ? undefined : `${elementIds.labelId}`,
         onMouseLeave: callAllEventHandlers(onMouseLeave, menuHandleMouseLeave),
         ...rest,
       }
@@ -397,7 +397,8 @@ function useSelect(userProps = {}) {
         'aria-controls': elementIds.menuId,
         'aria-expanded': latest.current.state.isOpen,
         'aria-haspopup': 'listbox',
-        'aria-labelledby': `${elementIds.labelId}`,
+        'aria-labelledby':
+          rest && rest['aria-label'] ? undefined : `${elementIds.labelId}`,
         id: elementIds.toggleButtonId,
         role: 'combobox',
         tabIndex: 0,