Skip to content

Commit

Permalink
Merge 574080c into 55e97a9
Browse files Browse the repository at this point in the history
  • Loading branch information
khiga8 committed Jun 28, 2024
2 parents 55e97a9 + 574080c commit bf1b7ad
Show file tree
Hide file tree
Showing 27 changed files with 283 additions and 7 deletions.
5 changes: 5 additions & 0 deletions .changeset/tender-parrots-refuse.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/react": patch
---

Add TrailingAction support to 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.
63 changes: 63 additions & 0 deletions e2e/components/NavList.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import {test, expect} from '@playwright/test'
import {visit} from '../test-helpers/storybook'
import {themes} from '../test-helpers/themes'

test.describe('NavList', () => {
test.describe('With TrailingAction', () => {
for (const theme of themes) {
test.describe(theme, () => {
test('default @vrt', async ({page}) => {
await visit(page, {
id: 'components-navlist--with-trailing-action',
globals: {
colorScheme: theme,
},
})

// Default state
expect(await page.screenshot()).toMatchSnapshot(`NavList.With TrailingAction.${theme}.png`)
})

test('axe @aat', async ({page}) => {
await visit(page, {
id: 'components-navlist--with-trailing-action',
globals: {
colorScheme: theme,
},
})
await expect(page).toHaveNoViolations()
})
})
}
})

test.describe('With Bad Example of SubNav and TrailingAction', () => {
for (const theme of themes) {
test.describe(theme, () => {
test('default @vrt', async ({page}) => {
await visit(page, {
id: 'components-navlist--with-bad-example-of-sub-nav-and-trailing-action',
globals: {
colorScheme: theme,
},
})

// Default state
expect(await page.screenshot()).toMatchSnapshot(
`NavList.With Bad Example of SubNav and TrailingAction.${theme}.png`,
)
})

test('axe @aat', async ({page}) => {
await visit(page, {
id: 'components-navlist--with-bad-example-of-sub-nav-and-trailing-action',
globals: {
colorScheme: theme,
},
})
await expect(page).toHaveNoViolations()
})
})
}
})
})
5 changes: 3 additions & 2 deletions packages/react/src/ActionList/Item.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -127,9 +127,10 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
}

const itemRole = role || inferredItemRole
const menuContext = container === 'ActionMenu' || container === 'SelectPanel'

if (slots.trailingAction) {
invariant(!container, `ActionList.TrailingAction can not be used within a ${container}.`)
invariant(!menuContext, `ActionList.TrailingAction can not be used within a ${container}.`)
}

/** Infer the proper selection attribute based on the item's role */
Expand Down Expand Up @@ -455,7 +456,7 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
{slots.blockDescription}
</Box>
</ItemWrapper>
{!inactive && Boolean(slots.trailingAction) && !container && slots.trailingAction}
{!inactive && !menuContext && Boolean(slots.trailingAction) && slots.trailingAction}
</LiBox>
</ItemContext.Provider>
)
Expand Down
1 change: 1 addition & 0 deletions packages/react/src/ActionList/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export type {ActionListDividerProps} from './Divider'
export type {ActionListDescriptionProps} from './Description'
export type {ActionListLeadingVisualProps, ActionListTrailingVisualProps} from './Visuals'
export type {ActionListHeadingProps} from './Heading'
export type {ActionListTrailingActionProps} from './TrailingAction'

