ActionList: Add role="option" if role="listbox" is present (#6049)

Author: TylerJDev

.changeset/soft-webs-study.md

@@ -0,0 +1,5 @@
+---
+'@primer/react': patch
+---
+
+ActionList: Ensure `role="option"` is added when `role="listbox"` is used; allow disabled items to remain focusable

packages/react/src/ActionList/ActionList.features.stories.tsx

@@ -329,7 +329,35 @@ export const MultiSelect = () => {
   )
 }
 
-export const ListBoxMultiSelect = () => {
+export const ListboxSingleSelect = () => {
+  const [selectedIndice, setSelectedIndice] = React.useState<number>(0)
+  const handleSelect = (index: number) => {
+    setSelectedIndice(index)
+  }
+
+  return (
+    <ActionList selectionVariant="single" role="listbox" aria-label="Projects">
+      {projects.map((project, index) => (
+        <ActionList.Item
+          key={index}
+          selected={selectedIndice === index}
+          aria-checked={selectedIndice === index}
+          onSelect={() => handleSelect(index)}
+          disabled={index === 3 ? true : undefined}
+          role="option"
+        >
+          <ActionList.LeadingVisual>
+            <TableIcon />
+          </ActionList.LeadingVisual>
+          {project.name}
+          <ActionList.Description variant="block">{project.scope}</ActionList.Description>
+        </ActionList.Item>
+      ))}
+    </ActionList>
+  )
+}
+
+export const ListboxMultiSelect = () => {
   const [selectedIndices, setSelectedIndices] = React.useState<number[]>([0])
   const handleSelect = (index: number) => {
     if (selectedIndices.includes(index)) {
@@ -339,7 +367,7 @@ export const ListBoxMultiSelect = () => {
     }
   }
   return (
-    <ActionList role="menu" selectionVariant="multiple" aria-label="Project">
+    <ActionList role="menu" selectionVariant="multiple" aria-label="Projects">
       {projects.map((project, index) => (
         <ActionList.Item
           key={index}

packages/react/src/ActionList/ActionList.test.tsx

@@ -94,7 +94,9 @@ describe('ActionList', () => {
     expect(document.activeElement).toHaveTextContent('Option 2')
 
     await userEvent.keyboard('{ArrowDown}')
-    expect(document.activeElement).not.toHaveTextContent('Option 3') // option 3 is disabled
+    expect(document.activeElement).toHaveTextContent('Option 3')
+
+    await userEvent.keyboard('{ArrowDown}')
     expect(document.activeElement).toHaveTextContent('Option 4')
 
     await userEvent.keyboard('{ArrowDown}')

packages/react/src/ActionList/Item.test.tsx

@@ -47,7 +47,6 @@ function SingleSelectListStory(): JSX.Element {
       {projects.map((project, index) => (
         <ActionList.Item
           key={index}
-          role="option"
           selected={index === selectedIndex}
           onSelect={() => setSelectedIndex(index)}
           disabled={project.disabled}
@@ -193,6 +192,7 @@ describe('ActionList.Item', () => {
     await userEvent.tab() // get focus on first element
     await userEvent.keyboard('{ArrowDown}')
     await userEvent.keyboard('{ArrowDown}')
+    await userEvent.keyboard('{ArrowDown}')
     expect(inactiveOption).toHaveFocus()
     expect(document.activeElement).toHaveAccessibleDescription(projects[3].inactiveText as string)
   })
@@ -220,6 +220,7 @@ describe('ActionList.Item', () => {
     await userEvent.keyboard('{ArrowDown}')
     await userEvent.keyboard('{ArrowDown}')
     await userEvent.keyboard('{ArrowDown}')
+    await userEvent.keyboard('{ArrowDown}')
     expect(inactiveOption).toHaveFocus()
     expect(document.activeElement).toHaveAccessibleDescription(projects[5].inactiveText as string)
   })
@@ -420,4 +421,18 @@ describe('ActionList.Item', () => {
     expect(button.parentElement?.tagName).toBe('LI')
     expect(button.textContent).toBe('Item 5')
   })
+
+  it('should add `role="option"` if `role="listbox"` and `selectionVariant` is present', async () => {
+    const {getAllByRole} = HTMLRender(
+      <ActionList role="listbox" selectionVariant="single">
+        <ActionList.Item>Item 1</ActionList.Item>
+        <ActionList.Item>Item 2</ActionList.Item>
+        <ActionList.Item>Item 3</ActionList.Item>
+        <ActionList.Item>Item 4</ActionList.Item>
+      </ActionList>,
+    )
+    const options = getAllByRole('option')
+    expect(options[0]).toBeInTheDocument()
+    expect(options).toHaveLength(4)
+  })
 })

packages/react/src/ActionList/Item.tsx

@@ -141,8 +141,8 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
       if (selectionVariant === 'single') inferredItemRole = 'menuitemradio'
       else if (selectionVariant === 'multiple') inferredItemRole = 'menuitemcheckbox'
       else inferredItemRole = 'menuitem'
-    } else if (container === 'SelectPanel' && listRole === 'listbox') {
-      if (selectionVariant !== undefined) inferredItemRole = 'option'
+    } else if (listRole === 'listbox') {
+      if (selectionVariant !== undefined && !role) inferredItemRole = 'option'
     }
 
     const itemRole = role || inferredItemRole
@@ -325,8 +325,7 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
 
     let focusable
 
-    // if item is disabled and is of type (menuitem*, option) it should remain focusable, if inactive, apply the same rules
-    if ((disabled && !inferredItemRole) || showInactiveIndicator) {
+    if (showInactiveIndicator) {
       focusable = true
     }