Skip to content

Commit

Permalink
[86by0byqr][select] fixed multiselect a11y (#1282)
Browse files Browse the repository at this point in the history
<!--- Provide a general summary of your changes in the Title above -->

## Motivation and Context

Multiselect wasn't annotated properly if it used with `options` prop
because default options rendered was adding aria-selected without taking
into account that `value` may be an array of selected options.

Also I've fixed a11y linting – select.option.checkbox should not have
any roles because it's just a visual representation of multiselect
option.

## How has this been tested?

Manually and with unit test.

## Types of changes

- [x] Bug fix (non-breaking change which fixes an issue).
- [ ] New feature (non-breaking change which adds functionality).
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected).
- [ ] Nice improve.

## Checklist:

<!--- Go over all the following points, and put an `x` in all the boxes
that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->

- [x] My code follows the code style of this project.
- [x] I have updated the documentation accordingly or it's not required.
- [x] Unit tests are not broken.
- [x] I have added changelog note to corresponding `CHANGELOG.md` file
with planned publish date.
- [x] I have added new unit tests on added of fixed functionality.
  • Loading branch information
msereniti authored Apr 12, 2024
1 parent dddc3ba commit 0a2220c
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 8 deletions.
6 changes: 6 additions & 0 deletions semcore/select/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@

CHANGELOG.md standards are inspired by [keepachangelog.com](https://keepachangelog.com/en/1.0.0/).

## [4.37.0] - 2024-04-10

### Fixed

- Multiselect interactions were not annotated properly by screen readers when select was used with `options` prop.

## [4.36.2] - 2024-04-10

### Fixed
Expand Down
25 changes: 25 additions & 0 deletions semcore/select/__tests__/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,31 @@ describe('Select Trigger', () => {
const results = await axe(container);
expect(results).toHaveNoViolations();
});
test('multiselect a11y', async () => {
vi.useFakeTimers();
const options = [
{
value: '1',
children: 'Option 1',
label: 'Option 1',
},
{
value: '2',
children: 'Option 2',
label: 'Option 2',
},
];
const { container } = render(
<Select visible value={['2']} disablePortal multiselect options={options} />,
);
act(() => {
vi.runAllTimers();
});
vi.useRealTimers();

const results = await axe(container);
expect(results).toHaveNoViolations();
});

test.concurrent('focus position preserve with mouse navigation', async () => {
vi.useFakeTimers();
Expand Down
9 changes: 1 addition & 8 deletions semcore/select/src/Select.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -246,12 +246,7 @@ class RootSelect extends Component {
<Select.Menu>
{options.map((option, index) => {
return (
<Select.Option
key={option.value}
id={`igc-${uid}-option-${index}`}
aria-selected={value === option.value}
{...option}
>
<Select.Option key={option.value} id={`igc-${uid}-option-${index}`} {...option}>
{multiselect && <Select.Option.Checkbox />}
{option.children}
</Select.Option>
Expand Down Expand Up @@ -351,9 +346,7 @@ function Checkbox(providedProps) {
{...componentProps}
className={cn(className, componentProps.className) || undefined}
style={{ ...style, ...componentProps.style }}
role='checkbox'
tabIndex={-1}
aria-checked={selected}
/>
);
}
Expand Down

0 comments on commit 0a2220c

Please sign in to comment.