/**
* Collection of list-related components.
Expand Down
31 changes: 31 additions & 0 deletions packages/react/src/NavList/NavList.docs.json
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,37 @@
"type": "React.RefObject<HTMLElement>"
}
]
},
{
"name": "NavList.TrailingAction",
"props": [
{
"name": "as",
"type": "a | button",
"defaultValue": "button",
"required": false,
"description": "HTML element to render as."
},
{
"name": "label",
"type": "string",
"defaultValue": "",
"required": true,
"description": "Acccessible name for the control."
},
{
"name": "icon",
"type": "string",
"defaultValue": "",
"required": true,
"description": "Octicon to pass into IconButton. When this is not set, TrailingAction renders as a `Button` instead of an `IconButton`."
},
{
"name": "href",
"type": "string",
"description": "href when the TrailingAction is rendered as a link."
}
]
}
]
}
88 changes: 88 additions & 0 deletions packages/react/src/NavList/NavList.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import type {Meta, StoryFn} from '@storybook/react'
import React from 'react'
import {PageLayout} from '../PageLayout'
import {NavList} from './NavList'
import {ArrowRightIcon, ArrowLeftIcon, BookIcon, FileDirectoryIcon} from '@primer/octicons-react'

const meta: Meta = {
title: 'Components/NavList',
Expand Down Expand Up @@ -246,4 +247,91 @@ export const WithGroup = () => (
</PageLayout>
)

export const WithTrailingAction = () => {
return (
<PageLayout>
<PageLayout.Pane position="start">
<NavList>
<NavList.Item>
<NavList.LeadingVisual>
<FileDirectoryIcon />
</NavList.LeadingVisual>
Item 1
<NavList.TrailingAction label="Expand sidebar" icon={ArrowLeftIcon} />
</NavList.Item>
<NavList.Item>
Item 2
<NavList.TrailingAction as="a" href="#" label="Some action" icon={ArrowRightIcon} />
</NavList.Item>
</NavList>
</PageLayout.Pane>
</PageLayout>
)
}

export const WithTrailingActionInSubItem = () => {
return (
<PageLayout>
<PageLayout.Pane position="start">
<NavList>
<NavList.Item>
<NavList.LeadingVisual>
<FileDirectoryIcon />
</NavList.LeadingVisual>
Item 1
<NavList.TrailingAction label="Expand sidebar" icon={ArrowLeftIcon} />
</NavList.Item>
<NavList.Item>
Item 2
<NavList.TrailingAction as="a" href="#" label="Some action" icon={ArrowRightIcon} />
</NavList.Item>
<NavList.Item>
Item 3
<NavList.SubNav>
<NavList.Item href="#">
Sub item 1
<NavList.TrailingAction label="Another action" icon={BookIcon} />
</NavList.Item>
</NavList.SubNav>
</NavList.Item>
</NavList>
</PageLayout.Pane>
</PageLayout>
)
}

export const WithBadExampleOfSubNavAndTrailingAction = () => {
return (
<PageLayout>
<PageLayout.Pane position="start">
<NavList>
<NavList.Item>
<NavList.LeadingVisual>
<FileDirectoryIcon />
</NavList.LeadingVisual>
Item 1
<NavList.TrailingAction label="Expand sidebar" icon={ArrowLeftIcon} />
</NavList.Item>
<NavList.Item>
Item 2
<NavList.TrailingAction as="a" href="#" label="Some action" icon={ArrowRightIcon} />
</NavList.Item>
<NavList.Item>
Item 3
<NavList.TrailingAction label="This is not supported and should not render!!!!!" icon={BookIcon} />
<NavList.SubNav>
<NavList.Item href="#">
Sub item 1
<NavList.TrailingAction label="Another action" icon={BookIcon} />
</NavList.Item>
</NavList.SubNav>
</NavList.Item>
</NavList>
</PageLayout.Pane>
</PageLayout>
)
}

WithBadExampleOfSubNavAndTrailingAction.storyName = 'With SubNav and Trailing Action (Bad example, do not copy)'

export default meta
54 changes: 54 additions & 0 deletions packages/react/src/NavList/NavList.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {render, fireEvent} from '@testing-library/react'
import React from 'react'
import {ThemeProvider, SSRProvider} from '..'
import {NavList} from './NavList'
import {FeatureFlags} from '../FeatureFlags'

type ReactRouterLikeLinkProps = {to: string; children: React.ReactNode}

Expand Down Expand Up @@ -65,6 +66,20 @@ describe('NavList', () => {
)
expect(container).toMatchSnapshot()
})

it('supports TrailingAction', async () => {
const {getByRole} = render(
<NavList>
<NavList.Item>
Item 1
<NavList.TrailingAction label="Some trailing action" />
</NavList.Item>
</NavList>,
)

const trailingAction = getByRole('button', {name: 'Some trailing action'})
expect(trailingAction).toBeInTheDocument()
})
})

describe('NavList.Item', () => {
Expand Down Expand Up @@ -334,4 +349,43 @@ describe('NavList.Item with NavList.SubNav', () => {
const currentLink = queryByRole('link', {name: 'Current'})
expect(currentLink).toBeVisible()
})

describe('TrailingAction', () => {
function NavListWithSubNavAndTrailingAction() {
return (
<FeatureFlags flags={{primer_react_action_list_item_as_button: true}}>
<NavList>
<NavList.Item href="#">
Item
<NavList.TrailingAction label="This should not be rendered" />
<NavList.SubNav>
<NavList.Item href="#">
Sub Item 1
<NavList.TrailingAction label="Trailing Action for Sub Item 1" />
</NavList.Item>
<NavList.Item href="#">Sub Item 2</NavList.Item>
</NavList.SubNav>
</NavList.Item>
</NavList>
</FeatureFlags>
)
}

it('does not render TrailingAction alongside SubNav', async () => {
const {queryByRole} = render(<NavListWithSubNavAndTrailingAction />)

const trailingAction = queryByRole('button', {name: 'This should not be rendered'})
expect(trailingAction).toBeNull()
})

it('supports TrailingAction within an Item inside SubNav', async () => {
const {getByRole, queryByRole} = render(<NavListWithSubNavAndTrailingAction />)

const itemWithSubNav = getByRole('button', {name: 'Item'})
fireEvent.click(itemWithSubNav)

expect(queryByRole('link', {name: 'Sub Item 1'})).toBeVisible()
expect(queryByRole('button', {name: 'Trailing Action for Sub Item 1'})).toBeVisible()
})
})
})
24 changes: 19 additions & 5 deletions packages/react/src/NavList/NavList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,12 @@ import {ChevronDownIcon} from '@primer/octicons-react'
import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic'
import React, {isValidElement} from 'react'
import styled from 'styled-components'
import type {ActionListDividerProps, ActionListLeadingVisualProps, ActionListTrailingVisualProps} from '../ActionList'
import type {
ActionListTrailingActionProps,
ActionListDividerProps,
ActionListLeadingVisualProps,
ActionListTrailingVisualProps,
} from '../ActionList'
import {ActionList} from '../ActionList'
import {ActionListContainerContext} from '../ActionList/ActionListContainerContext'
import Box from '../Box'
Expand Down Expand Up @@ -65,9 +70,9 @@ const Item = React.forwardRef<HTMLAnchorElement, NavListItemProps>(
// Get SubNav from children
const subNav = React.Children.toArray(children).find(child => isValidElement(child) && child.type === SubNav)

// Get children without SubNav
const childrenWithoutSubNav = React.Children.toArray(children).filter(child =>
isValidElement(child) ? child.type !== SubNav : true,
// Get children without SubNav or TrailingAction
const childrenWithoutSubNavOrTrailingAction = React.Children.toArray(children).filter(child =>
isValidElement(child) ? child.type !== SubNav && child.type !== TrailingAction : true,
)

if (!isValidElement(subNav) && defaultOpen)
Expand All @@ -78,7 +83,7 @@ const Item = React.forwardRef<HTMLAnchorElement, NavListItemProps>(
if (subNav && isValidElement(subNav)) {
return (
<ItemWithSubNav subNav={subNav} depth={depth} defaultOpen={defaultOpen} sx={sxProp}>
{childrenWithoutSubNav}
{childrenWithoutSubNavOrTrailingAction}
</ItemWithSubNav>
)
}
Expand Down Expand Up @@ -251,6 +256,14 @@ const Divider = ActionList.Divider

Divider.displayName = 'NavList.Divider'

// NavList.TrailingAction

export type NavListTrailingActionProps = ActionListTrailingActionProps

const TrailingAction = ActionList.TrailingAction

TrailingAction.displayName = 'NavList.TrailingAction'

// ----------------------------------------------------------------------------
// NavList.Group

Expand Down Expand Up @@ -285,6 +298,7 @@ export const NavList = Object.assign(Root, {
SubNav,
LeadingVisual,
TrailingVisual,
TrailingAction,
Divider,
Group,
})
19 changes: 19 additions & 0 deletions script/generate-e2e-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -800,6 +800,25 @@ const components = new Map([
],
},
],
[
'NavList',
{
stories: [
{
id: 'components-navlist--with-trailing-action',
name: 'With TrailingAction',
},
{
id: 'components-navlist--with-trailing-action-in-sub-item',
name: 'With TrailingAction in Sub Item',
},
{
id: 'components-navlist--with-bad-example-of-sub-nav-and-trailing-action',
name: 'With Bad Example of SubNav and TrailingAction',
},
],
},
],
[
'Pagehead',
{
Expand Down

0 comments on commit bf1b7ad

Please sign in to comment.