Add TrailingAction support to NavList (#4697)

* Update NavList to support TrailingAction

* Update conditional in ActionList to allow NavList to set TrailingAction

* add NavList.TrailingAction support

* Update logic to not render for ActionMenu or SelectPanel

Co-authored-by: Tyler Jones <tylerjdev@github.com>

* Create tender-parrots-refuse.md

* Fix syntax

* Wrap example with PageLayout

* Add subitem

* Filter out TrailingAction

* Add test coverage

* Move back to minimize diff

* Add bad storybook example

* Update Story name

* Update e2e tests

* Update stories

* test(vrt): update snapshots

* update docs

* Update packages/react/src/NavList/NavList.docs.json

Co-authored-by: Tyler Jones <tylerjdev@github.com>

* Fix description:

* Move to DevOnly

* Change to minor

* Update snapshots

---------

Co-authored-by: Tyler Jones <tylerjdev@github.com>
Co-authored-by: khiga8 <khiga8@users.noreply.github.com>

.changeset/tender-parrots-refuse.md

@@ -0,0 +1,5 @@
+---
+"@primer/react": minor
+---
+
+Add TrailingAction support to NavList

e2e/components/NavList.test.ts

@@ -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-devonly--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-devonly--with-bad-example-of-sub-nav-and-trailing-action',
+            globals: {
+              colorScheme: theme,
+            },
+          })
+          await expect(page).toHaveNoViolations()
+        })
+      })
+    }
+  })
+})

packages/react/src/ActionList/ActionList.docs.json

@@ -216,8 +216,8 @@
           "description": "Octicon to pass into IconButton. When this is not set, TrailingAction renders as a `Button` instead of an `IconButton`."
         },
         {
-          "name": "ref",
-          "type": "React.RefObject<HTMLAnchorElement>",
+          "name": "href",
+          "type": "string",
           "description": "href when the TrailingAction is rendered as a link."
         }
       ]

packages/react/src/ActionList/Item.tsx

@@ -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 */
@@ -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>
     )

packages/react/src/ActionList/index.ts

@@ -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.

packages/react/src/NavList/NavList.dev.stories.tsx

@@ -0,0 +1,49 @@
+import type {Meta} 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/DevOnly',
+  component: NavList,
+  parameters: {
+    layout: 'fullscreen',
+  },
+}
+
+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

packages/react/src/NavList/NavList.docs.json

@@ -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."
+        }
+      ]
     }
   ]
 }

packages/react/src/NavList/NavList.stories.tsx

@@ -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',
@@ -246,4 +247,57 @@ 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 default meta

packages/react/src/NavList/NavList.test.tsx

@@ -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}
 
@@ -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', () => {
@@ -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()
+    })
+  })
 })

packages/react/src/NavList/NavList.tsx

@@ -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'
@@ -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)
@@ -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>
       )
     }
@@ -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
 
@@ -285,6 +298,7 @@ export const NavList = Object.assign(Root, {
   SubNav,
   LeadingVisual,
   TrailingVisual,
+  TrailingAction,
   Divider,
   Group,
 })