diff --git a/.changeset/floppy-peaches-obey.md b/.changeset/floppy-peaches-obey.md
new file mode 100644
index 00000000000..6cb06e4f665
--- /dev/null
+++ b/.changeset/floppy-peaches-obey.md
@@ -0,0 +1,5 @@
+---
+'@primer/react': patch
+---
+
+Improves how we detect context for inactive item messaging by checking the `role` on `ActionList` instead of relying on parent components, making the logic more robust and consistent. It also fixes incorrect behavior in `NavList` and `SelectPanel`, and improves accessibility by correcting `aria` relationships on tooltip buttons.
diff --git a/.playwright/snapshots/components/ActionList.test.ts-snapshots/ActionList-Inactive-Multiselect-dark-colorblind-linux.png b/.playwright/snapshots/components/ActionList.test.ts-snapshots/ActionList-Inactive-Multiselect-dark-colorblind-linux.png
index e86dd5e7ca6..5e1a05501ce 100644
Binary files a/.playwright/snapshots/components/ActionList.test.ts-snapshots/ActionList-Inactive-Multiselect-dark-colorblind-linux.png and b/.playwright/snapshots/components/ActionList.test.ts-snapshots/ActionList-Inactive-Multiselect-dark-colorblind-linux.png differ
diff --git a/.playwright/snapshots/components/ActionList.test.ts-snapshots/ActionList-Inactive-Multiselect-dark-dimmed-linux.png b/.playwright/snapshots/components/ActionList.test.ts-snapshots/ActionList-Inactive-Multiselect-dark-dimmed-linux.png
index 7a15a9f4aff..ca7371a42ba 100644
Binary files a/.playwright/snapshots/components/ActionList.test.ts-snapshots/ActionList-Inactive-Multiselect-dark-dimmed-linux.png and b/.playwright/snapshots/components/ActionList.test.ts-snapshots/ActionList-Inactive-Multiselect-dark-dimmed-linux.png differ
diff --git a/.playwright/snapshots/components/ActionList.test.ts-snapshots/ActionList-Inactive-Multiselect-dark-high-contrast-linux.png b/.playwright/snapshots/components/ActionList.test.ts-snapshots/ActionList-Inactive-Multiselect-dark-high-contrast-linux.png
index 4229bb95ba1..b3b543be977 100644
Binary files a/.playwright/snapshots/components/ActionList.test.ts-snapshots/ActionList-Inactive-Multiselect-dark-high-contrast-linux.png and b/.playwright/snapshots/components/ActionList.test.ts-snapshots/ActionList-Inactive-Multiselect-dark-high-contrast-linux.png differ
diff --git a/.playwright/snapshots/components/ActionList.test.ts-snapshots/ActionList-Inactive-Multiselect-dark-linux.png b/.playwright/snapshots/components/ActionList.test.ts-snapshots/ActionList-Inactive-Multiselect-dark-linux.png
index e86dd5e7ca6..5e1a05501ce 100644
Binary files a/.playwright/snapshots/components/ActionList.test.ts-snapshots/ActionList-Inactive-Multiselect-dark-linux.png and b/.playwright/snapshots/components/ActionList.test.ts-snapshots/ActionList-Inactive-Multiselect-dark-linux.png differ
diff --git a/.playwright/snapshots/components/ActionList.test.ts-snapshots/ActionList-Inactive-Multiselect-dark-tritanopia-linux.png b/.playwright/snapshots/components/ActionList.test.ts-snapshots/ActionList-Inactive-Multiselect-dark-tritanopia-linux.png
index e86dd5e7ca6..5e1a05501ce 100644
Binary files a/.playwright/snapshots/components/ActionList.test.ts-snapshots/ActionList-Inactive-Multiselect-dark-tritanopia-linux.png and b/.playwright/snapshots/components/ActionList.test.ts-snapshots/ActionList-Inactive-Multiselect-dark-tritanopia-linux.png differ
diff --git a/.playwright/snapshots/components/ActionList.test.ts-snapshots/ActionList-Inactive-Multiselect-light-colorblind-linux.png b/.playwright/snapshots/components/ActionList.test.ts-snapshots/ActionList-Inactive-Multiselect-light-colorblind-linux.png
index 75f81b51a20..b643fe4c5cf 100644
Binary files a/.playwright/snapshots/components/ActionList.test.ts-snapshots/ActionList-Inactive-Multiselect-light-colorblind-linux.png and b/.playwright/snapshots/components/ActionList.test.ts-snapshots/ActionList-Inactive-Multiselect-light-colorblind-linux.png differ
diff --git a/.playwright/snapshots/components/ActionList.test.ts-snapshots/ActionList-Inactive-Multiselect-light-high-contrast-linux.png b/.playwright/snapshots/components/ActionList.test.ts-snapshots/ActionList-Inactive-Multiselect-light-high-contrast-linux.png
index 53d85d2b4b4..58535b72b2e 100644
Binary files a/.playwright/snapshots/components/ActionList.test.ts-snapshots/ActionList-Inactive-Multiselect-light-high-contrast-linux.png and b/.playwright/snapshots/components/ActionList.test.ts-snapshots/ActionList-Inactive-Multiselect-light-high-contrast-linux.png differ
diff --git a/.playwright/snapshots/components/ActionList.test.ts-snapshots/ActionList-Inactive-Multiselect-light-linux.png b/.playwright/snapshots/components/ActionList.test.ts-snapshots/ActionList-Inactive-Multiselect-light-linux.png
index 75f81b51a20..b643fe4c5cf 100644
Binary files a/.playwright/snapshots/components/ActionList.test.ts-snapshots/ActionList-Inactive-Multiselect-light-linux.png and b/.playwright/snapshots/components/ActionList.test.ts-snapshots/ActionList-Inactive-Multiselect-light-linux.png differ
diff --git a/.playwright/snapshots/components/ActionList.test.ts-snapshots/ActionList-Inactive-Multiselect-light-tritanopia-linux.png b/.playwright/snapshots/components/ActionList.test.ts-snapshots/ActionList-Inactive-Multiselect-light-tritanopia-linux.png
index 75f81b51a20..b643fe4c5cf 100644
Binary files a/.playwright/snapshots/components/ActionList.test.ts-snapshots/ActionList-Inactive-Multiselect-light-tritanopia-linux.png and b/.playwright/snapshots/components/ActionList.test.ts-snapshots/ActionList-Inactive-Multiselect-light-tritanopia-linux.png differ
diff --git a/packages/react/src/ActionList/ActionList.test.tsx b/packages/react/src/ActionList/ActionList.test.tsx
index 7723425a724..dbcf6bf421d 100644
--- a/packages/react/src/ActionList/ActionList.test.tsx
+++ b/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')
diff --git a/packages/react/src/ActionList/Item.test.tsx b/packages/react/src/ActionList/Item.test.tsx
index 0570f62cd61..631b84691cf 100644
--- a/packages/react/src/ActionList/Item.test.tsx
+++ b/packages/react/src/ActionList/Item.test.tsx
@@ -17,6 +17,10 @@ function SimpleActionList(): JSX.Element {
Link Item
+ Inactive item
+
+ Loading and inactive item
+
)
}
@@ -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()
+ 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()
- 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()
+ 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()
- 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()
diff --git a/packages/react/src/ActionList/Item.tsx b/packages/react/src/ActionList/Item.tsx
index 18f0e9cd1c4..8411f9ab3c8 100644
--- a/packages/react/src/ActionList/Item.tsx
+++ b/packages/react/src/ActionList/Item.tsx
@@ -112,7 +112,11 @@ export const Item = React.forwardRef(
} = React.useContext(ListContext)
const {selectionVariant: groupSelectionVariant} = React.useContext(GroupContext)
const inactive = Boolean(inactiveText)
- const showInactiveIndicator = inactive && container === undefined
+ // TODO change `menuContext` check to ```listRole !== undefined && ['menu', 'listbox'].includes(listRole)```
+ // once we have a better way to handle existing usage in dotcom that incorrectly use ActionList.TrailingAction
+ const menuContext = container === 'ActionMenu' || container === 'SelectPanel'
+ // TODO: when we change `menuContext` to check `listRole` instead of `container`
+ const showInactiveIndicator = inactive && !(listRole !== undefined && ['menu', 'listbox'].includes(listRole))
const onSelect = React.useCallback(
(
@@ -142,10 +146,12 @@ export const Item = React.forwardRef(
}
const itemRole = role || inferredItemRole
- const menuContext = container === 'ActionMenu' || container === 'SelectPanel'
if (slots.trailingAction) {
- invariant(!menuContext, `ActionList.TrailingAction can not be used within a ${container}.`)
+ invariant(
+ !menuContext,
+ `ActionList.TrailingAction can not be used within a list with an ARIA role of "menu" or "listbox".`,
+ )
}
/** Infer the proper selection attribute based on the item's role */
@@ -405,7 +411,8 @@ export const Item = React.forwardRef(
{childrenWithoutSlots}
{/* Loading message needs to be in here so it is read with the label */}
- {loading === true && Loading}
+ {/* If the item is inactive, we do not simultaneously announce that it is loading */}
+ {loading === true && !inactive && Loading}
{slots.description}
@@ -422,7 +429,7 @@ export const Item = React.forwardRef(
{
// 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 ? (
{inactiveText}
@@ -477,7 +484,8 @@ export const Item = React.forwardRef(
{childrenWithoutSlots}
{/* Loading message needs to be in here so it is read with the label */}
- {loading === true && Loading}
+ {/* If the item is inactive, we do not simultaneously announce that it is loading */}
+ {loading === true && !inactive && Loading}
{slots.description}
@@ -494,7 +502,7 @@ export const Item = React.forwardRef(
{
// 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 ? (
{inactiveText}
@@ -567,7 +575,8 @@ export const Item = React.forwardRef(
>
{childrenWithoutSlots}
{/* Loading message needs to be in here so it is read with the label */}
- {loading === true && Loading}
+ {/* If the item is inactive, we do not simultaneously announce that it is loading */}
+ {loading === true && !inactive && Loading}
{slots.inlineDescription}
@@ -584,7 +593,7 @@ export const Item = React.forwardRef(
{
// 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 ? (
-
-
diff --git a/packages/react/src/__tests__/__snapshots__/Autocomplete.test.tsx.snap b/packages/react/src/__tests__/__snapshots__/Autocomplete.test.tsx.snap
index a1b03328328..810f70e7c36 100644
--- a/packages/react/src/__tests__/__snapshots__/Autocomplete.test.tsx.snap
+++ b/packages/react/src/__tests__/__snapshots__/Autocomplete.test.tsx.snap
@@ -944,6 +944,12 @@ exports[`snapshots renders a menu that contains an item to add to the menu 1`] =
}
.c9 {
+ font-size: 12px;
+ line-height: 16px;
+ color: var(--fgColor-attention,var(--color-attention-fg,#9a6700));
+}
+
+.c10 {
height: 20px;
max-width: 20px;
display: -webkit-box;
@@ -965,12 +971,12 @@ exports[`snapshots renders a menu that contains an item to add to the menu 1`] =
color: var(--fgColor-muted,var(--color-fg-muted,#656d76));
}
-.c9 svg {
+.c10 svg {
font-size: 12px;
}
[data-variant="danger"]:not([aria-disabled]):not([data-inactive]):hover .c4,
-[data-variant="danger"]:active .c9 {
+[data-variant="danger"]:active .c10 {
color: var(--fgColor-default,var(--color-fg-default,#1F2328));
}
@@ -1100,8 +1106,8 @@ exports[`snapshots renders a menu that contains an item to add to the menu 1`] =
--divider-color: transparent !important;
}
-.c10:hover:not([aria-disabled]):not([data-inactive]):not([data-loading]) + .c2,
-.c10[data-focus-visible-added] + li {
+.c11:hover:not([aria-disabled]):not([data-inactive]):not([data-loading]) + .c2,
+.c11[data-focus-visible-added] + li {
--divider-color: transparent;
}
@@ -1197,6 +1203,9 @@ exports[`snapshots renders a menu that contains an item to add to the menu 1`] =
>
zero
+