Skip to content
Open
5 changes: 5 additions & 0 deletions .changeset/giant-oranges-fix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@rocket.chat/fuselage': patch
---

fix(fuselage): improve a11y for `MultiSelect` components
10 changes: 5 additions & 5 deletions packages/fuselage-forms/src/__snapshots__/test.spec.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -215,9 +215,6 @@ exports[`renders WithMultiSelect without crashing 1`] = `
class="rcx-box rcx-box--full rcx-field__row"
>
<div
aria-describedby="react-aria-:rb:-description react-aria-:rb:-error react-aria-:rb:-hint"
aria-invalid="true"
aria-labelledby="react-aria-:rb:-label"
class="rcx-box rcx-box--full rcx-select"
>
<div
Expand All @@ -226,12 +223,15 @@ exports[`renders WithMultiSelect without crashing 1`] = `
<div
class="rcx-box rcx-box--full rcx-css-w398ts"
>
<button
<div
aria-describedby="react-aria-:rb:-description react-aria-:rb:-error react-aria-:rb:-hint"
aria-expanded="false"
aria-haspopup="listbox"
aria-invalid="true"
aria-labelledby="react-aria-:rb:-label"
class="rcx-box rcx-box--full rcx-input-box--undecorated rcx-select__focus rcx-css-trljwa rcx-css-3fq6z9"
type="button"
role="combobox"
tabindex="0"
/>
<button
class="rcx-box rcx-chip rcx-css-trljwa"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { composeStories } from '@storybook/react-webpack5';
import { axe } from 'jest-axe';
import { withResizeObserverMock } from 'testing-utils/mocks/withResizeObserverMock';

import { render } from '../../testing';
Expand All @@ -11,6 +12,14 @@ withResizeObserverMock();

