Refactor(ActionList): ActionList.Item should render content as a button if parent is not interactive.

.changeset/purple-crabs-matter.md

@@ -0,0 +1,5 @@
+---
+"@primer/react": patch
+---
+
+Refactor(ActionList): ActionList.Item should render content as a button if parent is not interactive.

src/ActionList/Description.tsx

@@ -44,7 +44,6 @@ export const Description: React.FC<React.PropsWithChildren<ActionListDescription
  sx={merge({...styles, color: disabled ? 'fg.disabled' : 'fg.muted'}, sx as SxProp)}
  title={props.children as string}
  inline={true}
- maxWidth="100%"
  >
  {props.children}
  </Truncate>

src/ActionList/Item.tsx

@@ -9,10 +9,11 @@ import {defaultSxProp} from '../utils/defaultSxProp'
 import {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic'
 import {ActionListContainerContext} from './ActionListContainerContext'
 import {Description} from './Description'
-import {ActionListProps, ListContext} from './List'
+import {ListContext} from './List'
 import {Selection} from './Selection'
 import {ActionListItemProps, getVariantStyles, ItemContext, TEXT_ROW_HEIGHT} from './shared'
 import {LeadingVisual, TrailingVisual} from './Visuals'
+import {MenuContext} from '../ActionMenu/ActionMenu'
 import {GroupContext} from './Group'
 
 const LiBox = styled.li<SxProp>(sx)
@@ -29,6 +30,8 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
  id,
  role,
  _PrivateItemWrapper,
+ // @ts-ignore tabIndex is sometimes passed as a prop in dotcom.
+ tabIndex,
  ...props
  },
  forwardedRef,
@@ -38,10 +41,13 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
  trailingVisual: TrailingVisual,
  description: Description,
  })
+
  const {variant: listVariant, showDividers, selectionVariant: listSelectionVariant} = React.useContext(ListContext)
  const {container, afterSelect, selectionAttribute} = React.useContext(ActionListContainerContext)
+ const menuContext = React.useContext(MenuContext)
  const {selectionVariant: groupSelectionVariant} = React.useContext(GroupContext)
 
+ const selectionVariant = groupSelectionVariant ?? listSelectionVariant
  const onSelect = React.useCallback(
  (
  event: React.MouseEvent<HTMLLIElement> | React.KeyboardEvent<HTMLLIElement>,
@@ -55,10 +61,6 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
  [onSelectUser],
  )
 
- const selectionVariant: ActionListProps['selectionVariant'] = groupSelectionVariant
- ? groupSelectionVariant
- : listSelectionVariant
-
  /** Infer item role based on the container */
  let itemRole: ActionListItemProps['role']
  if (container === 'ActionMenu' || container === 'DropdownMenu') {
@@ -84,12 +86,22 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
  },
  }
 
+ const isTopLevelInteractive = () =>
+ _PrivateItemWrapper !== undefined ||
+ // @ts-ignore props.as may be defined, may not.
+ props.as === 'button' ||
+ // @ts-ignore props.as may be defined, may not.
+ props.as === 'a' ||
+ menuContext.anchorId !== undefined ||
+ role?.match(/menuitem/) ||
+ tabIndex !== undefined
+
  const styles = {
  position: 'relative',
  display: 'flex',
- paddingX: 2,
+ paddingX: isTopLevelInteractive() ? 2 : 0,
  fontSize: 1,
- paddingY: '6px', // custom value off the scale
+ paddingY: isTopLevelInteractive() ? '6px' : 0, // custom value off the scale
  lineHeight: TEXT_ROW_HEIGHT,
  minHeight: 5,
  marginX: listVariant === 'inset' ? 2 : 0,
@@ -145,6 +157,10 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
  borderTopWidth: showDividers ? `1px` : '0',
  borderColor: 'var(--divider-color, transparent)',
  },
+ 'button[data-component="ActionList.Item--DividerContainer"]': {
+ textAlign: 'left',
+ padding: 0,
+ },
  // show between 2 items
  ':not(:first-of-type)': {'--divider-color': theme?.colors.actionListItem.inlineDivider},
  // hide divider after dividers & group header, with higher importance!
@@ -182,13 +198,13 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
  const inlineDescriptionId = useId(id && `${id}--inline-description`)
  const blockDescriptionId = useId(id && `${id}--block-description`)
 
