Update ActionList checkbox styles

.changeset/silly-dots-lick.md

@@ -0,0 +1,7 @@
+---
+"@primer/react": minor
+---
+
+Update ActionList checkbox styles to form checkbox styles (impacts ActionMenu and SelectPanel)
+
+<!-- Changed components: ActionList -->

e2e/components/ActionList.test.ts

@@ -25,13 +25,7 @@ test.describe('ActionList', () => {
               colorScheme: theme,
             },
           })
-          await expect(page).toHaveNoViolations({
-            rules: {
-              'color-contrast': {
-                enabled: theme !== 'dark_dimmed',
-              },
-            },
-          })
+          await expect(page).toHaveNoViolations()
         })
       })
     }
@@ -59,13 +53,7 @@ test.describe('ActionList', () => {
               colorScheme: theme,
             },
           })
-          await expect(page).toHaveNoViolations({
-            rules: {
-              'color-contrast': {
-                enabled: theme !== 'dark_dimmed',
-              },
-            },
-          })
+          await expect(page).toHaveNoViolations()
         })
       })
     }
@@ -93,13 +81,7 @@ test.describe('ActionList', () => {
               colorScheme: theme,
             },
           })
-          await expect(page).toHaveNoViolations({
-            rules: {
-              'color-contrast': {
-                enabled: theme !== 'dark_dimmed',
-              },
-            },
-          })
+          await expect(page).toHaveNoViolations()
         })
       })
     }
@@ -127,13 +109,7 @@ test.describe('ActionList', () => {
               colorScheme: theme,
             },
           })
-          await expect(page).toHaveNoViolations({
-            rules: {
-              'color-contrast': {
-                enabled: theme !== 'dark_dimmed',
-              },
-            },
-          })
+          await expect(page).toHaveNoViolations()
         })
       })
     }
@@ -161,13 +137,7 @@ test.describe('ActionList', () => {
               colorScheme: theme,
             },
           })
-          await expect(page).toHaveNoViolations({
-            rules: {
-              'color-contrast': {
-                enabled: theme !== 'dark_dimmed',
-              },
-            },
-          })
+          await expect(page).toHaveNoViolations()
         })
       })
     }
@@ -195,13 +165,7 @@ test.describe('ActionList', () => {
               colorScheme: theme,
             },
           })
-          await expect(page).toHaveNoViolations({
-            rules: {
-              'color-contrast': {
-                enabled: theme !== 'dark_dimmed',
-              },
-            },
-          })
+          await expect(page).toHaveNoViolations()
         })
       })
     }
@@ -229,13 +193,7 @@ test.describe('ActionList', () => {
               colorScheme: theme,
             },
           })
-          await expect(page).toHaveNoViolations({
-            rules: {
-              'color-contrast': {
-                enabled: theme !== 'dark_dimmed',
-              },
-            },
-          })
+          await expect(page).toHaveNoViolations()
         })
       })
     }
@@ -253,7 +211,9 @@ test.describe('ActionList', () => {
           })
 
           // Default state
-          expect(await page.screenshot()).toMatchSnapshot(`ActionList.Multi Select.${theme}.png`)
+          expect(await page.screenshot({animations: 'disabled'})).toMatchSnapshot(
+            `ActionList.Multi Select.${theme}.png`,
+          )
         })
 
         test('axe @aat', async ({page}) => {
@@ -263,13 +223,7 @@ test.describe('ActionList', () => {
               colorScheme: theme,
             },
           })
-          await expect(page).toHaveNoViolations({
-            rules: {
-              'color-contrast': {
-                enabled: theme !== 'dark_dimmed',
-              },
-            },
-          })
+          await expect(page).toHaveNoViolations()
         })
       })
     }
@@ -297,13 +251,7 @@ test.describe('ActionList', () => {
               colorScheme: theme,
             },
           })
-          await expect(page).toHaveNoViolations({
-            rules: {
-              'color-contrast': {
-                enabled: theme !== 'dark_dimmed',
-              },
-            },
-          })
+          await expect(page).toHaveNoViolations()
         })
       })
     }
@@ -331,13 +279,7 @@ test.describe('ActionList', () => {
               colorScheme: theme,
             },
           })
-          await expect(page).toHaveNoViolations({
-            rules: {
-              'color-contrast': {
-                enabled: theme !== 'dark_dimmed',
-              },
-            },
-          })
+          await expect(page).toHaveNoViolations()
         })
       })
     }
@@ -365,13 +307,7 @@ test.describe('ActionList', () => {
               colorScheme: theme,
             },
           })
-          await expect(page).toHaveNoViolations({
-            rules: {
-              'color-contrast': {
-                enabled: theme !== 'dark_dimmed',
-              },
-            },
-          })
+          await expect(page).toHaveNoViolations()
         })
       })
     }
@@ -399,13 +335,7 @@ test.describe('ActionList', () => {
               colorScheme: theme,
             },
           })
-          await expect(page).toHaveNoViolations({
-            rules: {
-              'color-contrast': {
-                enabled: theme !== 'dark_dimmed',
-              },
-            },
-          })
+          await expect(page).toHaveNoViolations()
         })
       })
     }
@@ -433,13 +363,7 @@ test.describe('ActionList', () => {
               colorScheme: theme,
             },
           })
-          await expect(page).toHaveNoViolations({
-            rules: {
-              'color-contrast': {
-                enabled: theme !== 'dark_dimmed',
-              },
-            },
-          })
+          await expect(page).toHaveNoViolations()
         })
       })
     }
@@ -467,43 +391,60 @@ test.describe('ActionList', () => {
               colorScheme: theme,
             },
           })
