ActionList: Render ActionList.Item as button by default

src/ActionList/Description.tsx

@@ -27,7 +27,7 @@ export const Description: React.FC<React.PropsWithChildren<ActionListDescription
     minWidth: 0,
     marginLeft: variant === 'block' ? 0 : 2,
     color: 'fg.muted',
-    'li[aria-disabled="true"] &': {
+    'li > [aria-disabled="true"] &': {
       color: 'inherit',
     },
     'li[data-variant="danger"]:hover &, li[data-variant="danger"]:active &': {

src/ActionList/Item.tsx

@@ -29,11 +29,12 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
       id,
       role,
       _PrivateItemWrapper,
-      ...props
+      children,
+      ...rest
     },
     forwardedRef,
   ): JSX.Element => {
-    const [slots, childrenWithoutSlots] = useSlots(props.children, {
+    const [slots, childrenWithoutSlots] = useSlots(children, {
       leadingVisual: LeadingVisual,
       trailingVisual: TrailingVisual,
       blockDescription: [Description, props => props.variant === 'block'],
@@ -51,7 +52,7 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
 
     const onSelect = React.useCallback(
       (
-        event: React.MouseEvent<HTMLLIElement> | React.KeyboardEvent<HTMLLIElement>,
+        event: React.MouseEvent<HTMLButtonElement> | React.KeyboardEvent<HTMLButtonElement>,
         // eslint-disable-next-line @typescript-eslint/ban-types
         afterSelect?: Function,
       ) => {
@@ -93,6 +94,12 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
       },
     }
 
+    const listItemStyles = {
+      display: 'flex',
+      // show between 2 items
+      ':not(:first-of-type)': {'--divider-color': theme?.colors.actionListItem.inlineDivider},
+    }
+
     const styles = {
       position: 'relative',
       display: 'flex',
@@ -162,8 +169,6 @@ 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!
       '[data-component="ActionList.Divider"] + &': {'--divider-color': 'transparent !important'},
       // hide border on current and previous item
@@ -177,15 +182,15 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
     }
 
     const clickHandler = React.useCallback(
-      (event: React.MouseEvent<HTMLLIElement>) => {
+      (event: React.MouseEvent<HTMLButtonElement>) => {
         if (disabled) return
         onSelect(event, afterSelect)
       },
       [onSelect, disabled, afterSelect],
     )
 
     const keyPressHandler = React.useCallback(
-      (event: React.KeyboardEvent<HTMLLIElement>) => {
+      (event: React.KeyboardEvent<HTMLButtonElement>) => {
         if (disabled) return
         if ([' ', 'Enter'].includes(event.key)) {
           onSelect(event, afterSelect)
@@ -199,34 +204,46 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
     const inlineDescriptionId = `${itemId}--inline-description`
     const blockDescriptionId = `${itemId}--block-description`
 
-    const ItemWrapper = _PrivateItemWrapper || React.Fragment
+    type ActionListDefaultItemProps = ActionListItemProps & React.ButtonHTMLAttributes<HTMLButtonElement>
+
+    const DefaultItemWrapper = React.forwardRef(
+      ({sx = {}, as: Component = 'button', children, ...props}, forwardedRef) => {
+        return (
+          <Box as={Component} sx={merge<BetterSystemStyleObject>(styles, sx as SxProp)} ref={forwardedRef} {...props}>
+            {children}
+          </Box>
+        )
+      },
+    ) as PolymorphicForwardRefComponent<'button', ActionListDefaultItemProps>
 
-    const menuItemProps = {
+    const ItemWrapper = _PrivateItemWrapper ?? DefaultItemWrapper
+
+    const itemProps = {
       onClick: clickHandler,
       onKeyPress: keyPressHandler,
       'aria-disabled': disabled ? true : undefined,
+      // we need tabindex for only menu or listbox - because default is button and it is focusable by default
       tabIndex: disabled ? undefined : 0,
       'aria-labelledby': `${labelId} ${slots.inlineDescription ? inlineDescriptionId : ''}`,
       'aria-describedby': slots.blockDescription ? blockDescriptionId : undefined,
       ...(selectionAttribute && {[selectionAttribute]: selected}),
       role: role || itemRole,
       id: itemId,
+      styles,
     }
 
-    const containerProps = _PrivateItemWrapper ? {role: role || itemRole ? 'none' : undefined} : menuItemProps
-
-    const wrapperProps = _PrivateItemWrapper ? menuItemProps : {}
+    // This is on the li element. We don't need any props on the li, just need to undo the role for menu/listbox
+    const containerProps = {role: role || itemRole ? 'none' : undefined}
 
     return (
       <ItemContext.Provider value={{variant, disabled, inlineDescriptionId, blockDescriptionId}}>
         <LiBox
-          ref={forwardedRef}
-          sx={merge<BetterSystemStyleObject>(styles, sxProp)}
+          sx={merge<BetterSystemStyleObject>(listItemStyles, sxProp)}
           data-variant={variant === 'danger' ? variant : undefined}
           {...containerProps}
-          {...props}
         >
-          <ItemWrapper {...wrapperProps}>
+          {/* @ts-ignore I don't know what is the best type for event params */}
+          <ItemWrapper {...itemProps} ref={forwardedRef} {...rest}>
             <Selection selected={selected} />
             {slots.leadingVisual}
             <Box

src/ActionList/LinkItem.tsx

@@ -1,7 +1,7 @@
 import React from 'react'
 import {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic'
 import Link from '../Link'
-import {SxProp, merge} from '../sx'
+import {BetterSystemStyleObject, SxProp, merge} from '../sx'
 import {Item} from './Item'
 import {ActionListItemProps} from './shared'
 
@@ -22,32 +22,27 @@ type LinkProps = {
 export type ActionListLinkItemProps = Pick<ActionListItemProps, 'active' | 'children' | 'sx'> & LinkProps
 
 export const LinkItem = React.forwardRef(({sx = {}, active, as: Component, ...props}, forwardedRef) => {
-  const styles = {
-    // occupy full size of Item
-    paddingX: 2,
-    paddingY: '6px', // custom value off the scale
-    display: 'flex',
-    flexGrow: 1, // full width
-    borderRadius: 2,
-
-    // inherit Item styles
-    color: 'inherit',
-    '&:hover': {color: 'inherit', textDecoration: 'none'},
-  }
-
   return (
     <Item
       active={active}
       sx={{paddingY: 0, paddingX: 0}}
-      _PrivateItemWrapper={({children, onClick, ...rest}) => {
+      _PrivateItemWrapper={({children, styles, onClick, ...rest}) => {
         const clickHandler = (event: React.MouseEvent) => {
           onClick && onClick(event)
           props.onClick && props.onClick(event as React.MouseEvent<HTMLAnchorElement>)
         }
         return (
           <Link
             as={Component}
-            sx={merge(styles, sx as SxProp)}
+            sx={merge<BetterSystemStyleObject>(
+              {
+                ...styles, // shared style from Item
+                flexGrow: 1, // full width
+                color: 'inherit',
+                '&:hover': {color: 'inherit', textDecoration: 'none'},
+              },
+              sx as SxProp,
+            )}
             {...rest}
             {...props}
             onClick={clickHandler}

src/ActionList/shared.ts

@@ -1,5 +1,5 @@
 import React from 'react'
-import {SxProp} from '../sx'
+import {SxProp, BetterSystemStyleObject} from '../sx'
 import {AriaRole} from '../utils/types'
 
 export type ActionListItemProps = {
@@ -10,7 +10,7 @@ export type ActionListItemProps = {
   /**
    * Callback that will trigger both on click selection and keyboard selection.
    */
-  onSelect?: (event: React.MouseEvent<HTMLLIElement> | React.KeyboardEvent<HTMLLIElement>) => void
+  onSelect?: (event: React.MouseEvent<HTMLButtonElement> | React.KeyboardEvent<HTMLButtonElement>) => void
   /**
    * Is the `Item` is currently selected?
    */
@@ -41,17 +41,18 @@ export type ActionListItemProps = {
   /**
    * Private API for use internally only. Used by LinkItem to wrap contents in an anchor
    */
-  _PrivateItemWrapper?: React.FC<React.PropsWithChildren<MenuItemProps>>
+  _PrivateItemWrapper?: React.FC<React.PropsWithChildren<ItemWrapperProps>>
 } & SxProp
 
-type MenuItemProps = {
+export type ItemWrapperProps = {
   onClick?: (event: React.MouseEvent) => void
   onKeyPress?: (event: React.KeyboardEvent) => void
   'aria-disabled'?: boolean
   tabIndex?: number
   'aria-labelledby'?: string
   'aria-describedby'?: string
   role?: string
+  styles?: BetterSystemStyleObject
 }
 
 export type ItemContext = Pick<ActionListItemProps, 'variant' | 'disabled'> & {

src/ActionMenu/ActionMenu.examples.stories.tsx

@@ -279,7 +279,7 @@ export const MultipleSections = () => {
 export const DelayedMenuClose = () => {
   const [open, setOpen] = React.useState(false)
   const [copyLinkSuccess, setCopyLinkSuccess] = React.useState(false)
-  const onSelect = (event: React.MouseEvent<HTMLLIElement> | React.KeyboardEvent<HTMLLIElement>) => {
+  const onSelect = (event: React.MouseEvent<HTMLElement> | React.KeyboardEvent<HTMLElement>) => {
     event.preventDefault()
 
     setCopyLinkSuccess(true)

src/NavList/NavList.tsx

@@ -134,37 +134,35 @@ function ItemWithSubNav({children, subNav, depth, defaultOpen, sx: sxProp = defa
 
   return (
     <ItemWithSubNavContext.Provider value={{buttonId, subNavId, isOpen}}>
-      <Box as="li" aria-labelledby={buttonId} sx={{listStyle: 'none'}}>
-        <ActionList.Item
-          as="button"
-          id={buttonId}
-          aria-expanded={isOpen}
-          aria-controls={subNavId}
-          // When the subNav is closed, how should we indicated that the subNav contains the current item?
-          active={!isOpen && containsCurrentItem}
-          onClick={() => setIsOpen(open => !open)}
-          sx={merge<SxProp['sx']>(
-            {
-              ...getSubnavStyles(depth),
-              fontWeight: containsCurrentItem ? 'bold' : null, // Parent item is bold if any of it's sub-items are current
-            },
-            sxProp,
-          )}
-        >
-          {children}
-          {/* What happens if the user provides a TrailingVisual? */}
-          <ActionList.TrailingVisual>
-            <Octicon
-              icon={ChevronDownIcon}
-              sx={{
-                transform: isOpen ? 'rotate(180deg)' : 'rotate(0deg)',
-              }}
-            />
-          </ActionList.TrailingVisual>
-        </ActionList.Item>
-
-        <div ref={subNavRef}>{subNav}</div>
-      </Box>
+      <ActionList.Item
+        as="button"
+        id={buttonId}
+        aria-expanded={isOpen}
+        aria-controls={subNavId}
+        // When the subNav is closed, how should we indicated that the subNav contains the current item?
+        active={!isOpen && containsCurrentItem}
+        onClick={() => setIsOpen(open => !open)}
+        sx={merge<SxProp['sx']>(
+          {
+            ...getSubnavStyles(depth),
+            fontWeight: containsCurrentItem ? 'bold' : null, // Parent item is bold if any of it's sub-items are current
+          },
+          sxProp,
+        )}
+      >
+        {children}
+        {/* What happens if the user provides a TrailingVisual? */}
+        <ActionList.TrailingVisual>
+          <Octicon
+            icon={ChevronDownIcon}
+            sx={{
+              transform: isOpen ? 'rotate(180deg)' : 'rotate(0deg)',
+            }}
+          />
+        </ActionList.TrailingVisual>
+      </ActionList.Item>
+
+      <div ref={subNavRef}>{subNav}</div>
     </ItemWithSubNavContext.Provider>
   )
 }

src/SegmentedControl/SegmentedControl.tsx

@@ -125,10 +125,10 @@ const Root: React.FC<React.PropsWithChildren<SegmentedControlProps>> = ({
                 <ActionList.Item
                   key={`segmented-control-action-btn-${index}`}
                   selected={index === selectedIndex}
-                  onSelect={(event: React.MouseEvent<HTMLLIElement> | React.KeyboardEvent<HTMLLIElement>) => {
+                  onSelect={(event: React.MouseEvent<HTMLButtonElement> | React.KeyboardEvent<HTMLButtonElement>) => {
                     isUncontrolled && setSelectedIndexInternalState(index)
                     onChange && onChange(index)
-                    child.props.onClick && child.props.onClick(event as React.MouseEvent<HTMLLIElement>)
+                    child.props.onClick && child.props.onClick(event as React.MouseEvent<HTMLButtonElement>)
                   }}
                 >
                   {ChildIcon && <ChildIcon />} {getChildText(child)}