Skip to content

[♿️][Select]: refactor to improve accessibility by adding/removing aria-controls based on list visibility #103

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 26, 2024
Merged

[♿️][Select]: refactor to improve accessibility by adding/removing aria-controls based on list visibility #103

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 55 additions & 2 deletions lib/Select/Select.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,10 +183,10 @@ describe("Select", () => {
});

it("should have the required attributes", () => {
render(
const { rerender } = render(
<Select.Root
open
data-testid="root"
defaultOpen
defaultValue={["0", "1"]}
multiple
searchable
Expand Down Expand Up @@ -260,6 +260,59 @@ describe("Select", () => {
expect(option2).toHaveAttribute("aria-hidden", "false");
expect(option2).toHaveAttribute("aria-disabled", "false");
expect(option2).toHaveAttribute("aria-selected", "true");

rerender(
<Select.Root
open={false}
data-testid="root"
defaultValue={["0", "1"]}
multiple
searchable
label={{ screenReaderLabel: "Label" }}
>
<Select.Trigger data-testid="trigger">
<Select.Controller
autoFocus
data-testid="controller"
/>
</Select.Trigger>
<Select.List data-testid="list">
<Select.EmptyStatement data-testid="empty-statement">
No options found!
</Select.EmptyStatement>
<Select.Option
data-testid="o1"
disabled
value="0"
valueLabel="The Shawshank Redemption"
>
The Shawshank Redemption
</Select.Option>
<Select.Group
data-testid="group"
label={{ screenReaderLabel: "Group" }}
>
<Select.Option
data-testid="o2"
value="1"
valueLabel="The Godfather"
>
The Godfather
</Select.Option>
<Select.Option
data-testid="o3"
value="2"
valueLabel="The Godfather: Part 2"
>
The Godfather: Part 2
</Select.Option>
</Select.Group>
</Select.List>
</Select.Root>,
);

expect(list).not.toBeInTheDocument();
expect(controller).not.toHaveAttribute("aria-controls");
});

it("should properly move the active element by keyboard", async () => {
Expand Down
6 changes: 5 additions & 1 deletion lib/Select/components/Controller/Controller.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,11 @@ const ControllerBase = (props: Props, ref: React.Ref<HTMLInputElement>) => {

if (!listId) return;

node.setAttribute("aria-controls", listId);
if (!ctx.isListOpen && !ctx.keepMounted) {
node.removeAttribute("aria-controls");
} else {
node.setAttribute("aria-controls", listId);
}
};

const controllerProps = {
Expand Down
9 changes: 6 additions & 3 deletions lib/Select/components/List/List.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,12 +104,15 @@ const ListBase = (props: Props, ref: React.Ref<HTMLDivElement>) => {
if (!node) return;

const triggerId = ctx.elementsRegistry.getElementId("trigger");
const comboboxId = ctx.elementsRegistry.getElementId("combobox");

const triggerNode = document.getElementById(triggerId ?? "");
const comboboxId = ctx.elementsRegistry.getElementId("combobox");
const comboboxNode = document.getElementById(comboboxId ?? "");

comboboxNode?.setAttribute("aria-controls", id);
if (ctx.isListOpen || ctx.keepMounted) {
comboboxNode?.setAttribute("aria-controls", id);
} else {
comboboxNode?.removeAttribute("aria-controls");
}

if (!isInitialRenderComplete) return;

Expand Down