-          await expect(page).toHaveNoViolations({
-            rules: {
-              'color-contrast': {
-                enabled: theme !== 'dark_dimmed',
-              },
+          await expect(page).toHaveNoViolations()
+        })
+      })
+    }
+  })
+
+  test.describe('Disabled Multiselect', () => {
+    for (const theme of themes) {
+      test.describe(theme, () => {
+        test('default @vrt', async ({page}) => {
+          await visit(page, {
+            id: 'components-actionlist-features--disabled-multiselect',
+            globals: {
+              colorScheme: theme,
+            },
+          })
+
+          // Default state
+          expect(await page.screenshot()).toMatchSnapshot(`ActionList.Disabled Multiselect.${theme}.png`)
+        })
+
+        test('axe @aat', async ({page}) => {
+          await visit(page, {
+            id: 'components-actionlist-features--disabled-multiselect',
+            globals: {
+              colorScheme: theme,
             },
           })
+          await expect(page).toHaveNoViolations()
         })
       })
     }
   })
 
-  test.describe('All combinations', () => {
+  test.describe('Disabled Selected Multiselect', () => {
     for (const theme of themes) {
       test.describe(theme, () => {
         test('default @vrt', async ({page}) => {
           await visit(page, {
-            id: 'components-actionlist-examples--all-combinations',
+            id: 'components-actionlist-features--disabled-selected-multiselect',
             globals: {
               colorScheme: theme,
             },
           })
 
-          // Resize to fit long page
-          await page.setViewportSize({width: 1000, height: 1000})
-
           // Default state
-          expect(await page.screenshot()).toMatchSnapshot(`ActionList.All combinations.${theme}.png`)
-
-          // Hover state
-          await page.getByRole('listitem', {name: 'Danger inline description'}).hover()
-          expect(await page.screenshot()).toMatchSnapshot(`ActionList.All combinations.${theme}.hover.png`)
+          expect(await page.screenshot({animations: 'disabled'})).toMatchSnapshot(
+            `ActionList.Disabled Selected Multiselect.${theme}.png`,
+          )
         })
 
         test('axe @aat', async ({page}) => {
           await visit(page, {
-            id: 'components-actionlist-examples--all-combinations',
+            id: 'components-actionlist-features--disabled-selected-multiselect',
             globals: {
               colorScheme: theme,
             },

script/generate-e2e-tests.js

@@ -68,6 +68,14 @@ const components = new Map([
           id: 'components-actionlist-features--with-icons',
           name: 'With Icons',
         },
+        {
+          id: 'components-actionlist-features--disabled-multiselect',
+          name: 'Disabled Multiselect',
+        },
+        {
+          id: 'components-actionlist-features--disabled-selected-multiselect',
+          name: 'Disabled Selected Multiselect',
+        },
       ],
     },
   ],

src/ActionList/ActionList.features.stories.tsx

@@ -176,16 +176,23 @@ export const SingleSelect = () => {
 }
 
 export const MultiSelect = () => {
-  const [selectedIndex, setSelectedIndex] = React.useState(0)
+  const [selectedIndices, setSelectedIndices] = React.useState<number[]>([0])
+  const handleSelect = (index: number) => {
+    if (selectedIndices.includes(index)) {
+      setSelectedIndices(selectedIndices.filter(i => i !== index))
+    } else {
+      setSelectedIndices([...selectedIndices, index])
+    }
+  }
   return (
     <ActionList selectionVariant="multiple" showDividers role="menu" aria-label="Project">
       {projects.map((project, index) => (
         <ActionList.Item
           key={index}
-          role="menuitemradio"
-          selected={index === selectedIndex}
-          aria-checked={index === selectedIndex}
-          onSelect={() => setSelectedIndex(index)}
+          role="menuitemcheckbox"
+          selected={selectedIndices.includes(index)}
+          aria-checked={selectedIndices.includes(index)}
+          onSelect={() => handleSelect(index)}
         >
           <ActionList.LeadingVisual>
             <TableIcon />
@@ -198,6 +205,28 @@ export const MultiSelect = () => {
   )
 }
 
+export const DisabledSelectedMultiselect = () => (
+  <ActionList selectionVariant="multiple" role="menu" aria-label="Project">
+    <ActionList.Item role="menuitemcheckbox" selected aria-checked disabled>
+      Selected disabled item
+    </ActionList.Item>
+    <ActionList.Item role="menuitemcheckbox" selected={false} aria-checked={false}>
+      Item 2
+    </ActionList.Item>
+  </ActionList>
+)
+
+export const DisabledMultiselect = () => (
+  <ActionList selectionVariant="multiple" role="menu" aria-label="Project">
+    <ActionList.Item role="menuitemcheckbox" selected={false} aria-checked={false} disabled>
+      Disabled item
+    </ActionList.Item>
+    <ActionList.Item role="menuitemcheckbox" selected={false} aria-checked={false}>
+      Item 2
+    </ActionList.Item>
+  </ActionList>
+)
+
 export const DisabledItem = () => {
   const [selectedIndex, setSelectedIndex] = React.useState(0)
   return (

src/ActionList/Item.tsx

@@ -97,7 +97,13 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
       transition: 'background 33.333ms linear',
       color: getVariantStyles(variant, disabled).color,
       cursor: 'pointer',
-      '&[aria-disabled]': {cursor: 'not-allowed'},
+      '&[aria-disabled]': {
+        cursor: 'not-allowed',
+        '[data-component="ActionList.Checkbox"]': {
+          bg: selected ? 'fg.muted' : 'var(--color-input-disabled-bg, rgba(175, 184, 193, 0.2))',
+          borderColor: selected ? 'fg.muted' : 'var(--color-input-disabled-bg, rgba(175, 184, 193, 0.2))',
+        },
+      },
 
       // Button reset styles (to support as="button")
       appearance: 'none',