Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds loading state to ActionList items #4051

Merged
merged 33 commits into from
Jul 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
330d2ea
adds loading state to inactive items
mperrotti Dec 11, 2023
6906bec
adds changeset
mperrotti Dec 11, 2023
6fc5176
test(vrt): update snapshots
mperrotti Dec 12, 2023
26a4827
Merge branch 'main' of github.com:primer/react into mp/loading-action…
mperrotti Dec 14, 2023
52056a3
Merge branch 'main' of github.com:primer/react into mp/loading-action…
mperrotti Dec 18, 2023
da33bd8
Merge branch 'main' into mp/loading-actionlist-item
mperrotti Dec 18, 2023
a37edde
test(vrt): update snapshots
mperrotti Dec 18, 2023
4c48183
Merge branch 'main' of github.com:primer/react into mp/loading-action…
mperrotti Jan 10, 2024
17336f0
Merge branch 'mp/loading-actionlist-item' of github.com:primer/react …
mperrotti Jan 10, 2024
19fe10e
moves Spinner changes to a different branch
mperrotti Jan 10, 2024
d54e651
Merge branch 'main' of github.com:primer/react into mp/loading-action…
mperrotti Jan 11, 2024
57fa1c7
test(vrt): update snapshots
mperrotti Jan 11, 2024
022b776
Merge branch 'main' into mp/loading-actionlist-item
mperrotti Jan 17, 2024
c6b92ee
addresses a11y feedback about tooltip label and description
mperrotti Jan 17, 2024
88a1800
Merge branch 'mp/loading-actionlist-item' of github.com:primer/react …
mperrotti Jan 17, 2024
0df204c
Merge branch 'main' of github.com:primer/react into mp/loading-action…
mperrotti Feb 8, 2024
b17b1e1
Merge branch 'main' of github.com:primer/react into mp/loading-action…
mperrotti Jun 27, 2024
3c6da6f
update outdated Tooltip import
mperrotti Jun 27, 2024
a0c28da
attempt to fix circular dependency
mperrotti Jun 27, 2024
1c2f4ea
another attempt at resolving circular dependency
mperrotti Jun 27, 2024
1884847
Merge branch 'main' into mp/loading-actionlist-item
mperrotti Jun 28, 2024
c204c06
fixes merge mistakes
mperrotti Jun 28, 2024
d76380b
Merge branch 'main' of github.com:primer/react into mp/loading-action…
mperrotti Jun 28, 2024
f0fadc9
Merge branch 'mp/loading-actionlist-item' of github.com:primer/react …
mperrotti Jun 28, 2024
a9a08e8
Merge branch 'main' of github.com:primer/react into mp/loading-action…
mperrotti Jul 3, 2024
9e240e3
addresses PR feedback
mperrotti Jul 3, 2024
8ef70c8
Merge branch 'main' into mp/loading-actionlist-item
mperrotti Jul 3, 2024
b0b3651
Merge branch 'main' into mp/loading-actionlist-item
mperrotti Jul 3, 2024
5090548
rm loading link item from feature story
mperrotti Jul 3, 2024
1a54bb6
test(vrt): update snapshots
mperrotti Jul 3, 2024
91aadc0
rm 'default state test' assertion for ActionMenus w/ loading and inac…
mperrotti Jul 3, 2024
38c6643
Merge branch 'mp/loading-actionlist-item' of github.com:primer/react …
mperrotti Jul 3, 2024
2c2f814
addresses last bit of PR feedback
mperrotti Jul 5, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/lucky-squids-happen.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@primer/react': minor
---

Adds a loading state to ActionList items. Also allows the Spinner component to accept screenreader text.

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

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

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

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

test.describe('Group With Filled Title', () => {
for (const theme of themes) {
test.describe(theme, () => {
Expand Down
30 changes: 30 additions & 0 deletions e2e/components/ActionMenu.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,36 @@ test.describe('ActionMenu', () => {
}
})

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

// 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--loading-items',
globals: {
colorScheme: theme,
},
})
await expect(page).toHaveNoViolations()
})
})
}
})

