Skip to content

Commit

Permalink
Update ActionList checkbox styles (#3607)
Browse files Browse the repository at this point in the history
* add checkbox styles

* add stories

* add stories to e2e

* lint

* test(vrt): update snapshots

* fix test

* test(vrt): update snapshots

* snapshot fixes

* test(vrt): update snapshots

* Create silly-dots-lick.md

* thanks josh!

* Update silly-dots-lick.md

* test(vrt): update snapshots

* docs(ActionList): update disabled multi-select examples

---------

Co-authored-by: langermank <langermank@users.noreply.github.com>
Co-authored-by: Josh Black <joshblack@users.noreply.github.com>
  • Loading branch information
3 people authored Aug 24, 2023
1 parent 1eb4142 commit c0e22fc
Show file tree
Hide file tree
Showing 38 changed files with 206 additions and 138 deletions.
7 changes: 7 additions & 0 deletions .changeset/silly-dots-lick.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@primer/react": minor
---

Update ActionList checkbox styles to form checkbox styles (impacts ActionMenu and SelectPanel)

<!-- Changed components: ActionList -->
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified blob-report/report-1.zip
Binary file not shown.
Binary file modified blob-report/report-2.zip
Binary file not shown.
Binary file modified blob-report/report-3.zip
Binary file not shown.
Binary file modified blob-report/report-4.zip
Binary file not shown.
157 changes: 49 additions & 108 deletions e2e/components/ActionList.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,7 @@ test.describe('ActionList', () => {
colorScheme: theme,
},
})
await expect(page).toHaveNoViolations({
rules: {
'color-contrast': {
enabled: theme !== 'dark_dimmed',
},
},
})
await expect(page).toHaveNoViolations()
})
})
}
Expand Down Expand Up @@ -59,13 +53,7 @@ test.describe('ActionList', () => {
colorScheme: theme,
},
})
await expect(page).toHaveNoViolations({
rules: {
'color-contrast': {
enabled: theme !== 'dark_dimmed',
},
},
})
await expect(page).toHaveNoViolations()
})
})
}
Expand Down Expand Up @@ -93,13 +81,7 @@ test.describe('ActionList', () => {
colorScheme: theme,
},
})
await expect(page).toHaveNoViolations({
rules: {
'color-contrast': {
enabled: theme !== 'dark_dimmed',
},
},
})
await expect(page).toHaveNoViolations()
})
})
}
Expand Down Expand Up @@ -127,13 +109,7 @@ test.describe('ActionList', () => {
colorScheme: theme,
},
})
await expect(page).toHaveNoViolations({
rules: {
'color-contrast': {
enabled: theme !== 'dark_dimmed',
},
},
})
await expect(page).toHaveNoViolations()
})
})
}
Expand Down Expand Up @@ -161,13 +137,7 @@ test.describe('ActionList', () => {
colorScheme: theme,
},
})
await expect(page).toHaveNoViolations({
rules: {
'color-contrast': {
enabled: theme !== 'dark_dimmed',
},
},
})
await expect(page).toHaveNoViolations()
})
})
}
Expand Down Expand Up @@ -195,13 +165,7 @@ test.describe('ActionList', () => {
colorScheme: theme,
},
})
await expect(page).toHaveNoViolations({
rules: {
'color-contrast': {
enabled: theme !== 'dark_dimmed',
},
},
})
await expect(page).toHaveNoViolations()
})
})
}
Expand Down Expand Up @@ -229,13 +193,7 @@ test.describe('ActionList', () => {
colorScheme: theme,
},
})
await expect(page).toHaveNoViolations({
rules: {
'color-contrast': {
enabled: theme !== 'dark_dimmed',
},
},
})
await expect(page).toHaveNoViolations()
})
})
}
Expand All @@ -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}) => {
Expand All @@ -263,13 +223,7 @@ test.describe('ActionList', () => {
colorScheme: theme,
},
})
await expect(page).toHaveNoViolations({
rules: {
'color-contrast': {
enabled: theme !== 'dark_dimmed',
},
},
})
await expect(page).toHaveNoViolations()
})
})
}
Expand Down Expand Up @@ -297,13 +251,7 @@ test.describe('ActionList', () => {
colorScheme: theme,
},
})
await expect(page).toHaveNoViolations({
rules: {
'color-contrast': {
enabled: theme !== 'dark_dimmed',
},
},
})
await expect(page).toHaveNoViolations()
})
})
}
Expand Down Expand Up @@ -331,13 +279,7 @@ test.describe('ActionList', () => {
colorScheme: theme,
},
})
await expect(page).toHaveNoViolations({
rules: {
'color-contrast': {
enabled: theme !== 'dark_dimmed',
},
},
})
await expect(page).toHaveNoViolations()
})
})
}
Expand Down Expand Up @@ -365,13 +307,7 @@ test.describe('ActionList', () => {
colorScheme: theme,
},
})
await expect(page).toHaveNoViolations({
rules: {
'color-contrast': {
enabled: theme !== 'dark_dimmed',
},
},
})
await expect(page).toHaveNoViolations()
})
})
}
Expand Down Expand Up @@ -399,13 +335,7 @@ test.describe('ActionList', () => {
colorScheme: theme,
},
})
await expect(page).toHaveNoViolations({
rules: {
'color-contrast': {
enabled: theme !== 'dark_dimmed',
},
},
})
await expect(page).toHaveNoViolations()
})
})
}
Expand Down Expand Up @@ -433,13 +363,7 @@ test.describe('ActionList', () => {
colorScheme: theme,
},
})
await expect(page).toHaveNoViolations({
rules: {
'color-contrast': {
enabled: theme !== 'dark_dimmed',
},
},
})
await expect(page).toHaveNoViolations()
})
})
}
Expand Down Expand Up @@ -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,
},
Expand Down
8 changes: 8 additions & 0 deletions script/generate-e2e-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
},
],
},
],
Expand Down
39 changes: 34 additions & 5 deletions src/ActionList/ActionList.features.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 />
Expand All @@ -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 (
Expand Down
8 changes: 7 additions & 1 deletion src/ActionList/Item.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
Loading

0 comments on commit c0e22fc

Please sign in to comment.