describe('[MultiSelect Component]', () => {
it('renders without crashing', () => {
render(<Default />);
const tree = render(<Default />);
expect(tree.baseElement).toMatchSnapshot();
});

it('%s should have no a11y violations', async () => {
const { container } = render(<Default />);

const results = await axe(container);
expect(results).toHaveNoViolations();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const options: SelectOption[] = [
];

const Template: StoryFn<typeof MultiSelect> = (args) => (
<MultiSelect {...args} />
<MultiSelect aria-label='MultiSelect' {...args} />
);

export const Default: StoryFn<typeof MultiSelect> = Template.bind({});
Expand Down
28 changes: 25 additions & 3 deletions packages/fuselage/src/components/MultiSelect/MultiSelect.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@
const [cursor, handleKeyDown, handleKeyUp, reset, [visible, hide, show]] =
useCursor(index, filteredOptions, internalChanged);

useEffect(reset, [filter]);

Check warning on line 117 in packages/fuselage/src/components/MultiSelect/MultiSelect.tsx

View workflow job for this annotation

GitHub Actions / Build and Test

React Hook useEffect has a missing dependency: 'reset'. Either include it or remove the dependency array

const innerRef = useRef<HTMLElement>(null);
const anchorRef = useMergedRefs(ref, innerRef);
Expand Down Expand Up @@ -144,6 +144,19 @@
return show();
});

const listboxId = props.id ? `${props.id}-listbox` : undefined;

const {
id,
name,
'aria-label': ariaLabel,
'aria-labelledby': ariaLabelledBy,
'aria-describedby': ariaDescribedBy,
'aria-invalid': ariaInvalid,
'aria-required': ariaRequired,
...containerProps
} = props;

return (
<Box
is='div'
Expand All @@ -152,7 +165,7 @@
ref={containerRef}
onClick={handleClick}
disabled={disabled}
{...props}
{...containerProps}
>
<FlexItem grow={1}>
<Margins inline='x4'>
Expand All @@ -175,8 +188,17 @@
'onBlur': hide,
'onKeyDown': handleKeyDown,
'onKeyUp': handleKeyUp,
'role': 'combobox',
'aria-expanded': visible === AnimatedVisibility.VISIBLE,
'aria-labelledby': props['aria-labelledby'],
'aria-haspopup': 'listbox',
'aria-controls': listboxId,
id,
name,
'aria-label': ariaLabel,
'aria-labelledby': ariaLabelledBy,
'aria-describedby': ariaDescribedBy,
'aria-invalid': ariaInvalid,
'aria-required': ariaRequired,
})}
{internalValue.map((value: SelectOption[0]) => {
const currentOption = options.find(
Expand Down Expand Up @@ -237,7 +259,7 @@
multiple
filter={filter}
renderItem={renderItem || CheckOption}
role='listbox'
id={listboxId}
options={filteredOptions}
onSelect={internalChanged}
cursor={cursor}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type {
AriaAttributes,
FocusEventHandler,
KeyboardEventHandler,
MouseEventHandler,
Expand All @@ -15,18 +16,17 @@ type MultiSelectAnchorProps = {
onBlur: FocusEventHandler;
onKeyUp: KeyboardEventHandler;
onKeyDown: KeyboardEventHandler;
};
role?: string;
id?: string;
name?: string;
} & AriaAttributes;

const MultiSelectAnchor = forwardRef<Element, MultiSelectAnchorProps>(
function MultiSelectAnchor(props, ref) {
function MultiSelectAnchor({ children, ...props }, ref) {
return (
<SelectFocus
rcx-input-box--undecorated
ref={ref}
aria-haspopup='listbox'
order={1}
{...props}
/>
<SelectFocus rcx-input-box--undecorated ref={ref} order={1} {...props}>
{children}
</SelectFocus>
);
},
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,7 @@ export type MultiSelectAnchorParams = {
onBlur: FocusEventHandler;
onKeyUp: KeyboardEventHandler;
onKeyDown: KeyboardEventHandler;
role?: string;
id?: string;
name?: string;
} & AriaAttributes;
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type {
AriaAttributes,
FocusEventHandler,
FormEvent,
KeyboardEventHandler,
Expand All @@ -20,7 +21,10 @@ type MultiSelectFilteredAnchorProps = {
onBlur: FocusEventHandler;
onKeyUp: KeyboardEventHandler;
onKeyDown: KeyboardEventHandler;
};
role?: string;
id?: string;
name?: string;
} & AriaAttributes;

const MultiSelectFilteredAnchor = forwardRef<
HTMLInputElement,
Expand All @@ -40,7 +44,6 @@ const MultiSelectFilteredAnchor = forwardRef<
}
{...props}
rcx-input-box--undecorated
aria-haspopup='listbox'
order={1}
/>
</FlexItem>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing

exports[`[MultiSelect Component] renders without crashing 1`] = `
<body>
<div>
<div
class="rcx-box rcx-box--full rcx-select"
>
<div
class="rcx-box rcx-box--full rcx-css-1sr8su7"
>
<div
class="rcx-box rcx-box--full rcx-css-w398ts"
>
<div
aria-expanded="false"
aria-haspopup="listbox"
aria-label="MultiSelect"
class="rcx-box rcx-box--full rcx-input-box--undecorated rcx-select__focus rcx-css-trljwa rcx-css-3fq6z9"
role="combobox"
tabindex="0"
>
Placeholder here...
</div>
</div>
</div>
<div
class="rcx-box rcx-box--full rcx-select__addon rcx-css-x7bl3q rcx-css-1bprq55"
>
<i
aria-hidden="true"
class="rcx-box rcx-box--full rcx-icon--name-chevron-down rcx-icon rcx-css-4pvxx3"
>
</i>
</div>
</div>
</div>
</body>
`;
2 changes: 2 additions & 0 deletions packages/fuselage/src/components/Options/Options.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ const Options = forwardRef<HTMLElement, OptionsProps>(function Options(
renderItem: OptionComponent = Option,
onSelect,
customEmpty,
id,
...props
},
ref,
Expand Down Expand Up @@ -122,6 +123,7 @@ const Options = forwardRef<HTMLElement, OptionsProps>(function Options(
is='ol'
aria-multiselectable={multiple || true}
role='listbox'
id={id}
aria-activedescendant={
options?.[cursor]?.[0]
? String(options?.[cursor]?.[0])
Expand Down
4 changes: 2 additions & 2 deletions packages/fuselage/src/components/Select/SelectFocus.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ const SelectFocus = forwardRef<Element, SelectFocusProps>(
fontScale='p2m'
color='hint'
rcx-select__focus
is='button'
type='button'
is='div'
tabIndex={0}
{...props}
/>
);
Expand Down
Loading