Introduce TrailingAction to ActionList (#4634)

* Add fixes for ActionList

* More type fixes

* temp type fixes

* Add condition for inactive items

* add yet another condition

* chore(deps): bump changesets/action from 1.4.5 to 1.4.6 (#4282)

Bumps [changesets/action](https://github.com/changesets/action) from 1.4.5 to 1.4.6.
- [Release notes](https://github.com/changesets/action/releases)
- [Changelog](https://github.com/changesets/action/blob/main/CHANGELOG.md)
- [Commits](changesets/action@f13b1ba...e2f8e96)

---
updated-dependencies:
- dependency-name: changesets/action
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* test(e2e): add e2e test for SelectPanel2 default story (#4279)

* fix(SelectPanel2): add aria-labelledby to listbox

* test(e2e): add e2e test for SelectPanel2 default story

* chore: add changeset

* test(vrt): update snapshots

---------

Co-authored-by: Josh Black <joshblack@users.noreply.github.com>

* Address a few v8 color bugs (#4278)

* small bug fixes with v8

* Create khaki-schools-lay.md

* test(vrt): update snapshots

* snippy snaps

* test(vrt): update snapshots

* test: update snapshots

* test: update snapshots

* try commenting flakey tests

* test: comment out flakey snapshot test

---------

Co-authored-by: langermank <langermank@users.noreply.github.com>
Co-authored-by: Josh Black <joshblack@users.noreply.github.com>

* chore(deps-dev): bump ip from 2.0.0 to 2.0.1 (#4291)

Bumps [ip](https://github.com/indutny/node-ip) from 2.0.0 to 2.0.1.
- [Commits](indutny/node-ip@v2.0.0...v2.0.1)

---
updated-dependencies:
- dependency-name: ip
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Change how `ref` is handled

* Change types

* Update storybook example types

* Update types on component

* Add another type

* Update type in `SegmentedControl`

* Add back `li`-only type

* Add onto `onSelect` type

* Update more types

* Type fixes for `LinkItem`

* Changes from feedback

* Change types

* Replace `role="list"` with context

* Add feature flag to `ActionList.Item

* Add back forwardedRef in cases where valid role is true

* Update FF name

* Add lint disable

* Update FF name

* Add changeset

* Remove `list` condition

* Rename FF

* Address feedback

* Add feature flag story

* Add new test

* Add export

* Updated forwardRef to be polymorphic

* Spike: Add trailingAction to ActionList.Item

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

* Updated forwardRef to be polymorphic

* TrailingAction style adjustments

* test(vrt): update snapshots

* Remove unused story

* Limit TrailingAction props

* Fix missing href for as link

* Add Inactive example

* Add styles for `showOnHover`

* Fixed href overload issue

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

* Made sure that TrailingAction doesn't show when inactive state

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

* test(vrt): update snapshots

* Add fixes for button styles

* Fix for flex styles

* Update storybookd descriptions

* Add block description example

* Add tests

* Remove FF from tests

* update e2e stories

* spike on allowing text (#4659)

* spike on allowing text

* showOnHover support

* Add overflow example

* Add visual tests

* Update from display to visibility

* Create calm-crabs-raise.md

* Disallow usage in `ActionMenu`

* Update ActionList.test.tsx

* Remove hover/focus styles

* test(vrt): update snapshots

* Apply suggestions from code review

* Apply suggestions from code review

* Fix merge conflict

* Remove Hover examples

* Remove showOnHover code

* Add FF const back

* Remove some usage of `hoverStyles`

* test(vrt): update snapshots

* Add props to TrailingAction

* update types

 Co-authored-by: Josh Black <joshblack@users.noreply.github.com>

* update docs

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Tyler Jones <tylerjdev@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Josh Black <joshblack@github.com>
Co-authored-by: Josh Black <joshblack@users.noreply.github.com>
Co-authored-by: Katie Langerman <18661030+langermank@users.noreply.github.com>
Co-authored-by: langermank <langermank@users.noreply.github.com>
Co-authored-by: khiga8 <khiga8@users.noreply.github.com>
Co-authored-by: TylerJDev <TylerJDev@users.noreply.github.com>

.changeset/calm-crabs-raise.md

@@ -0,0 +1,5 @@
+---
+"@primer/react": minor
+---
+
+Introduce ActionList.TrailingAction to support secondary action on ActionList.Item

e2e/components/ActionList.test.ts

@@ -626,4 +626,32 @@ test.describe('ActionList', () => {
       })
     }
   })
+
+  test.describe('With Trailing Action', () => {
+    for (const theme of themes) {
+      test.describe(theme, () => {
+        test('default @vrt', async ({page}) => {
+          await visit(page, {
+            id: 'components-actionlist-features--with-trailing-action',
+            globals: {
+              colorScheme: theme,
+            },
+          })
+
+          // Default state
+          expect(await page.screenshot()).toMatchSnapshot(`ActionList.With Trailing Action.${theme}.png`)
+        })
+
+        test('axe @aat', async ({page}) => {
+          await visit(page, {
+            id: 'components-actionlist-features--with-trailing-action',
+            globals: {
+              colorScheme: theme,
+            },
+          })
+          await expect(page).toHaveNoViolations()
+        })
+      })
+    }
+  })
 })

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

@@ -191,6 +191,37 @@
         }
       ]
     },
+    {
+      "name": "ActionList.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": "ref",
+          "type": "React.RefObject<HTMLAnchorElement>",
+          "description": "href when the TrailingAction is rendered as a link."
+        }
+      ]
+    },
     {
       "name": "ActionList.Description",
       "props": [

packages/react/src/ActionList/ActionList.features.stories.tsx

@@ -734,3 +734,61 @@ export const ActionListWithButtonSemantics = () => {
 }
 
 ActionListWithButtonSemantics.storyName = 'With Button Semantics (Behind feature flag)'
+
+export const WithTrailingAction = () => {
+  return (
+    <FeatureFlags flags={{primer_react_action_list_item_as_button: true}}>
+      <ActionList>
+        <ActionList.Item>
+          <ActionList.LeadingVisual>
+            <FileDirectoryIcon />
+          </ActionList.LeadingVisual>
+          Item 1 (with default TrailingAction)
+          <ActionList.TrailingAction label="Expand sidebar" icon={ArrowLeftIcon} />
+        </ActionList.Item>
+        <ActionList.Item>
+          Item 2 (with link TrailingAction)
+          <ActionList.TrailingAction as="a" href="#" label="Some action 1" icon={ArrowRightIcon} />
+        </ActionList.Item>
+        <ActionList.Item>
+          Item 3<ActionList.Description>This is an inline description.</ActionList.Description>
+          <ActionList.TrailingAction label="Some action 2" icon={BookIcon} />
+        </ActionList.Item>
+        <ActionList.Item>
+          Item 4<ActionList.Description variant="block">This is a block description.</ActionList.Description>
+          <ActionList.TrailingAction label="Some action 3" icon={BookIcon} />
+        </ActionList.Item>
+        <ActionList.Item>
+          Item 5<ActionList.Description variant="block">This is a block description.</ActionList.Description>
+          <ActionList.TrailingAction label="Some action 4" />
+        </ActionList.Item>
+        <ActionList.Item>
+          Item 6
+          <ActionList.TrailingAction href="#" as="a" label="Some action 5" />
+        </ActionList.Item>
+        <ActionList.LinkItem href="#">
+          LinkItem 1
+          <ActionList.Description>
+            with TrailingAction this is a long description and should not cause horizontal scroll on smaller screen
+            sizes
+          </ActionList.Description>
+          <ActionList.TrailingAction label="Another action" />
+        </ActionList.LinkItem>
+        <ActionList.LinkItem href="#">
+          LinkItem 2
+          <ActionList.Description>
+            with TrailingVisual this is a long description and should not cause horizontal scroll on smaller screen
+            sizes
+          </ActionList.Description>
+          <ActionList.TrailingVisual>
+            <TableIcon />
+          </ActionList.TrailingVisual>
+        </ActionList.LinkItem>
+        <ActionList.Item inactiveText="Unavailable due to an outage">
+          Inactive Item<ActionList.Description>With TrailingAction</ActionList.Description>
+          <ActionList.TrailingAction as="a" href="#" label="Some action 8" icon={ArrowRightIcon} />
+        </ActionList.Item>
+      </ActionList>
+    </FeatureFlags>
+  )
+}

packages/react/src/ActionList/ActionList.test.tsx

@@ -4,6 +4,7 @@ import axe from 'axe-core'
 import React from 'react'
 import theme from '../theme'
 import {ActionList} from '.'
+import {BookIcon} from '@primer/octicons-react'
 import {behavesAsComponent, checkExports} from '../utils/testing'
 import {BaseStyles, ThemeProvider, SSRProvider, ActionMenu} from '..'
 import {FeatureFlags} from '../FeatureFlags'
@@ -445,4 +446,64 @@ describe('ActionList', () => {
     const listItems = container.querySelectorAll('li')
     expect(listItems.length).toBe(2)
   })
+
+  it('should render the trailing action as a button (default)', async () => {
+    const {container} = HTMLRender(
+      <ActionList>
+        <ActionList.Item>
+          Item 1
+          <ActionList.TrailingAction icon={BookIcon} label="Action" />
+        </ActionList.Item>
+      </ActionList>,
+    )
+
+    const action = container.querySelector('button[aria-labelledby]')
+    expect(action).toHaveAccessibleName('Action')
+  })
+
+  it('should render the trailing action as a link', async () => {
+    const {container} = HTMLRender(
+      <ActionList>
+        <ActionList.Item>
+          Item 1
+          <ActionList.TrailingAction as="a" href="#" icon={BookIcon} label="Action" />
+        </ActionList.Item>
+      </ActionList>,
+    )
+
+    const action = container.querySelector('a[href="#"][aria-labelledby]')
+    expect(action).toHaveAccessibleName('Action')
+  })
+
+  it('should do action when trailing action is clicked', async () => {
+    const onClick = jest.fn()
+    const component = HTMLRender(
+      <ActionList>
+        <ActionList.Item>
+          Item 1
+          <ActionList.TrailingAction icon={BookIcon} label="Action" onClick={onClick} />
+        </ActionList.Item>
+      </ActionList>,
+    )
+
+    const trailingAction = await waitFor(() => component.getByRole('button', {name: 'Action'}))
+    fireEvent.click(trailingAction)
+    expect(onClick).toHaveBeenCalled()
+  })
+
+  it('should focus the trailing action', async () => {
+    HTMLRender(
+      <ActionList>
+        <ActionList.Item>
+          Item 1
+          <ActionList.TrailingAction icon={BookIcon} label="Action" />
+        </ActionList.Item>
+      </ActionList>,
+    )
+
+    await userEvent.tab()
+    expect(document.activeElement).toHaveTextContent('Item 1')
+    await userEvent.tab()
+    expect(document.activeElement).toHaveAccessibleName('Action')
+  })
 })

packages/react/src/ActionList/Item.tsx

@@ -20,7 +20,9 @@ import {Selection} from './Selection'
 import {getVariantStyles, ItemContext, TEXT_ROW_HEIGHT, ListContext} from './shared'
 import type {VisualProps} from './Visuals'
 import {LeadingVisual, TrailingVisual} from './Visuals'
+import {TrailingAction} from './TrailingAction'
 import {ConditionalWrapper} from '../internal/components/ConditionalWrapper'
+import {invariant} from '../utils/invariant'
 import {useFeatureFlag} from '../FeatureFlags'
 
 const LiBox = styled.li<SxProp>(sx)
@@ -71,14 +73,15 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
     const [slots, childrenWithoutSlots] = useSlots(props.children, {
       leadingVisual: LeadingVisual,
       trailingVisual: TrailingVisual,
+      trailingAction: TrailingAction,
       blockDescription: [Description, props => props.variant === 'block'],
       inlineDescription: [Description, props => props.variant !== 'block'],
     })
 
     const {container, afterSelect, selectionAttribute, defaultTrailingVisual} =
       React.useContext(ActionListContainerContext)
 
-    const buttonSemantics = useFeatureFlag('primer_react_action_list_item_as_button')
+    const buttonSemanticsFeatureFlag = useFeatureFlag('primer_react_action_list_item_as_button')
 
     // Be sure to avoid rendering the container unless there is a default
     const wrappedDefaultTrailingVisual = defaultTrailingVisual ? (
@@ -125,12 +128,19 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
 
     const itemRole = role || inferredItemRole
 
+    if (slots.trailingAction) {
+      invariant(!container, `ActionList.TrailingAction can not be used within a ${container}.`)
+    }
+
     /** Infer the proper selection attribute based on the item's role */
     let inferredSelectionAttribute: 'aria-selected' | 'aria-checked' | undefined
     if (itemRole === 'menuitemradio' || itemRole === 'menuitemcheckbox') inferredSelectionAttribute = 'aria-checked'
     else if (itemRole === 'option') inferredSelectionAttribute = 'aria-selected'
 
     const itemSelectionAttribute = selectionAttribute || inferredSelectionAttribute
+    // Ensures ActionList.Item retains list item semantics if a valid ARIA role is applied, or if item is inactive
+    const listSemantics = listRole === 'listbox' || listRole === 'menu' || inactive || container === 'NavList'
+    const buttonSemantics = !listSemantics && !_PrivateItemWrapper && buttonSemanticsFeatureFlag
 
     const {theme} = useTheme()
 
@@ -149,10 +159,32 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
       },
     }
 
+    const hoverStyles = {
+      '@media (hover: hover) and (pointer: fine)': {
+        ':hover:not([aria-disabled]):not([data-inactive])': {
+          backgroundColor: `actionListItem.${variant}.hoverBg`,
+          color: getVariantStyles(variant, disabled, inactive).hoverColor,
+          boxShadow: `inset 0 0 0 max(1px, 0.0625rem) ${theme?.colors.actionListItem.default.activeBorder}`,
+        },
+        '&:focus-visible, > a.focus-visible, &:focus.focus-visible': {
+          outline: 'none',
+          border: `2 solid`,
+          boxShadow: `0 0 0 2px ${theme?.colors.accent.emphasis}`,
+        },
+        ':active:not([aria-disabled]):not([data-inactive])': {
+          backgroundColor: `actionListItem.${variant}.activeBg`,
+          color: getVariantStyles(variant, disabled, inactive).hoverColor,
+        },
+      },
+    }
+
     const listItemStyles = {
       display: 'flex',
       // show between 2 items
       ':not(:first-of-type)': {'--divider-color': theme?.colors.actionListItem.inlineDivider},
+      width: 'calc(100% - 16px)',
+      marginX: buttonSemantics ? '2' : '0',
+      ...(buttonSemantics ? hoverStyles : {}),
     }
 
     const styles = {
@@ -163,7 +195,7 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
       paddingY: '6px', // custom value off the scale
       lineHeight: TEXT_ROW_HEIGHT,
       minHeight: 5,
-      marginX: listVariant === 'inset' ? 2 : 0,
+      marginX: listVariant === 'inset' && !buttonSemantics ? 2 : 0,
       borderRadius: 2,
       transition: 'background 33.333ms linear',
       color: getVariantStyles(variant, disabled, inactive).color,
@@ -181,7 +213,7 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
       appearance: 'none',
       background: 'unset',
       border: 'unset',
-      width: listVariant === 'inset' ? 'calc(100% - 16px)' : '100%',
+      width: listVariant === 'inset' && !buttonSemantics ? 'calc(100% - 16px)' : '100%',
       fontFamily: 'unset',
       textAlign: 'unset',
       marginY: 'unset',
@@ -224,6 +256,7 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
         borderTopWidth: showDividers ? `1px` : '0',
         borderColor: 'var(--divider-color, transparent)',
       },
+
       // show between 2 items
       ':not(:first-of-type)': {'--divider-color': theme?.colors.actionListItem.inlineDivider},
       // hide divider after dividers & group header, with higher importance!
@@ -268,8 +301,6 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
     const inlineDescriptionId = `${itemId}--inline-description`
     const blockDescriptionId = `${itemId}--block-description`
     const inactiveWarningId = inactive && !showInactiveIndicator ? `${itemId}--warning-message` : undefined
-    // Ensures ActionList.Item retains list item semantics if a valid ARIA role is applied, or if item is inactive
-    const listSemantics = listRole === 'listbox' || listRole === 'menu' || inactive || container === 'NavList'
 
     const ButtonItemWrapper = React.forwardRef(({as: Component = 'button', children, ...props}, forwardedRef) => {
       return (
@@ -285,7 +316,7 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
     }) as PolymorphicForwardRefComponent<React.ElementType, ActionListItemProps>
 
     let DefaultItemWrapper = React.Fragment
-    if (buttonSemantics) {
+    if (buttonSemanticsFeatureFlag) {
       DefaultItemWrapper = listSemantics ? React.Fragment : ButtonItemWrapper
     }
 
@@ -313,7 +344,7 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
     let containerProps
     let wrapperProps
 
-    if (buttonSemantics) {
+    if (buttonSemanticsFeatureFlag) {
       containerProps = _PrivateItemWrapper
         ? {role: itemRole ? 'none' : undefined, ...props}
         : // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
@@ -337,9 +368,9 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
         value={{variant, disabled, inactive: Boolean(inactiveText), inlineDescriptionId, blockDescriptionId}}
       >
         <LiBox
-          ref={buttonSemantics || listSemantics ? forwardedRef : null}
+          ref={buttonSemanticsFeatureFlag || listSemantics ? forwardedRef : null}
           sx={
-            buttonSemantics
+            buttonSemanticsFeatureFlag
               ? merge<BetterSystemStyleObject>(
                   listSemantics || _PrivateItemWrapper ? styles : listItemStyles,
                   listSemantics || _PrivateItemWrapper ? sxProp : {},
@@ -424,6 +455,7 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
               {slots.blockDescription}
             </Box>
           </ItemWrapper>
+          {!inactive && Boolean(slots.trailingAction) && !container && slots.trailingAction}
         </LiBox>
       </ItemContext.Provider>
     )