Skip to content

Commit

Permalink
Merge 22e5b57 into b9ed8ee
Browse files Browse the repository at this point in the history
  • Loading branch information
mperrotti authored Dec 11, 2023
2 parents b9ed8ee + 22e5b57 commit a910600
Show file tree
Hide file tree
Showing 45 changed files with 672 additions and 150 deletions.
7 changes: 7 additions & 0 deletions .changeset/smooth-tips-breathe.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@primer/react': minor
---

Supports inactive ActionList items by letting users pass the required message to the `inactiveText` prop.

<!-- Changed components: ActionList, ActionMenu, NavList -->
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.
60 changes: 60 additions & 0 deletions e2e/components/ActionList.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -454,4 +454,64 @@ test.describe('ActionList', () => {
})
}
})

test.describe('Inactive Item', () => {
for (const theme of themes) {
test.describe(theme, () => {
test('default @vrt', async ({page}) => {
await visit(page, {
id: 'components-actionlist-features--inactive-item',
globals: {
colorScheme: theme,
},
})

// Default state
expect(await page.screenshot({animations: 'disabled'})).toMatchSnapshot(
`ActionList.Inactive Item.${theme}.png`,
)
})

test('axe @aat', async ({page}) => {
await visit(page, {
id: 'components-actionlist-features--inactive-item',
globals: {
colorScheme: theme,
},
})
await expect(page).toHaveNoViolations()
})
})
}
})

test.describe('Inactive Multiselect', () => {
for (const theme of themes) {
test.describe(theme, () => {
test('default @vrt', async ({page}) => {
await visit(page, {
id: 'components-actionlist-features--inactive-multiselect',
globals: {
colorScheme: theme,
},
})

// Default state
expect(await page.screenshot({animations: 'disabled'})).toMatchSnapshot(
`ActionList.Inactive Multiselect.${theme}.png`,
)
})

test('axe @aat', async ({page}) => {
await visit(page, {
id: 'components-actionlist-features--inactive-multiselect',
globals: {
colorScheme: theme,
},
})
await expect(page).toHaveNoViolations()
})
})
}
})
})
35 changes: 35 additions & 0 deletions e2e/components/ActionMenu.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,41 @@ test.describe('ActionMenu', () => {
}
})

test.describe('Inactive Items', () => {
for (const theme of themes) {
test.describe(theme, () => {
test('default @vrt', async ({page}) => {
await visit(page, {
id: 'components-actionmenu-features--links-and-actions',
globals: {
colorScheme: theme,
},
})

// Default state
expect(await page.screenshot({animations: 'disabled'})).toMatchSnapshot(
`ActionMenu.Links And Actions.${theme}.png`,
)

// Open menu
await page.locator('button', {hasText: 'Open menu'}).waitFor()
await page.getByRole('button', {name: 'Open menu'}).click()
expect(await page.screenshot({animations: 'disabled'})).toMatchSnapshot()
})

test('axe @aat', async ({page}) => {
await visit(page, {
id: 'components-actionmenu-features--links-and-actions',
globals: {
colorScheme: theme,
},
})
await expect(page).toHaveNoViolations()
})
})
}
})

