Skip to content

Commit

Permalink
feat: getter prop with aria-label will not add aria-labelledby (#1488)
Browse files Browse the repository at this point in the history
  • Loading branch information
silviuaavram authored Mar 20, 2023
1 parent a4c3714 commit 20314b5
Show file tree
Hide file tree
Showing 11 changed files with 71 additions and 12 deletions.
6 changes: 6 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 12 additions & 3 deletions src/__tests__/downshift.aria.js
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand All @@ -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,
Expand All @@ -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})}>
Expand Down
2 changes: 1 addition & 1 deletion src/downshift.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
6 changes: 6 additions & 0 deletions src/hooks/useCombobox/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions src/hooks/useCombobox/__tests__/getInputProps.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
9 changes: 9 additions & 0 deletions src/hooks/useCombobox/__tests__/getMenuProps.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
6 changes: 4 additions & 2 deletions src/hooks/useCombobox/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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',
Expand Down
6 changes: 6 additions & 0 deletions src/hooks/useSelect/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 5 additions & 3 deletions src/hooks/useSelect/__tests__/getMenuProps.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
})

Expand Down
9 changes: 9 additions & 0 deletions src/hooks/useSelect/__tests__/getToggleButtonProps.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
7 changes: 4 additions & 3 deletions src/hooks/useSelect/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 20314b5

Please # to comment.