Correct inactive ActionList.Item behavior in NavList and SelectPanel contexts

.changeset/floppy-peaches-obey.md

@@ -0,0 +1,18 @@
+---
+'@primer/react': patch
+---
+
+- Instead of relying on parent component checks (e.g. “is this `ActionList` wrapped in an `ActionMenu` or `SelectPanel`?”) to decide between a tooltip or inline inactive message, we now check the `role` on `ActionList`.
+  - If `role` is `"menu"` or `"listbox"`, the inactive message is rendered inline (e.g. in `ActionMenu` or `SelectPanel`)
+  - Otherwise, the message is shown in a tooltip
+- This `role`-based logic fixes inactive NavList item behavior:
+  - Now shows the message in a tooltip instead of inline
+- This `role`-based logic fixes inactive SelectPanel item behavior:
+  - Now shows the message inline instead of in a tooltip
+  - **Important note:** Inactive text only works in `SelectPanel` items when using the modern `ActionList`. The deprecated `ActionList` does not support inactive items.
+- Uses the same `role`-based logic to determine whether `ActionList.TrailingAction` is allowed inside an item
+  - Previously, we relied on parent component checks to block nesting interactive elements
+  - This change is unrelated to inactive states, but it's a more robust way to prevent interactive conflicts
+- Updates `aria-describedby` and `aria-labelledby` associations for tooltip buttons on inactive items:
+  - **Before**: `aria-describedby` → item label ("Item 1"), `aria-labelledby` → inactive message ("Unavailable due to an outage")
+  - **After**: `aria-labelledby` → item label, `aria-describedby` → inactive message

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

