Merge ec25af5 into e957fef

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

packages/react/src/ActionList/TrailingAction.tsx

@@ -0,0 +1,55 @@
+import React, {forwardRef} from 'react'
+import Box from '../Box'
+import {Button, IconButton} from '../Button'
+import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic'
+
+export type ActionListTrailingActionProps = {
+  as?: 'button' | 'a'
+  href?: string
+  icon?: React.ElementType
+  label: string
+}
+
+export const TrailingAction = forwardRef(({as = 'button', icon, label, href = null, ...props}, forwardedRef) => {
+  if (!icon) {
+    return (
+      <Box
+        data-component="ActionList.TrailingAction"
+        as="span"
+        sx={{
+          flexShrink: 0,
+        }}
+      >
+        {/* @ts-expect-error TODO: Fix this */}
+        <Button variant="invisible" as={as} href={href} ref={forwardedRef} {...props}>
+          {label}
+        </Button>
+      </Box>
+    )
+  } else {
+    return (
+      <Box
+        as="span"
+        data-component="ActionList.TrailingAction"
+        sx={{
+          flexShrink: 0,
+        }}
+      >
+        <IconButton
+          as={as}
+          aria-label={label}
+          icon={icon}
+          variant="invisible"
+          unsafeDisableTooltip={false}
+          tooltipDirection="w"
+          href={href}
+          // @ts-expect-error StyledButton wants both Anchor and Button refs
+          ref={forwardedRef}
+          {...props}
+        />
+      </Box>
+    )
+  }
+}) as PolymorphicForwardRefComponent<'button' | 'a', ActionListTrailingActionProps>
+
+TrailingAction.displayName = 'ActionList.TrailingAction'

packages/react/src/ActionList/index.ts

@@ -4,6 +4,7 @@ import {Item} from './Item'
 import {LinkItem} from './LinkItem'
 import {Divider} from './Divider'
 import {Description} from './Description'
+import {TrailingAction} from './TrailingAction'
 import {LeadingVisual, TrailingVisual} from './Visuals'
 import {Heading} from './Heading'
 
@@ -46,4 +47,7 @@ export const ActionList = Object.assign(List, {
 
   /** Heading for `ActionList.Group` */
   GroupHeading,
+
+  /** Secondary action */
+  TrailingAction,
 })