feat(react-menu): replace keydown handlers by useARIAButtonShorthand on MenuItem

apps/public-docsite-v9/src/shims/MenuShim.tsx

@@ -15,6 +15,7 @@ import {
   MenuItemCheckbox,
   MenuItemCheckboxProps,
   MenuProps,
+  MenuDividerProps,
 } from '@fluentui/react-components';
 
 export const shimMenuProps = (props: IContextualMenuProps): Partial<MenuProps> => {
@@ -54,7 +55,7 @@ const shimMenuHeaderProps = (props: IContextualMenuItem): MenuGroupHeaderProps =
 export const MenuItemShim = (props: IContextualMenuItem) => {
   if (props.itemType === ContextualMenuItemType.Divider) {
     const shimProps = shimMenuItemProps(props);
-    return <MenuDivider {...shimProps} />;
+    return <MenuDivider {...(shimProps as MenuDividerProps)} />;
   }
 
   if (props.itemType === ContextualMenuItemType.Section) {

change/@fluentui-react-menu-c4342a62-8136-40bb-83a5-5269a00d5b85.json

@@ -0,0 +1,7 @@
+{
+  "type": "minor",
+  "comment": "feat: replace keydown handlers by useARIAButtonShorthand on MenuItem",
+  "packageName": "@fluentui/react-menu",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}

packages/react-components/react-menu/etc/react-menu.api.md

@@ -6,7 +6,9 @@
 
 /// <reference types="react" />
 
+import type { ARIAButtonElement } from '@fluentui/react-aria';
 import { ARIAButtonResultProps } from '@fluentui/react-aria';
+import type { ARIAButtonSlotProps } from '@fluentui/react-aria';
 import { ARIAButtonType } from '@fluentui/react-aria';
 import type { ComponentProps } from '@fluentui/react-utilities';
 import type { ComponentState } from '@fluentui/react-utilities';
@@ -128,7 +130,6 @@ export const menuItemClassNames: SlotClassNames<MenuItemSlots>;
 
 // @public (undocumented)
 export type MenuItemProps = ComponentProps<Partial<MenuItemSlots>> & {
-    disabled?: boolean;
     hasSubmenu?: boolean;
     persistOnClick?: boolean;
 };
@@ -153,14 +154,12 @@ export type MenuItemSelectableProps = {
 
 // @public
 export type MenuItemSelectableState = MenuItemSelectableProps & {
-    checkedItems: string[];
-    onCheckedValueChange: (e: React_2.MouseEvent | React_2.KeyboardEvent, name: string, checkedItems: string[]) => void;
     checked: boolean;
 };
 
 // @public (undocumented)
 export type MenuItemSlots = {
-    root: Slot<'div'>;
+    root: Slot<ARIAButtonSlotProps<'div'>>;
     icon?: Slot<'span'>;
     checkmark?: Slot<'span'>;
     submenuIndicator?: Slot<'span'>;
@@ -169,7 +168,9 @@ export type MenuItemSlots = {
 };
 
 // @public (undocumented)
-export type MenuItemState = ComponentState<MenuItemSlots> & Pick<MenuItemProps, 'disabled' | 'hasSubmenu' | 'persistOnClick'>;
+export type MenuItemState = ComponentState<MenuItemSlots> & Required<Pick<MenuItemProps, 'disabled' | 'hasSubmenu' | 'persistOnClick'>> & {
+    isNativeButton: boolean;
+};
 
 // @public
 export const MenuList: ForwardRefComponent<MenuListProps>;
@@ -398,16 +399,16 @@ export const useMenuGroupHeaderStyles_unstable: (state: MenuGroupHeaderState) =>
 export const useMenuGroupStyles_unstable: (state: MenuGroupState) => MenuGroupState;
 
 // @public
-export const useMenuItem_unstable: (props: MenuItemProps, ref: React_2.Ref<HTMLElement>) => MenuItemState;
+export const useMenuItem_unstable: (props: MenuItemProps, ref: React_2.Ref<ARIAButtonElement<'div'>>) => MenuItemState;
 
 // @public
-export const useMenuItemCheckbox_unstable: (props: MenuItemCheckboxProps, ref: React_2.Ref<HTMLElement>) => MenuItemCheckboxState;
+export const useMenuItemCheckbox_unstable: (props: MenuItemCheckboxProps, ref: React_2.Ref<ARIAButtonElement<'div'>>) => MenuItemCheckboxState;
 
 // @public (undocumented)
 export const useMenuItemCheckboxStyles_unstable: (state: MenuItemCheckboxState) => void;
 
 // @public
-export const useMenuItemRadio_unstable: (props: MenuItemRadioProps, ref: React_2.Ref<HTMLElement>) => MenuItemRadioState;
+export const useMenuItemRadio_unstable: (props: MenuItemRadioProps, ref: React_2.Ref<ARIAButtonElement<'div'>>) => MenuItemRadioState;
 
 // @public (undocumented)
 export const useMenuItemRadioStyles_unstable: (state: MenuItemRadioState) => void;

packages/react-components/react-menu/src/components/Menu/Menu.types.ts

@@ -162,6 +162,8 @@ export type MenuOpenChangeData = {
   bubble?: boolean;
   /**
    * Indicates whether the change of state was a keyboard interaction
+   * @deprecated
+   * This should not be used, since `Enter`, `Space` and click should be interpreted as the same thing as a click
    */
   keyboard?: boolean;
   open: boolean;

packages/react-components/react-menu/src/components/Menu/useMenu.tsx

@@ -14,6 +14,7 @@ import { useMenuContext_unstable } from '../../contexts/menuContext';
 import { MENU_ENTER_EVENT, useOnMenuMouseEnter } from '../../utils/index';
 import { useIsSubmenu } from '../../utils/useIsSubmenu';
 import type { MenuOpenChangeData, MenuOpenEvents, MenuProps, MenuState } from './Menu.types';
+import { Tab } from '@fluentui/keyboard-keys';
 
 /**
  * Create the state required to render Menu.
@@ -160,7 +161,6 @@ const useMenuOpenState = (
   const parentSetOpen = useMenuContext_unstable(context => context.setOpen);
   const onOpenChange: MenuState['onOpenChange'] = useEventCallback((e, data) => state.onOpenChange?.(e, data));
 
-  const shouldHandleKeyboardRef = React.useRef(false);
   const shouldHandleTabRef = React.useRef(false);
   const pressedShiftRef = React.useRef(false);
   const setOpenTimeout = React.useRef(0);
@@ -183,10 +183,9 @@ const useMenuOpenState = (
       state.setContextTarget(undefined);
     }
 
-    if (data.keyboard) {
-      shouldHandleKeyboardRef.current = true;
-      shouldHandleTabRef.current = (e as React.KeyboardEvent).key === 'Tab';
-      pressedShiftRef.current = (e as React.KeyboardEvent).shiftKey;
+    if (e.type === 'keydown' && (e as React.KeyboardEvent<HTMLElement>).key === Tab) {
+      shouldHandleTabRef.current = true;
+      pressedShiftRef.current = (e as React.KeyboardEvent<HTMLElement>).shiftKey;
     }
 
     if (data.bubble) {
@@ -288,15 +287,14 @@ const useMenuOpenState = (
       focusFirst();
     }
 
-    if (shouldHandleKeyboardRef.current && !open) {
+    if (!open) {
       if (shouldHandleTabRef.current && !state.isSubmenu) {
         pressedShiftRef.current ? focusBeforeMenuTrigger() : focusAfterMenuTrigger();
       } else {
         state.triggerRef.current?.focus();
       }
     }
 
-    shouldHandleKeyboardRef.current = false;
     shouldHandleTabRef.current = false;
     pressedShiftRef.current = false;
   }, [state.triggerRef, state.isSubmenu, open, focusFirst, focusAfterMenuTrigger, focusBeforeMenuTrigger]);

packages/react-components/react-menu/src/components/MenuItem/MenuItem.test.tsx

@@ -7,11 +7,12 @@ import { isConformant } from '../../common/isConformant';
 import { MenuTriggerContextProvider } from '../../contexts/menuTriggerContext';
 import { MenuListProvider } from '../../contexts/menuListContext';
 import { mockUseMenuContext } from '../../common/mockUseMenuContext';
+import type { MenuItemProps } from './MenuItem.types';
 
 jest.mock('../../contexts/menuContext');
 
 describe('MenuItem', () => {
-  isConformant({
+  isConformant<MenuItemProps>({
     Component: MenuItem,
     displayName: 'MenuItem',
     testOptions: {

packages/react-components/react-menu/src/components/MenuItem/MenuItem.types.ts

@@ -1,7 +1,8 @@
+import type { ARIAButtonSlotProps } from '@fluentui/react-aria';
 import type { ComponentProps, ComponentState, Slot } from '@fluentui/react-utilities';
 
 export type MenuItemSlots = {
-  root: Slot<'div'>;
+  root: Slot<ARIAButtonSlotProps<'div'>>;
 
   /**
    * Icon slot rendered before children content
@@ -32,13 +33,6 @@ export type MenuItemSlots = {
 };
 
 export type MenuItemProps = ComponentProps<Partial<MenuItemSlots>> & {
-  /**
-   * Applies disabled styles to menu item but remains focusable
-   *
-   * @default false
-   */
-  disabled?: boolean;
-
   /**
    * If the menu item is a trigger for a submenu
    *
@@ -55,4 +49,6 @@ export type MenuItemProps = ComponentProps<Partial<MenuItemSlots>> & {
 };
 
 export type MenuItemState = ComponentState<MenuItemSlots> &
-  Pick<MenuItemProps, 'disabled' | 'hasSubmenu' | 'persistOnClick'>;
+  Required<Pick<MenuItemProps, 'disabled' | 'hasSubmenu' | 'persistOnClick'>> & {
+    isNativeButton: boolean;
+  };

packages/react-components/react-menu/src/components/MenuItem/__snapshots__/MenuItem.test.tsx.snap

@@ -5,6 +5,7 @@ exports[`MenuItem renders a default state 1`] = `
   className="fui-MenuItem"
   onClick={[Function]}
   onKeyDown={[Function]}
+  onKeyUp={[Function]}
   onMouseEnter={[Function]}
   role="menuitem"
   tabIndex={0}

packages/react-components/react-menu/src/components/MenuItem/useCharacterSearch.ts

@@ -1,15 +1,15 @@
 import * as React from 'react';
 import { useMenuListContext_unstable } from '../../contexts/menuListContext';
 import type { MenuItemState } from '../../components/index';
+import type { ARIAButtonElementIntersection } from '@fluentui/react-aria';
 
 export const useCharacterSearch = (state: MenuItemState, ref: React.RefObject<HTMLElement>) => {
   const setFocusByFirstCharacter = useMenuListContext_unstable(context => context.setFocusByFirstCharacter);
 
-  const { onKeyDown: onKeyDownBase } = state.root;
-  state.root.onKeyDown = e => {
-    if (onKeyDownBase) {
-      onKeyDownBase(e);
-    }
+  const { onKeyDown: originalOnKeyDown } = state.root;
+
+  state.root.onKeyDown = (e: React.KeyboardEvent<ARIAButtonElementIntersection>) => {
+    originalOnKeyDown?.(e);
 
     if (e.key?.length > 1) {
       return;

packages/react-components/react-menu/src/components/MenuItem/useMenuItem.tsx

@@ -1,11 +1,5 @@
 import * as React from 'react';
-import {
-  useEventCallback,
-  shouldPreventDefaultOnKeyDown,
-  resolveShorthand,
-  useMergedRefs,
-  getNativeElementProps,
-} from '@fluentui/react-utilities';
+import { useEventCallback, resolveShorthand, useMergedRefs, getNativeElementProps } from '@fluentui/react-utilities';
 import { useFluent_unstable as useFluent } from '@fluentui/react-shared-contexts';
 import { useCharacterSearch } from './useCharacterSearch';
 import { useMenuTriggerContext_unstable } from '../../contexts/menuTriggerContext';
@@ -19,29 +13,40 @@ import {
 import { useMenuListContext_unstable } from '../../contexts/menuListContext';
 import { useMenuContext_unstable } from '../../contexts/menuContext';
 import type { MenuItemProps, MenuItemState } from './MenuItem.types';
+import type { ARIAButtonElement, ARIAButtonElementIntersection, ARIAButtonSlotProps } from '@fluentui/react-aria';
+import { useARIAButtonShorthand } from '@fluentui/react-aria';
+import { Enter, Space } from '@fluentui/keyboard-keys';
 
 const ChevronRightIcon = bundleIcon(ChevronRightFilled, ChevronRightRegular);
 const ChevronLeftIcon = bundleIcon(ChevronLeftFilled, ChevronLeftRegular);
 
 /**
  * Returns the props and state required to render the component
  */
-export const useMenuItem_unstable = (props: MenuItemProps, ref: React.Ref<HTMLElement>): MenuItemState => {
+export const useMenuItem_unstable = (props: MenuItemProps, ref: React.Ref<ARIAButtonElement<'div'>>): MenuItemState => {
+  const isSubmenuTrigger = useMenuTriggerContext_unstable();
+  const persistOnClickContext = useMenuContext_unstable(context => context.persistOnItemClick);
+  const {
+    as = 'div',
+    disabled,
+    disabledFocusable,
+    hasSubmenu = isSubmenuTrigger,
+    persistOnClick = persistOnClickContext,
+  } = props;
   const hasIcons = useMenuListContext_unstable(context => context.hasIcons);
   const hasCheckmarks = useMenuListContext_unstable(context => context.hasCheckmarks);
   const setOpen = useMenuContext_unstable(context => context.setOpen);
-  const persistOnClickContext = useMenuContext_unstable(context => context.persistOnItemClick);
-  const dismissedWithKeyboardRef = React.useRef(false);
-
-  const isSubmenuTrigger = useMenuTriggerContext_unstable();
-  const hasSubmenu = props.hasSubmenu ?? isSubmenuTrigger;
 
   const { dir } = useFluent();
-  const innerRef = React.useRef<HTMLElement>(null);
+  const innerRef = React.useRef<ARIAButtonElementIntersection<'div'>>(null);
+  const dismissedWithKeyboardRef = React.useRef(false);
+
+  const isDisabled = Boolean(disabled || disabledFocusable);
 
   const state: MenuItemState = {
     hasSubmenu,
-    ...props,
+    disabled: isDisabled,
+    persistOnClick,
     components: {
       root: 'div',
       icon: 'span',
@@ -50,13 +55,40 @@ export const useMenuItem_unstable = (props: MenuItemProps, ref: React.Ref<HTMLEl
       content: 'span',
       secondaryContent: 'span',
     },
-    root: getNativeElementProps('div', {
-      ref: useMergedRefs(ref, innerRef),
-      role: 'menuitem',
-      tabIndex: 0,
-      'aria-disabled': props.disabled,
-      ...props,
-    }),
+    isNativeButton: as === 'button',
+    root: getNativeElementProps(
+      as,
+      useARIAButtonShorthand<ARIAButtonSlotProps<'div'>>(
+        { disabled: false, disabledFocusable: isDisabled, as },
+        {
+          required: true,
+          defaultProps: {
+            role: 'menuitem',
+            ...props,
+            ref: useMergedRefs(ref, innerRef) as React.Ref<ARIAButtonElementIntersection<'div'>>,
+            onKeyDown: useEventCallback(event => {
+              props.onKeyDown?.(event);
+              if (!event.isDefaultPrevented() && (event.key === Space || event.key === Enter)) {
+                dismissedWithKeyboardRef.current = true;
+              }
+            }),
+            onMouseEnter: useEventCallback(event => {
+              innerRef.current?.focus();
+
+              props.onMouseEnter?.(event);
+            }),
+            onClick: useEventCallback(event => {
+              if (!hasSubmenu && !persistOnClick) {
+                setOpen(event, { open: false, keyboard: dismissedWithKeyboardRef.current, bubble: true });
+                dismissedWithKeyboardRef.current = false;
+              }
+
+              props.onClick?.(event);
+            }),
+          },
+        },
+      ),
+    ),
     icon: resolveShorthand(props.icon, { required: hasIcons }),
     checkmark: resolveShorthand(props.checkmark, { required: hasCheckmarks }),
     submenuIndicator: resolveShorthand(props.submenuIndicator, {
@@ -71,52 +103,6 @@ export const useMenuItem_unstable = (props: MenuItemProps, ref: React.Ref<HTMLEl
     }),
     secondaryContent: resolveShorthand(props.secondaryContent),
   };
-
-  const { onClick: onClickOriginal, onKeyDown: onKeyDownOriginal } = state.root;
-  state.root.onKeyDown = e => {
-    if (shouldPreventDefaultOnKeyDown(e)) {
-      if (state.disabled) {
-        e.preventDefault();
-        e.stopPropagation();
-        return;
-      }
-
-      dismissedWithKeyboardRef.current = true;
-      e.preventDefault();
-      (e.target as HTMLElement)?.click();
-    }
-
-    onKeyDownOriginal?.(e);
-  };
-
-  state.root.onClick = e => {
-    if (state.disabled) {
-      e.preventDefault();
-      e.stopPropagation();
-      return;
-    }
-
-    let shouldPersist = persistOnClickContext;
-    // prop wins over context;
-    if (state.persistOnClick !== undefined && persistOnClickContext !== state.persistOnClick) {
-      shouldPersist = state.persistOnClick;
-    }
-
-    if (!hasSubmenu && !shouldPersist) {
-      setOpen(e, { open: false, keyboard: dismissedWithKeyboardRef.current, bubble: true });
-      dismissedWithKeyboardRef.current = false;
-    }
-
-    onClickOriginal?.(e);
-  };
-
-  const { onMouseEnter: onMouseEnterOriginal } = state.root;
-  state.root.onMouseEnter = useEventCallback(e => {
-    innerRef.current?.focus();
-
-    onMouseEnterOriginal?.(e);
-  });
-
   useCharacterSearch(state, innerRef);
   return state;
 };