@@ -100,7 +100,8 @@ describe('ActionList', () => {
     expect(document.activeElement).toHaveTextContent('Option 4')
 
     await userEvent.keyboard('{ArrowDown}')
-    expect(document.activeElement).toHaveAccessibleName('Unavailable due to an outage')
+    expect(document.activeElement).toHaveAccessibleName('Option 5')
+    expect(document.activeElement).toHaveAccessibleDescription('Unavailable due to an outage')
 
     await userEvent.keyboard('{ArrowUp}')
     expect(document.activeElement).toHaveTextContent('Option 4')

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

@@ -17,6 +17,10 @@ function SimpleActionList(): JSX.Element {
       <ActionList.LinkItem href="//github.com" title="anchor" aria-keyshortcuts="d">
         Link Item
       </ActionList.LinkItem>
+      <ActionList.Item inactiveText="Unavailable due to an outage">Inactive item</ActionList.Item>
+      <ActionList.Item inactiveText="Unavailable due to an outage" loading>
+        Loading and inactive item
+      </ActionList.Item>
     </ActionList>
   )
 }
@@ -171,23 +175,53 @@ describe('ActionList.Item', () => {
     fireEvent.keyPress(option, {key: 'Enter', charCode: 13})
     expect(option).toBeInTheDocument()
   })
-  it('should focus the button around the leading visual when tabbing to an inactive item', async () => {
+  it('should focus the button around the alert icon when tabbing to an inactive item', async () => {
+    const component = HTMLRender(<SimpleActionList />)
+    const inactiveIndicatorButton = await waitFor(() => component.getByRole('button', {name: 'Inactive item'}))
+    await userEvent.tab()
+    await userEvent.tab()
+    await userEvent.tab()
+    await userEvent.tab()
+    await userEvent.tab()
+    await userEvent.tab() // focuses 6th element, which is inactive
+    expect(inactiveIndicatorButton).toHaveFocus()
+    expect(document.activeElement).toHaveAccessibleDescription('Unavailable due to an outage')
+  })
+  it('should focus the option or menu item when moving focus to an inactive item **in a listbox**', async () => {
     const component = HTMLRender(<SingleSelectListStory />)
-    const inactiveOptionButton = await waitFor(() => component.getByRole('button', {name: projects[3].inactiveText}))
+    const inactiveOption = await waitFor(() => component.getByRole('option', {name: projects[3].name}))
     await userEvent.tab() // get focus on first element
     await userEvent.keyboard('{ArrowDown}')
     await userEvent.keyboard('{ArrowDown}')
-    expect(inactiveOptionButton).toHaveFocus()
+    expect(inactiveOption).toHaveFocus()
+    expect(document.activeElement).toHaveAccessibleDescription(projects[3].inactiveText as string)
   })
   it('should behave as inactive if both inactiveText and loading props are passed', async () => {
+    const component = HTMLRender(<SimpleActionList />)
+    const inactiveIndicatorButton = await waitFor(() =>
+      component.getByRole('button', {name: 'Loading and inactive item'}),
+    )
+    await userEvent.tab()
+    await userEvent.tab()
+    await userEvent.tab()
+    await userEvent.tab()
+    await userEvent.tab()
+    await userEvent.tab()
+    await userEvent.tab() // focuses 7th element, which is inactive AND has a loading prop
+    expect(inactiveIndicatorButton).toHaveFocus()
+    expect(document.activeElement).toHaveAccessibleDescription('Unavailable due to an outage')
+  })
+
+  it('should behave as inactive if both inactiveText and loading props are passed **in a listbox**', async () => {
     const component = HTMLRender(<SingleSelectListStory />)
-    const inactiveOptionButton = await waitFor(() => component.getByRole('button', {name: projects[5].inactiveText}))
+    const inactiveOption = await waitFor(() => component.getByRole('option', {name: projects[5].name}))
     await userEvent.tab() // get focus on first element
     await userEvent.keyboard('{ArrowDown}')
     await userEvent.keyboard('{ArrowDown}')
     await userEvent.keyboard('{ArrowDown}')
     await userEvent.keyboard('{ArrowDown}')
-    expect(inactiveOptionButton).toHaveFocus()
+    expect(inactiveOption).toHaveFocus()
+    expect(document.activeElement).toHaveAccessibleDescription(projects[5].inactiveText as string)
   })
   it('should call onClick for a link item', async () => {
     const onClick = jest.fn()

packages/react/src/ActionList/Item.tsx

@@ -112,7 +112,8 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
     } = React.useContext(ListContext)
     const {selectionVariant: groupSelectionVariant} = React.useContext(GroupContext)
     const inactive = Boolean(inactiveText)
-    const showInactiveIndicator = inactive && container === undefined
+    const menuContext = listRole !== undefined && ['menu', 'listbox'].includes(listRole)
+    const showInactiveIndicator = inactive && !menuContext
 
     const onSelect = React.useCallback(
       (
@@ -142,7 +143,6 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
     }
 
     const itemRole = role || inferredItemRole
-    const menuContext = container === 'ActionMenu' || container === 'SelectPanel'
 
     if (slots.trailingAction) {
       invariant(!menuContext, `ActionList.TrailingAction can not be used within a ${container}.`)
@@ -405,7 +405,8 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
                     <span id={labelId} className={classes.ItemLabel}>
                       {childrenWithoutSlots}
                       {/* Loading message needs to be in here so it is read with the label */}
-                      {loading === true && <VisuallyHidden>Loading</VisuallyHidden>}
+                      {/* If the item is inactive, we do not simultaneously announce that it is loading */}
+                      {loading === true && !inactive && <VisuallyHidden>Loading</VisuallyHidden>}
                     </span>
                     {slots.description}
                   </ConditionalWrapper>
@@ -422,7 +423,7 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
                   {
                     // If the item is inactive, but it's not in an overlay (e.g. ActionMenu, SelectPanel),
                     // render the inactive warning message directly in the item.
-                    inactive && container ? (
+                    !showInactiveIndicator ? (
                       <span className={classes.InactiveWarning} id={inactiveWarningId}>
                         {inactiveText}
                       </span>
@@ -477,7 +478,8 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
                   <span id={labelId} className={classes.ItemLabel}>
                     {childrenWithoutSlots}
                     {/* Loading message needs to be in here so it is read with the label */}
-                    {loading === true && <VisuallyHidden>Loading</VisuallyHidden>}
+                    {/* If the item is inactive, we do not simultaneously announce that it is loading */}
+                    {loading === true && !inactive && <VisuallyHidden>Loading</VisuallyHidden>}
                   </span>
                   {slots.description}
                 </ConditionalWrapper>
@@ -494,7 +496,7 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
                 {
                   // If the item is inactive, but it's not in an overlay (e.g. ActionMenu, SelectPanel),
                   // render the inactive warning message directly in the item.
-                  inactive && container ? (
+                  !showInactiveIndicator ? (
                     <span className={classes.InactiveWarning} id={inactiveWarningId}>
                       {inactiveText}
                     </span>
@@ -567,7 +569,8 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
                   >
                     {childrenWithoutSlots}
                     {/* Loading message needs to be in here so it is read with the label */}
-                    {loading === true && <VisuallyHidden>Loading</VisuallyHidden>}
+                    {/* If the item is inactive, we do not simultaneously announce that it is loading */}
+                    {loading === true && !inactive && <VisuallyHidden>Loading</VisuallyHidden>}
                   </Box>
                   {slots.inlineDescription}
                 </ConditionalWrapper>
@@ -584,7 +587,7 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
               {
                 // If the item is inactive, but it's not in an overlay (e.g. ActionMenu, SelectPanel),
                 // render the inactive warning message directly in the item.
-                inactive && container ? (
+                !showInactiveIndicator ? (
                   <Box
                     as="span"
                     sx={{

packages/react/src/ActionList/Visuals.tsx

@@ -168,8 +168,8 @@ export const VisualOrIndicator: React.FC<
 
   return inactiveText ? (
     <span className={classes.InactiveButtonWrap}>
-      <Tooltip text={inactiveText} type="label">
-        <button type="button" className={classes.InactiveButtonReset} aria-describedby={labelId}>
+      <Tooltip text={inactiveText} type="description">
+        <button type="button" className={classes.InactiveButtonReset} aria-labelledby={labelId}>
           <VisualComponent>
             <AlertIcon />
           </VisualComponent>