test.describe('Multi Select', () => {
for (const theme of themes) {
test.describe(theme, () => {
Expand Down
5 changes: 5 additions & 0 deletions packages/react/src/ActionList/ActionList.docs.json
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,11 @@
"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. \nIf there is a leading visual, the alert icon will replace the leading visual. \n If there is a trailing visual, it will replace the trailing visual.\n If there is no visual passed, it will be shown in the trailing visual slot to preserve left alignment of item content. \nText will appear in a tooltip triggered by the alert icon in ActionList items, but text will appear below the description or title on ActionMenu items."
},
{
"name": "loading",
"type": "boolean",
"description": "Whether the item is loading."
},
{
"name": "role",
"type": "AriaRole",
Expand Down
39 changes: 39 additions & 0 deletions packages/react/src/ActionList/ActionList.examples.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,7 @@ export function AllCombinations(): JSX.Element {
<StarIcon />
</ActionList.TrailingVisual>
</ActionList.Item>
{/* Inactive items */}
<ActionList.Item inactiveText="Unavailable due to an outage">
L + B + T<ActionList.Description variant="block">Block description</ActionList.Description>
</ActionList.Item>
Expand Down Expand Up @@ -493,6 +494,44 @@ export function AllCombinations(): JSX.Element {
<StarIcon />
</ActionList.TrailingVisual>
</ActionList.Item>
{/* Loading items */}
<ActionList.Item loading>
L + B + T<ActionList.Description variant="block">Block description</ActionList.Description>
</ActionList.Item>
<ActionList.Item loading>
L + B + T<ActionList.Description variant="inline">Inline description</ActionList.Description>
</ActionList.Item>
<ActionList.Item loading>
<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 loading>
<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 loading>
L + B + T<ActionList.Description variant="block">Block description</ActionList.Description>
<ActionList.TrailingVisual>
<StarIcon />
</ActionList.TrailingVisual>
</ActionList.Item>
<ActionList.Item loading>
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
13 changes: 13 additions & 0 deletions packages/react/src/ActionList/ActionList.features.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,19 @@ export const InactiveItem = () => {
)
}

export const LoadingItem = () => {
return (
<ActionList aria-label="Project">
{projects.map((project, index) => (
<ActionList.Item key={index} loading={index === 1}>
{project.name}
<ActionList.Description variant="block">{project.scope}</ActionList.Description>
</ActionList.Item>
))}
</ActionList>
)
}

export const Links = () => (
<>
<ActionList.Heading as="h1" sx={{fontSize: 1}}>
Expand Down
12 changes: 12 additions & 0 deletions packages/react/src/ActionList/ActionList.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,11 @@ ItemPlayground.argTypes = {
},
options: icons,
},
loading: {
control: {
type: 'boolean',
},
},
trailingVisual: {
control: {
type: 'select',
Expand All @@ -164,6 +169,7 @@ ItemPlayground.args = {
variant: 'default',
id: 'item-1',
leadingVisual: null,
loading: false,
trailingVisual: null,
selectionVariant: 'single',
}
Expand Down Expand Up @@ -216,6 +222,7 @@ LinkItemPlayground.args = {
id: 'item-1',
inactiveText: '',
leadingVisual: null,
loading: false,
trailingVisual: null,
}
LinkItemPlayground.argTypes = {
Expand All @@ -241,6 +248,11 @@ LinkItemPlayground.argTypes = {
},
options: icons,
},
loading: {
control: {
type: 'boolean',
},
},
trailingVisual: {
control: {
type: 'select',
Expand Down
44 changes: 40 additions & 4 deletions packages/react/src/ActionList/ActionList.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,13 @@ const projects = [
{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'},
{name: 'Loading Project', scope: 'github/primer', loading: true},
{
name: 'Inactive and Loading Project',
scope: 'github/primer',
loading: true,
inactiveText: 'Unavailable due to an outage, but loading still passed',
},
]
function SingleSelectListStory(): JSX.Element {
const [selectedIndex, setSelectedIndex] = React.useState(0)
Expand All @@ -49,6 +56,7 @@ function SingleSelectListStory(): JSX.Element {
onSelect={() => setSelectedIndex(index)}
disabled={project.disabled}
inactiveText={project.inactiveText}
loading={project.loading}
>
{project.name}
</ActionList.Item>
Expand Down Expand Up @@ -145,6 +153,24 @@ describe('ActionList', () => {
expect(options[3]).toHaveAttribute('aria-selected', 'false')
})

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

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

fireEvent.click(options[4])

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

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

expect(options[0]).toHaveAttribute('aria-selected', 'true')
expect(options[4]).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 @@ -178,10 +204,20 @@ describe('ActionList', () => {

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)
const inactiveOptionButton = await waitFor(() => component.getByRole('button', {name: projects[3].inactiveText}))
const inactiveIndex = projects.findIndex(project => project.inactiveText === projects[3].inactiveText)

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

expect(inactiveOptionButton).toHaveFocus()
})

it('should behave as inactive if both inactiveText and loading props are passed', async () => {
const component = HTMLRender(<SingleSelectListStory />)
const inactiveOptionButton = await waitFor(() => component.getByRole('button', {name: projects[5].inactiveText}))
const inactiveIndex = projects.findIndex(project => project.inactiveText === projects[5].inactiveText)

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