- const ItemWrapper = _PrivateItemWrapper || React.Fragment
+ const ItemWrapper = _PrivateItemWrapper || Box
 
  const menuItemProps = {
  onClick: clickHandler,
  onKeyPress: keyPressHandler,
  'aria-disabled': disabled ? true : undefined,
- tabIndex: disabled ? undefined : 0,
+ tabIndex: disabled || !isTopLevelInteractive() ? undefined : 0,
  'aria-labelledby': `${labelId} ${
  slots.description && slots.description.props.variant !== 'block' ? inlineDescriptionId : ''
  }`,
@@ -199,17 +215,41 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
 
  const containerProps = _PrivateItemWrapper ? {role: role || itemRole ? 'none' : undefined} : menuItemProps
 
- const wrapperProps = _PrivateItemWrapper ? menuItemProps : {}
+ const wrapperProps = _PrivateItemWrapper
+ ? menuItemProps
+ : {
+ sx: {
+ display: 'flex',
+ paddingX: isTopLevelInteractive() ? 0 : 2,
+ paddingY: isTopLevelInteractive() ? 0 : '6px', // custom value off the scale
+ flexGrow: 1,
+ },
+ }
 
  return (
  <ItemContext.Provider value={{variant, disabled, inlineDescriptionId, blockDescriptionId}}>
  <LiBox ref={forwardedRef} sx={merge<BetterSystemStyleObject>(styles, sxProp)} {...containerProps} {...props}>
+ {/* @ts-ignore onClick prop is only passed when _PrivateItemWrapper is set by ActionList.LinkItem. */}
  <ItemWrapper {...wrapperProps}>
  <Selection selected={selected} />
  {slots.leadingVisual}
  <Box
  data-component="ActionList.Item--DividerContainer"
- sx={{display: 'flex', flexDirection: 'column', flexGrow: 1, minWidth: 0}}
+ sx={{
+ display: 'flex',
+ flexDirection: 'column',
+ flexGrow: 1,
+ minWidth: 0,
+ borderStyle: 'none',
+ backgroundColor: 'transparent',
+ cursor: 'inherit',
+ fontSize: 'inherit',
+ color: getVariantStyles(variant, disabled).color,
+ lineHeight: '20px',
+ }}
+ // @ts-ignore `as` prop may be passed to ActionList.Item, even if it isn't defined in ActionListItemProps.
+ // If this item is inside an ActionMenu, don't render an interactive button.
+ as={isTopLevelInteractive() ? 'div' : 'button'}
  >
  <ConditionalBox if={Boolean(slots.trailingVisual)} sx={{display: 'flex', flexGrow: 1}}>
  <ConditionalBox

src/__tests__/ActionMenu.test.tsx → src/ActionMenu/ActionMenu.test.tsx

@@ -3,7 +3,7 @@ import userEvent from '@testing-library/user-event'
 import {axe, toHaveNoViolations} from 'jest-axe'
 import React from 'react'
 import theme from '../theme'
-import {ActionMenu, ActionList, BaseStyles, ThemeProvider, SSRProvider} from '..'
+import {ActionMenu, MenuContext, ActionList, BaseStyles, ThemeProvider, SSRProvider} from '..'
 import {behavesAsComponent, checkExports} from '../utils/testing'
 import {SingleSelect} from '../ActionMenu/ActionMenu.features.stories'
 import {MixedSelection} from '../ActionMenu/ActionMenu.examples.stories'
@@ -47,6 +47,7 @@ describe('ActionMenu', () => {
  checkExports('ActionMenu', {
  default: undefined,
  ActionMenu,
+ MenuContext,
  })
 
  it('should open Menu on MenuButton click', async () => {

src/ActionMenu/ActionMenu.tsx

@@ -16,7 +16,8 @@ export type MenuContextProps = Pick<
 > & {
  onClose?: (gesture: 'anchor-click' | 'click-outside' | 'escape' | 'tab') => void
 }
-const MenuContext = React.createContext<MenuContextProps>({renderAnchor: null, open: false})
+
+export const MenuContext = React.createContext<MenuContextProps>({renderAnchor: null, open: false})
 
 export type ActionMenuProps = {
  /**

src/NavList/NavList.test.tsx

@@ -211,7 +211,7 @@ describe('NavList.Item with NavList.SubNav', () => {
  </ThemeProvider>,
  )
 
- const button = getByRole('button')
+ const button = getByRole('button', {name: 'Item'})
 
  // Starts open
  expect(queryByRole('list', {name: 'Item'})).toBeVisible()