Skip to content

Commit

Permalink
Remove value prefix on Select combobox accessible name (#4329)
Browse files Browse the repository at this point in the history
* Remove value prefix on Select combobox accessible name

* Add test for accessible name with no value selected

* Adjust to use everything after first space, allowing another label reference
  • Loading branch information
dougmacknz authored Nov 21, 2023
1 parent 41ff866 commit 3b77a41
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 13 deletions.
5 changes: 5 additions & 0 deletions .changeset/long-wombats-provide.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@kaizen/components": patch
---

Remove value prefix on Select combobox accessible name
43 changes: 31 additions & 12 deletions packages/components/src/__future__/Select/Select.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,35 @@ const SelectWrapper = ({

describe("<Select />", () => {
describe("Trigger", () => {
it("makes sure the menu to be labelled by trigger", () => {
it("has the label as the accessible name", () => {
const { getByRole } = render(<SelectWrapper />)
const menu = getByRole("combobox", {
name: "Mock Label",
})
expect(menu).toBeInTheDocument()
})

it("has the value when an item is selected", () => {
const { getByRole } = render(<SelectWrapper selectedKey="batch-brew" />)
const menu = getByRole("combobox", {
name: "Batch brew Mock Label",
name: "Mock Label",
})
expect(menu).toHaveTextContent("Batch brew")
})

it("allows more aria-labelledby references to be sent in", () => {
const { getByRole } = render(
<>
<div id="extra-label">extra label stuff</div>
<SelectWrapper aria-labelledby="extra-label" />
</>
)
const menu = getByRole("combobox", {
name: "Mock Label extra label stuff",
})
expect(menu).toBeInTheDocument()
})

describe("when uncontrolled", () => {
it("does not show the menu initially", () => {
const { queryByRole } = render(<SelectWrapper />)
Expand Down Expand Up @@ -76,7 +97,7 @@ describe("<Select />", () => {
/>
)
const trigger = getByRole("combobox", {
name: "Batch brew Mock Label",
name: "Mock Label",
})
await user.click(trigger)
await waitFor(() => {
Expand All @@ -93,7 +114,7 @@ describe("<Select />", () => {
<SelectWrapper selectedKey="batch-brew" />
)
const trigger = getByRole("combobox", {
name: "Batch brew Mock Label",
name: "Mock Label",
})
await user.click(trigger)
await waitFor(() => {
Expand All @@ -108,7 +129,7 @@ describe("<Select />", () => {
<SelectWrapper selectedKey="batch-brew" defaultOpen />
)
const trigger = getByRole("combobox", {
name: "Batch brew Mock Label",
name: "Mock Label",
})

await user.click(trigger)
Expand Down Expand Up @@ -147,7 +168,7 @@ describe("<Select />", () => {
<SelectWrapper selectedKey="batch-brew" />
)
const trigger = getByRole("combobox", {
name: "Batch brew Mock Label",
name: "Mock Label",
})
await user.tab()
await waitFor(() => {
Expand All @@ -160,7 +181,7 @@ describe("<Select />", () => {
<SelectWrapper selectedKey="batch-brew" />
)
const trigger = getByRole("combobox", {
name: "Batch brew Mock Label",
name: "Mock Label",
})
await user.tab()
await waitFor(() => {
Expand Down Expand Up @@ -305,9 +326,7 @@ describe("<Select />", () => {
})
await user.keyboard("{Enter}")

await user.click(
getByRole("combobox", { name: "Short black Mock Label" })
)
await user.click(getByRole("combobox", { name: "Mock Label" }))
await waitFor(() => {
expect(
getByRole("option", { name: "Short black", selected: true })
Expand All @@ -320,7 +339,7 @@ describe("<Select />", () => {
const { getByRole } = render(
<SelectWrapper onSelectionChange={spy} defaultOpen />
)
const trigger = getByRole("combobox", { name: "Select Mock Label" })
const trigger = getByRole("combobox", { name: "Mock Label" })

await user.tab()
await waitFor(() => {
Expand All @@ -332,7 +351,7 @@ describe("<Select />", () => {
await user.keyboard("{Enter}")
await waitFor(() => {
expect(spy).toHaveBeenCalledTimes(1)
expect(trigger).toHaveAccessibleName("Short black Mock Label")
expect(trigger).toHaveAccessibleName("Mock Label")
})
})
})
Expand Down
15 changes: 14 additions & 1 deletion packages/components/src/__future__/Select/Select.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -116,13 +116,26 @@ export const Select = <Option extends SelectOption = SelectOption>({

const {
labelProps,
triggerProps,
triggerProps: reactAriaTriggerProps,
valueProps,
menuProps,
errorMessageProps,
descriptionProps,
} = useSelect(ariaSelectProps, state, triggerRef)

// Hack incoming:
// react-aria/useSelect wants to prefix the combobox's accessible name with the value of the select.
// We use role=combobox, meaning screen readers will read the value.
// So we're modifying the `aria-labelledby` property to remove the value element id.
// Issue: https://github.com/adobe/react-spectrum/issues/4091
const reactAriaLabelledBy = reactAriaTriggerProps["aria-labelledby"]
const triggerProps = {
...reactAriaTriggerProps,
"aria-labelledby": reactAriaLabelledBy?.substring(
reactAriaLabelledBy.indexOf(" ") + 1
),
}

const { buttonProps } = useButton(triggerProps, triggerRef)
const selectToggleProps = {
...buttonProps,
Expand Down

0 comments on commit 3b77a41

Please sign in to comment.