Skip to content

Commit

Permalink
fix: select: ensure aria attributes target the intended element (#612)
Browse files Browse the repository at this point in the history
* fix: select: ensure aria attributes target the intended element

* chore: dropdown: add ariaref ut and refine role null check

* chore: dropdown: update role null check to handle all elements
  • Loading branch information
dkilgore-eightfold authored May 4, 2023
1 parent 189c6e8 commit 387158d
Show file tree
Hide file tree
Showing 5 changed files with 124 additions and 65 deletions.
87 changes: 68 additions & 19 deletions src/components/Dropdown/Dropdown.test.tsx
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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() });
Expand Down Expand Up @@ -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}
Expand All @@ -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();
Expand Down Expand Up @@ -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');
});
});
22 changes: 22 additions & 0 deletions src/components/Dropdown/Dropdown.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export const Dropdown: FC<DropdownProps> = React.memo(
React.forwardRef<DropdownRef, DropdownProps>(
(
{
ariaRef,
children,
classNames,
closeOnDropdownClick = true,
Expand Down Expand Up @@ -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.hasAttribute('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),
Expand Down
6 changes: 6 additions & 0 deletions src/components/Dropdown/Dropdown.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down
2 changes: 1 addition & 1 deletion src/components/Select/Select.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand All @@ -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 ||
Expand Down
Loading

0 comments on commit 387158d

Please sign in to comment.