test.describe('Links And Actions', () => {
for (const theme of themes) {
test.describe(theme, () => {
Expand Down
8 changes: 7 additions & 1 deletion src/ActionList/ActionList.docs.json
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@
"name": "onSelect",
"type": "(event: React.MouseEvent<HTMLLIElement> | React.KeyboardEvent<HTMLLIElement>) => void",
"defaultValue": "",
"description": "Callback that is called when the item is selected using either the mouse or keyboard. `event.preventDefault()` will prevent a menu from closing when within an `<ActionMenu />`"
"description": "Callback that is called when the item is selected using either the mouse or keyboard. `event.preventDefault()` will prevent a menu from closing when within an `<ActionMenu />`. This is not called for disabled or inactive items."
},
{
"name": "selected",
Expand All @@ -82,6 +82,12 @@
"defaultValue": "false",
"description": "Items that are disabled can not be clicked, selected, or navigated to."
},
{
"name": "inactiveText",
"type": "string",
"defaultValue": "",
"description": "Text describing why the item is inactive. This may be used when an item's usual functionality is unavailable due to a system error such as a database outage."
},
{
"name": "role",
"type": "AriaRole",
Expand Down
37 changes: 37 additions & 0 deletions src/ActionList/ActionList.examples.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,43 @@ export function AllCombinations(): JSX.Element {
<StarIcon />
</ActionList.TrailingVisual>
</ActionList.Item>
<ActionList.Item inactiveText="Unavailable due to an outage">
L + B + T<ActionList.Description variant="block">Block description</ActionList.Description>
</ActionList.Item>
<ActionList.Item inactiveText="Unavailable due to an outage">
L + B + T<ActionList.Description variant="inline">Inline description</ActionList.Description>
</ActionList.Item>
<ActionList.Item inactiveText="Unavailable due to an outage">
<ActionList.LeadingVisual>
<StarIcon />
</ActionList.LeadingVisual>
L + I + T<ActionList.Description variant="inline">inline description</ActionList.Description>
<ActionList.TrailingVisual>
<StarIcon />
</ActionList.TrailingVisual>
</ActionList.Item>
<ActionList.Item inactiveText="Unavailable due to an outage">
<ActionList.LeadingVisual>
<StarIcon />
</ActionList.LeadingVisual>
L + B + T<ActionList.Description variant="block">Block description</ActionList.Description>
<ActionList.TrailingVisual>
<StarIcon />
</ActionList.TrailingVisual>
</ActionList.Item>
<ActionList.Item inactiveText="Unavailable due to an outage">
L + B + T<ActionList.Description variant="block">Block description</ActionList.Description>
<ActionList.TrailingVisual>
<StarIcon />
</ActionList.TrailingVisual>
</ActionList.Item>
<ActionList.Item inactiveText="Unavailable due to an outage">
I + B + T<ActionList.Description variant="inline">inline description</ActionList.Description>
<ActionList.Description variant="block">Block description</ActionList.Description>
<ActionList.TrailingVisual>
<StarIcon />
</ActionList.TrailingVisual>
</ActionList.Item>
<ActionList.Item variant="danger">
<ActionList.LeadingVisual>
<StarIcon />
Expand Down
61 changes: 61 additions & 0 deletions src/ActionList/ActionList.features.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,25 @@ export const SingleSelect = () => {
)
}

export const InactiveSingleSelect = () => {
const [selectedIndex, setSelectedIndex] = React.useState(1)
return (
<ActionList selectionVariant="single" showDividers role="menu" aria-label="Project">
<ActionList.Item role="menuitem" selected={false} inactiveText="Unavailable due to an outage">
Inactive item
</ActionList.Item>
<ActionList.Item
role="menuitemradio"
selected={selectedIndex === 1}
aria-checked={selectedIndex === 1}
onSelect={() => setSelectedIndex(1)}
>
Item 2
</ActionList.Item>
</ActionList>
)
}

export const MultiSelect = () => {
const [selectedIndices, setSelectedIndices] = React.useState<number[]>([0])
const handleSelect = (index: number) => {
Expand Down Expand Up @@ -322,6 +341,32 @@ export const DisabledMultiselect = () => (
</ActionList>
)

export const InactiveMultiselect = () => {
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" role="menu" aria-label="Project">
<ActionList.Item role="menuitem" selected={false} inactiveText="Unavailable due to an outage">
Inactive item
</ActionList.Item>
<ActionList.Item
role="menuitemcheckbox"
selected={selectedIndices.includes(1)}
aria-checked={selectedIndices.includes(1)}
onSelect={() => handleSelect(1)}
>
Item 2
</ActionList.Item>
</ActionList>
)
}

export const DisabledItem = () => {
const [selectedIndex, setSelectedIndex] = React.useState(0)
return (
Expand All @@ -346,6 +391,22 @@ export const DisabledItem = () => {
)
}

export const InactiveItem = () => {
return (
<ActionList aria-label="Project">
{projects.map((project, index) => (
<ActionList.Item key={index} inactiveText={index === 1 ? 'Unavailable due to an outage' : undefined}>
<ActionList.LeadingVisual>
<TableIcon />
</ActionList.LeadingVisual>
{project.name}
<ActionList.Description variant="block">{project.scope}</ActionList.Description>
</ActionList.Item>
))}
</ActionList>
)
}

export const Links = () => (
<>
<Heading as="h1" id="list-heading" sx={{fontSize: 1}}>
Expand Down
6 changes: 6 additions & 0 deletions src/ActionList/ActionList.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,11 @@ ItemPlayground.argTypes = {
type: 'boolean',
},
},
inactiveText: {
control: {
type: 'text',
},
},
variant: {
control: 'radio',
options: ['default', 'danger'],
Expand Down Expand Up @@ -155,6 +160,7 @@ ItemPlayground.args = {
selected: false,
active: false,
disabled: false,
inactiveText: '',
variant: 'default',
role: 'listitem',
id: 'item-1',
Expand Down
35 changes: 35 additions & 0 deletions src/ActionList/ActionList.test.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {render as HTMLRender, waitFor, fireEvent} from '@testing-library/react'
import userEvent from '@testing-library/user-event'
import {axe} from 'jest-axe'
import React from 'react'
import theme from '../theme'
Expand Down Expand Up @@ -31,6 +32,7 @@ const projects = [
{name: 'Primer Backlog', scope: 'GitHub'},
{name: 'Primer React', scope: 'github/primer'},
{name: 'Disabled Project', scope: 'github/primer', disabled: true},
{name: 'Inactive Project', scope: 'github/primer', inactiveText: 'Unavailable due to an outage'},
]
function SingleSelectListStory(): JSX.Element {
const [selectedIndex, setSelectedIndex] = React.useState(0)
Expand All @@ -45,6 +47,7 @@ function SingleSelectListStory(): JSX.Element {
aria-selected={index === selectedIndex}
onSelect={() => setSelectedIndex(index)}
disabled={project.disabled}
inactiveText={project.inactiveText}
>
{project.name}
</ActionList.Item>
Expand Down Expand Up @@ -123,6 +126,24 @@ describe('ActionList', () => {
expect(options[2]).toHaveAttribute('aria-selected', 'false')
})

it('should skip onSelect on inactive items', async () => {
const component = HTMLRender(<SingleSelectListStory />)
const options = await waitFor(() => component.getAllByRole('option'))

expect(options[0]).toHaveAttribute('aria-selected', 'true')
expect(options[3]).toHaveAttribute('aria-selected', 'false')

fireEvent.click(options[3])

expect(options[0]).toHaveAttribute('aria-selected', 'true')
expect(options[3]).toHaveAttribute('aria-selected', 'false')

fireEvent.keyPress(options[3], {key: 'Enter', charCode: 13})

expect(options[0]).toHaveAttribute('aria-selected', 'true')
expect(options[3]).toHaveAttribute('aria-selected', 'false')
})

it('should throw when selected is provided without a selectionVariant on parent', async () => {
// we expect console.error to be called, so we suppress that in the test
const mockError = jest.spyOn(console, 'error').mockImplementation(() => jest.fn())
Expand Down Expand Up @@ -154,6 +175,20 @@ describe('ActionList', () => {
expect(option).toBeInTheDocument()
})

it('should focus the button around the leading visual when tabbing to an inactive item', async () => {
const component = HTMLRender(<SingleSelectListStory />)
const inactiveOptionButton = await waitFor(() =>
component.getByRole('button', {description: projects[3].inactiveText}),
)
const inactiveIndex = projects.findIndex(project => 'inactiveText' in project)

for (let i = 0; i < inactiveIndex; i++) {
await userEvent.tab()
}

expect(inactiveOptionButton).toHaveFocus()
})

it('should call onClick for a link item', async () => {
const onClick = jest.fn()
const component = HTMLRender(
Expand Down
Loading

0 comments on commit a910600

Please sign in to comment.