Fix missing selection attributes in ActionList items (#4096)

* include selectionAttribute in ListContext * remove aria-selected from test to assert default ActionList selection behavior * changeset * clarify variable names * only apply aria selection attributes to the appropriate roles * infer selection attribute based on item role * remove variable no longer needed * update snapshots
primer · Dec 21, 2023 · 1b9011d · 1b9011d
1 parent 1a1d89c
commit 1b9011d
Show file tree

Hide file tree

Showing 4 changed files with 57 additions and 9 deletions.
diff --git a/.changeset/rare-jeans-rush.md b/.changeset/rare-jeans-rush.md
@@ -0,0 +1,5 @@
+---
+'@primer/react': patch
+---
+
+Fix missing `aria-selected` & `aria-checked` attributes in ActionList items
diff --git a/src/ActionList/ActionList.test.tsx b/src/ActionList/ActionList.test.tsx
@@ -44,7 +44,6 @@ function SingleSelectListStory(): JSX.Element {
           key={index}
           role="option"
           selected={index === selectedIndex}
-          aria-selected={index === selectedIndex}
           onSelect={() => setSelectedIndex(index)}
           disabled={project.disabled}
           inactiveText={project.inactiveText}

diff --git a/src/ActionList/Item.tsx b/src/ActionList/Item.tsx
@@ -98,15 +98,24 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
       : listSelectionVariant
 
     /** Infer item role based on the container */
-    let itemRole: ActionListItemProps['role']
+    let inferredItemRole: ActionListItemProps['role']
     if (container === 'ActionMenu') {
-      if (selectionVariant === 'single') itemRole = 'menuitemradio'
-      else if (selectionVariant === 'multiple') itemRole = 'menuitemcheckbox'
-      else itemRole = 'menuitem'
+      if (selectionVariant === 'single') inferredItemRole = 'menuitemradio'
+      else if (selectionVariant === 'multiple') inferredItemRole = 'menuitemcheckbox'
+      else inferredItemRole = 'menuitem'
     } else if (container === 'SelectPanel' && listRole === 'listbox') {
-      if (selectionVariant !== undefined) itemRole = 'option'
+      if (selectionVariant !== undefined) inferredItemRole = 'option'
     }
 
+    const itemRole = role || inferredItemRole
+
+    /** Infer the proper selection attribute based on the item's role */
+    let inferredSelectionAttribute: 'aria-selected' | 'aria-checked' | undefined
+    if (itemRole === 'menuitemradio' || itemRole === 'menuitemcheckbox') inferredSelectionAttribute = 'aria-checked'
+    else if (itemRole === 'option') inferredSelectionAttribute = 'aria-selected'
+
+    const itemSelectionAttribute = selectionAttribute || inferredSelectionAttribute
+
     const {theme} = useTheme()
 
     const activeStyles = {
@@ -234,6 +243,10 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
 
     const ItemWrapper = _PrivateItemWrapper || React.Fragment
 
+    // only apply aria-selected and aria-checked to selectable items
+    const selectableRoles = ['menuitemradio', 'menuitemcheckbox', 'option']
+    const includeSelectionAttribute = itemSelectionAttribute && itemRole && selectableRoles.includes(itemRole)
+
     const menuItemProps = {
       onClick: clickHandler,
       onKeyPress: keyPressHandler,
@@ -244,12 +257,12 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
       'aria-describedby': slots.blockDescription
         ? [blockDescriptionId, inactiveWarningId].join(' ')
         : inactiveWarningId,
-      ...(selectionAttribute && {[selectionAttribute]: selected}),
-      role: role || itemRole,
+      ...(includeSelectionAttribute && {[itemSelectionAttribute]: selected}),
+      role: itemRole,
       id: itemId,
     }
 
-    const containerProps = _PrivateItemWrapper ? {role: role || itemRole ? 'none' : undefined} : menuItemProps
+    const containerProps = _PrivateItemWrapper ? {role: itemRole ? 'none' : undefined} : menuItemProps
 
     const wrapperProps = _PrivateItemWrapper ? menuItemProps : {}
 

diff --git a/src/__tests__/__snapshots__/Autocomplete.test.tsx.snap b/src/__tests__/__snapshots__/Autocomplete.test.tsx.snap
@@ -789,6 +789,7 @@ exports[`snapshots renders a menu that contains an item to add to the menu 1`] =
       >
         <li
           aria-labelledby="0--label "
+          aria-selected={false}
           className="c2 c3"
           data-id="0"
           id="0"
@@ -819,6 +820,7 @@ exports[`snapshots renders a menu that contains an item to add to the menu 1`] =
         </li>
         <li
           aria-labelledby="1--label "
+          aria-selected={false}
           className="c2 c3"
           data-id="1"
           id="1"
@@ -849,6 +851,7 @@ exports[`snapshots renders a menu that contains an item to add to the menu 1`] =
         </li>
         <li
           aria-labelledby="2--label "
+          aria-selected={false}
           className="c2 c3"
           data-id="2"
           id="2"
@@ -879,6 +882,7 @@ exports[`snapshots renders a menu that contains an item to add to the menu 1`] =
         </li>
         <li
           aria-labelledby="3--label "
+          aria-selected={false}
           className="c2 c3"
           data-id="3"
           id="3"
@@ -909,6 +913,7 @@ exports[`snapshots renders a menu that contains an item to add to the menu 1`] =
         </li>
         <li
           aria-labelledby="4--label "
+          aria-selected={false}
           className="c2 c3"
           data-id="4"
           id="4"
@@ -939,6 +944,7 @@ exports[`snapshots renders a menu that contains an item to add to the menu 1`] =
         </li>
         <li
           aria-labelledby="5--label "
+          aria-selected={false}
           className="c2 c3"
           data-id="5"
           id="5"
@@ -969,6 +975,7 @@ exports[`snapshots renders a menu that contains an item to add to the menu 1`] =
         </li>
         <li
           aria-labelledby="6--label "
+          aria-selected={false}
           className="c2 c3"
           data-id="6"
           id="6"
@@ -999,6 +1006,7 @@ exports[`snapshots renders a menu that contains an item to add to the menu 1`] =
         </li>
         <li
           aria-labelledby="7--label "
+          aria-selected={false}
           className="c2 c3"
           data-id="7"
           id="7"
@@ -1029,6 +1037,7 @@ exports[`snapshots renders a menu that contains an item to add to the menu 1`] =
         </li>
         <li
           aria-labelledby="20--label "
+          aria-selected={false}
           className="c2 c3"
           data-id="20"
           id="20"
@@ -1059,6 +1068,7 @@ exports[`snapshots renders a menu that contains an item to add to the menu 1`] =
         </li>
         <li
           aria-labelledby="21--label "
+          aria-selected={false}
           className="c2 c3"
           data-id="21"
           id="21"
@@ -1089,6 +1099,7 @@ exports[`snapshots renders a menu that contains an item to add to the menu 1`] =
         </li>
         <li
           aria-labelledby="add-new-item--label "
+          aria-selected={false}
           className="c2 c3"
           data-id="add-new-item"
           id="add-new-item"
@@ -1512,6 +1523,7 @@ exports[`snapshots renders a multiselect input 1`] = `
       >
         <li
           aria-labelledby="0--label "
+          aria-selected={false}
           className="c2 c3"
           data-id="0"
           id="0"
@@ -1542,6 +1554,7 @@ exports[`snapshots renders a multiselect input 1`] = `
         </li>
         <li
           aria-labelledby="1--label "
+          aria-selected={false}
           className="c2 c3"
           data-id="1"
           id="1"
@@ -1572,6 +1585,7 @@ exports[`snapshots renders a multiselect input 1`] = `
         </li>
         <li
           aria-labelledby="2--label "
+          aria-selected={false}
           className="c2 c3"
           data-id="2"
           id="2"
@@ -1602,6 +1616,7 @@ exports[`snapshots renders a multiselect input 1`] = `
         </li>
         <li
           aria-labelledby="3--label "
+          aria-selected={false}
           className="c2 c3"
           data-id="3"
           id="3"
@@ -1632,6 +1647,7 @@ exports[`snapshots renders a multiselect input 1`] = `
         </li>
         <li
           aria-labelledby="4--label "
+          aria-selected={false}
           className="c2 c3"
           data-id="4"
           id="4"
@@ -1662,6 +1678,7 @@ exports[`snapshots renders a multiselect input 1`] = `
         </li>
         <li
           aria-labelledby="5--label "
+          aria-selected={false}
           className="c2 c3"
           data-id="5"
           id="5"
@@ -1692,6 +1709,7 @@ exports[`snapshots renders a multiselect input 1`] = `
         </li>
         <li
           aria-labelledby="6--label "
+          aria-selected={false}
           className="c2 c3"
           data-id="6"
           id="6"
@@ -1722,6 +1740,7 @@ exports[`snapshots renders a multiselect input 1`] = `
         </li>
         <li
           aria-labelledby="7--label "
+          aria-selected={false}
           className="c2 c3"
           data-id="7"
           id="7"
@@ -1752,6 +1771,7 @@ exports[`snapshots renders a multiselect input 1`] = `
         </li>
         <li
           aria-labelledby="20--label "
+          aria-selected={false}
           className="c2 c3"
           data-id="20"
           id="20"
@@ -1782,6 +1802,7 @@ exports[`snapshots renders a multiselect input 1`] = `
         </li>
         <li
           aria-labelledby="21--label "
+          aria-selected={false}
           className="c2 c3"
           data-id="21"
           id="21"
@@ -2329,6 +2350,7 @@ exports[`snapshots renders a multiselect input with selected menu items 1`] = `
       >
         <li
           aria-labelledby="0--label "
+          aria-selected={true}
           className="c2 c3"
           data-id="0"
           id="0"
@@ -2359,6 +2381,7 @@ exports[`snapshots renders a multiselect input with selected menu items 1`] = `
         </li>
         <li
           aria-labelledby="1--label "
+          aria-selected={true}
           className="c2 c3"
           data-id="1"
           id="1"
@@ -2389,6 +2412,7 @@ exports[`snapshots renders a multiselect input with selected menu items 1`] = `
         </li>
         <li
           aria-labelledby="2--label "
+          aria-selected={true}
           className="c2 c3"
           data-id="2"
           id="2"
@@ -2419,6 +2443,7 @@ exports[`snapshots renders a multiselect input with selected menu items 1`] = `
         </li>
         <li
           aria-labelledby="3--label "
+          aria-selected={false}
           className="c2 c8"
           data-id="3"
           id="3"
@@ -2449,6 +2474,7 @@ exports[`snapshots renders a multiselect input with selected menu items 1`] = `
         </li>
         <li
           aria-labelledby="4--label "
+          aria-selected={false}
           className="c2 c8"
           data-id="4"
           id="4"
@@ -2479,6 +2505,7 @@ exports[`snapshots renders a multiselect input with selected menu items 1`] = `
         </li>
         <li
           aria-labelledby="5--label "
+          aria-selected={false}
           className="c2 c8"
           data-id="5"
           id="5"
@@ -2509,6 +2536,7 @@ exports[`snapshots renders a multiselect input with selected menu items 1`] = `
         </li>
         <li
           aria-labelledby="6--label "
+          aria-selected={false}
           className="c2 c8"
           data-id="6"
           id="6"
@@ -2539,6 +2567,7 @@ exports[`snapshots renders a multiselect input with selected menu items 1`] = `
         </li>
         <li
           aria-labelledby="7--label "
+          aria-selected={false}
           className="c2 c8"
           data-id="7"
           id="7"
@@ -2569,6 +2598,7 @@ exports[`snapshots renders a multiselect input with selected menu items 1`] = `
         </li>
         <li
           aria-labelledby="20--label "
+          aria-selected={false}
           className="c2 c8"
           data-id="20"
           id="20"
@@ -2599,6 +2629,7 @@ exports[`snapshots renders a multiselect input with selected menu items 1`] = `
         </li>
         <li
           aria-labelledby="21--label "
+          aria-selected={false}
           className="c2 c8"
           data-id="21"
           id="21"