Skip to content

Commit

Permalink
fix(ActionList): place id on item with role (#3691)
Browse files Browse the repository at this point in the history 
* fix(ActionList): place `id` on item with role

* chore: add changeset

* test: update snapshots and tests with new id

---------

Co-authored-by: Josh Black <joshblack@users.noreply.github.com>
  • Loading branch information
@joshblack
joshblack and joshblack authored Oct 10, 2023
1 parent 115e09c commit f4648b1
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 25 deletions.
7 changes: 7 additions & 0 deletions .changeset/giant-bobcats-deliver.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@primer/react': patch
---

Update ActionList to place `id` on item with an ARIA role

<!-- Changed components: ActionList -->
9 changes: 5 additions & 4 deletions src/ActionList/Item.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,10 +190,10 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
[onSelect, disabled, afterSelect],
)

// use props.id if provided, otherwise generate one.
const labelId = useId(id)
const inlineDescriptionId = useId(id && `${id}--inline-description`)
const blockDescriptionId = useId(id && `${id}--block-description`)
const itemId = useId(id)
const labelId = `${itemId}--label`
const inlineDescriptionId = `${itemId}--inline-description`
const blockDescriptionId = `${itemId}--block-description`

const ItemWrapper = _PrivateItemWrapper || React.Fragment

Expand All @@ -208,6 +208,7 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
'aria-describedby': slots.description?.props.variant === 'block' ? blockDescriptionId : undefined,
...(selectionAttribute && {[selectionAttribute]: selected}),
role: role || itemRole,
id: itemId,
}

const containerProps = _PrivateItemWrapper ? {role: role || itemRole ? 'none' : undefined} : menuItemProps
Expand Down
49 changes: 29 additions & 20 deletions src/NavList/__snapshots__/NavList.test.tsx.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -306,9 +306,10 @@ exports[`NavList renders a simple list 1`] = `
>
<a
aria-current="page"
aria-labelledby="react-aria-2 "
aria-labelledby="react-aria-2--label "
class="c3"
href="/"
id="react-aria-2"
tabindex="0"
>
<div
Expand All @@ -317,7 +318,7 @@ exports[`NavList renders a simple list 1`] = `
>
<span
class="c5"
id="react-aria-2"
id="react-aria-2--label"
>
Home
</span>
Expand All @@ -328,9 +329,10 @@ exports[`NavList renders a simple list 1`] = `
class="c1 c6"
>
<a
aria-labelledby="react-aria-5 "
aria-labelledby="react-aria-3--label "
class="c3"
href="/about"
id="react-aria-3"
tabindex="0"
>
<div
Expand All @@ -339,7 +341,7 @@ exports[`NavList renders a simple list 1`] = `
>
<span
class="c5"
id="react-aria-5"
id="react-aria-3--label"
>
About
</span>
Expand All @@ -350,9 +352,10 @@ exports[`NavList renders a simple list 1`] = `
class="c1 c6"
>
<a
aria-labelledby="react-aria-8 "
aria-labelledby="react-aria-4--label "
class="c3"
href="/contact"
id="react-aria-4"
tabindex="0"
>
<div
Expand All @@ -361,7 +364,7 @@ exports[`NavList renders a simple list 1`] = `
>
<span
class="c5"
id="react-aria-8"
id="react-aria-4--label"
>
Contact
</span>
Expand Down Expand Up @@ -736,9 +739,10 @@ exports[`NavList renders with groups 1`] = `
>
<a
aria-current="page"
aria-labelledby="react-aria-3 "
aria-labelledby="react-aria-3--label "
class="c7"
href="/getting-started"
id="react-aria-3"
tabindex="0"
>
<div
Expand All @@ -747,7 +751,7 @@ exports[`NavList renders with groups 1`] = `
>
<span
class="c9"
id="react-aria-3"
id="react-aria-3--label"
>
Getting started
</span>
Expand All @@ -770,22 +774,23 @@ exports[`NavList renders with groups 1`] = `
role="presentation"
>
<span
id="react-aria-6"
id="react-aria-4"
>
Components
</span>
</div>
<ul
aria-labelledby="react-aria-6"
aria-labelledby="react-aria-4"
class="c4"
>
<li
class="c5 c10"
>
<a
aria-labelledby="react-aria-7 "
aria-labelledby="react-aria-5--label "
class="c7"
href="/Avatar"
id="react-aria-5"
tabindex="0"
>
<div
Expand All @@ -794,7 +799,7 @@ exports[`NavList renders with groups 1`] = `
>
<span
class="c9"
id="react-aria-7"
id="react-aria-5--label"
>
Avatar
</span>
Expand Down Expand Up @@ -1200,8 +1205,9 @@ exports[`NavList.Item with NavList.SubNav does not have active styles if SubNav
<button
aria-controls="react-aria-3"
aria-expanded="true"
aria-labelledby="react-aria-2 "
aria-labelledby="react-aria-2--label "
class="c2 c3"
id="react-aria-2"
tabindex="0"
>
<div
Expand All @@ -1213,7 +1219,7 @@ exports[`NavList.Item with NavList.SubNav does not have active styles if SubNav
>
<span
class="c6"
id="react-aria-2"
id="react-aria-2--label"
>
Item
</span>
Expand Down Expand Up @@ -1250,9 +1256,10 @@ exports[`NavList.Item with NavList.SubNav does not have active styles if SubNav
>
<a
aria-current="page"
aria-labelledby="react-aria-4 "
aria-labelledby="react-aria-4--label "
class="c11"
href="#"
id="react-aria-4"
tabindex="0"
>
<div
Expand All @@ -1261,7 +1268,7 @@ exports[`NavList.Item with NavList.SubNav does not have active styles if SubNav
>
<span
class="c6"
id="react-aria-4"
id="react-aria-4--label"
>
Sub Item
</span>
Expand Down Expand Up @@ -1680,8 +1687,9 @@ exports[`NavList.Item with NavList.SubNav has active styles if SubNav contains t
<button
aria-controls="react-aria-3"
aria-expanded="false"
aria-labelledby="react-aria-2 "
aria-labelledby="react-aria-2--label "
class="c2 c3"
id="react-aria-2"
tabindex="0"
>
<div
Expand All @@ -1693,7 +1701,7 @@ exports[`NavList.Item with NavList.SubNav has active styles if SubNav contains t
>
<span
class="c6"
id="react-aria-2"
id="react-aria-2--label"
>
Item
</span>
Expand Down Expand Up @@ -1730,9 +1738,10 @@ exports[`NavList.Item with NavList.SubNav has active styles if SubNav contains t
>
<a
aria-current="page"
aria-labelledby="react-aria-4 "
aria-labelledby="react-aria-4--label "
class="c11"
href="#"
id="react-aria-4"
tabindex="0"
>
<div
Expand All @@ -1741,7 +1750,7 @@ exports[`NavList.Item with NavList.SubNav has active styles if SubNav contains t
>
<span
class="c6"
id="react-aria-4"
id="react-aria-4--label"
>
Sub Item
</span>
Expand Down
2 changes: 1 addition & 1 deletion src/drafts/InlineAutocomplete/InlineAutocomplete.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ describe('InlineAutocomplete', () => {
await user.keyboard('{ArrowDown}')

expect(input).toHaveFocus()
expect(input).toHaveAttribute('aria-activedescendant', expect.stringContaining('option-1'))
expect(input).toHaveAttribute('aria-activedescendant', 'github')
expect(within(getByRole('listbox')).queryAllByRole('option')[1]).toHaveAttribute('aria-selected', 'true')

await user.keyboard('{Enter}')
Expand Down

0 comments on commit f4648b1

Please sign in to comment.