Menu hold affordance (#1370)

.changeset/smart-monkeys-leave.md

@@ -0,0 +1,5 @@
+---
+'@keystar/ui': patch
+---
+
+Remove unused "longpress" behaviour on `Menu` component.

design-system/pkg/src/menu/MenuTrigger.tsx

@@ -6,7 +6,6 @@ import { Placement } from '@react-types/overlays';
 import React, { ForwardedRef, forwardRef, Fragment, useRef } from 'react';
 
 import { Popover, Tray } from '@keystar/ui/overlays';
-import { SlotProvider } from '@keystar/ui/slots';
 import { tokenSchema, useIsMobileDevice } from '@keystar/ui/style';
 
 import { MenuContext } from './context';
@@ -25,19 +24,18 @@ export const MenuTrigger = forwardRef(function MenuTrigger(
   let menuTriggerRef = domRef || triggerRef;
   let menuRef = useRef<HTMLUListElement>(null);
   let {
-    children,
     align = 'start',
-    shouldFlip = true,
-    direction = 'bottom',
+    children,
     closeOnSelect,
-    trigger = 'press',
+    direction = 'bottom',
+    shouldFlip = true,
   } = props;
 
   let [menuTrigger, menu] = React.Children.toArray(children);
   let state = useMenuTriggerState(props);
 
   let { menuTriggerProps, menuProps } = useMenuTrigger(
-    { trigger },
+    { trigger: 'press' },
     state,
     menuTriggerRef
   );
@@ -96,17 +94,13 @@ export const MenuTrigger = forwardRef(function MenuTrigger(
 
   return (
     <Fragment>
-      <SlotProvider
-        slots={{ actionButton: { holdAffordance: trigger === 'longPress' } }}
+      <PressResponder
+        {...menuTriggerProps}
+        ref={menuTriggerRef}
+        isPressed={state.isOpen}
       >
-        <PressResponder
-          {...menuTriggerProps}
-          ref={menuTriggerRef}
-          isPressed={state.isOpen}
-        >
-          {menuTrigger}
-        </PressResponder>
-      </SlotProvider>
+        {menuTrigger}
+      </PressResponder>
       <MenuContext.Provider
         // TODO: Fix this type error
         // @ts-expect-error

design-system/pkg/src/menu/stories/Menu.stories.tsx

@@ -662,13 +662,6 @@ WithFalsyKey.story = {
   name: 'with falsy key',
 };
 
-export const MenuTriggerWithTriggerLongPress = () =>
-  render(defaultMenu, { trigger: 'longPress' });
-
-MenuTriggerWithTriggerLongPress.story = {
-  name: 'MenuTrigger with trigger="longPress"',
-};
-
 // .add('controlled isOpen', () => <ControlledOpeningMenuTrigger />)
 
 let customMenuItem = (item: any) => {

design-system/pkg/src/menu/test/MenuTrigger.test.tsx

@@ -16,22 +16,16 @@ import {
   Section,
 } from '..';
 import { Button, ButtonProps } from '@keystar/ui/button';
-import { SelectionMode } from '@react-types/shared';
 import { createRef } from 'react';
 import {
   RenderResult,
   act,
-  DEFAULT_LONG_PRESS_TIME,
   fireEvent,
-  fireLongPress,
   firePress,
-  fireTouch,
-  installPointerEvent,
   render,
   renderWithProvider,
   within,
   KEYS,
-  waitFor,
 } from '#test-utils';
 
 let triggerText = 'Menu Button';
@@ -760,222 +754,6 @@ describe('menu/MenuTrigger', () => {
     let menu2Item1 = within(menu2).getByText('Whiskey');
     expect(menu2Item1).toBeInTheDocument();
   });
-
-  describe('trigger="longPress" open behavior', function () {
-    installPointerEvent();
-
-    const ERROR_MENU_NOT_FOUND = new Error('Menu not found');
-    const getMenuOrThrow = (tree: RenderResult, button: HTMLElement) => {
-      try {
-        let menu = tree.getByRole('menu');
-        expect(menu).toBeTruthy();
-        expect(menu).toHaveAttribute('aria-labelledby', button.id);
-      } catch (e) {
-        throw ERROR_MENU_NOT_FOUND;
-      }
-    };
-
-    it('should open the menu on longPress', function () {
-      verifyMenuToggle(
-        { onOpenChange, trigger: 'longPress' },
-        {},
-        (button, menu) => {
-          expect(button).toHaveAttribute('aria-describedby');
-          expect(
-            document.getElementById(button.getAttribute('aria-describedby')!)
-          ).toHaveTextContent(
-            'Long press or press Alt + ArrowDown to open menu'
-          );
-
-          if (!menu) {
-            fireLongPress(button);
-          } else {
-            fireTouch(button);
-          }
-        }
-      );
-    });
-
-    it('should not open menu on click', function () {
-      let tree = renderComponent({ onOpenChange, trigger: 'longPress' });
-      let button = tree.getByRole('button');
-
-      act(() => {
-        fireTouch(button);
-        setTimeout(
-          () => {
-            expect(getMenuOrThrow).toThrow(ERROR_MENU_NOT_FOUND);
-          },
-          0,
-          tree,
-          button
-        );
-        jest.runAllTimers();
-      });
-    });
-
-    it(`should not open menu on short press (default threshold set to ${DEFAULT_LONG_PRESS_TIME}ms)`, function () {
-      let tree = renderComponent({ onOpenChange, trigger: 'longPress' });
-      let button = tree.getByRole('button');
-
-      act(() => {
-        fireTouch(button);
-        setTimeout(
-          () => {
-            expect(getMenuOrThrow).toThrow(ERROR_MENU_NOT_FOUND);
-          },
-          DEFAULT_LONG_PRESS_TIME / 2,
-          tree,
-          button
-        );
-        jest.runAllTimers();
-      });
-    });
-
-    it('should not open the menu on Enter', function () {
-      let tree = renderComponent({ onOpenChange, trigger: 'longPress' });
-      let button = tree.getByRole('button');
-      act(() => {
-        fireTouch(button);
-        setTimeout(
-          () => {
-            expect(getMenuOrThrow).toThrow(ERROR_MENU_NOT_FOUND);
-          },
-          0,
-          tree,
-          button
-        );
-        jest.runAllTimers();
-      });
-    });
-
-    it('should not open the menu on Space', function () {
-      let tree = renderComponent({ onOpenChange, trigger: 'longPress' });
-      let button = tree.getByRole('button');
-      act(() => {
-        fireTouch(button);
-        setTimeout(
-          () => {
-            expect(getMenuOrThrow).toThrow(ERROR_MENU_NOT_FOUND);
-          },
-          0,
-          tree,
-          button
-        );
-        jest.runAllTimers();
-      });
-    });
-
-    it('should open the menu on Alt+ArrowUp', function () {
-      verifyMenuToggle(
-        { onOpenChange, trigger: 'longPress' },
-        {},
-        (button, menu) => {
-          if (!menu) {
-            fireEvent.keyDown(button, { key: 'ArrowUp', altKey: true });
-          } else {
-            fireTouch(button);
-          }
-        }
-      );
-    });
-
-    it('should open the menu on Alt+ArrowDown', function () {
-      verifyMenuToggle(
-        { onOpenChange, trigger: 'longPress' },
-        {},
-        (button, menu) => {
-          if (!menu) {
-            fireEvent.keyDown(button, { key: 'ArrowDown', altKey: true });
-          } else {
-            fireTouch(button);
-          }
-        }
-      );
-    });
-  });
-
-  describe('trigger="longPress" focus behavior', function () {
-    installPointerEvent();
-
-    function expectMenuItemToBeActive(
-      tree: RenderResult,
-      idx: number,
-      selectionMode: SelectionMode
-    ) {
-      let menuItemRole = 'menuitem';
-      if (selectionMode === 'multiple') {
-        menuItemRole = 'menuitemcheckbox';
-      } else if (selectionMode === 'single') {
-        menuItemRole = 'menuitemradio';
-      }
-      let menu = tree.getByRole('menu');
-      expect(menu).toBeTruthy();
-      let menuItems = within(menu).getAllByRole(menuItemRole);
-      let selectedItem = menuItems[idx < 0 ? menuItems.length + idx : idx];
-      expect(selectedItem).toBe(document.activeElement);
-      return menu;
-    }
-
-    it('should focus the selected item on menu open', async function () {
-      let tree = renderComponent(
-        { trigger: 'longPress' },
-        { selectedKeys: ['Bar'], selectionMode: 'single' }
-      );
-      let button = tree.getByRole('button');
-      act(() => {
-        fireLongPress(button);
-        jest.runAllTimers();
-      });
-      let menu = expectMenuItemToBeActive(tree, 1, 'single');
-      act(() => {
-        fireTouch(button);
-        jest.runAllTimers();
-      });
-
-      await waitFor(() => {
-        expect(menu).not.toBeInTheDocument();
-      });
-
-      // Opening menu via Alt+ArrowUp still autofocuses the selected item
-      fireEvent.keyDown(button, { key: 'ArrowUp', altKey: true });
-      menu = expectMenuItemToBeActive(tree, 1, 'single');
-
-      act(() => {
-        fireTouch(button);
-        jest.runAllTimers();
-      });
-      await waitFor(() => {
-        expect(menu).not.toBeInTheDocument();
-      });
-
-      // Opening menu via Alt+ArrowDown still autofocuses the selected item
-      fireEvent.keyDown(button, { key: 'ArrowDown', altKey: true });
-      menu = expectMenuItemToBeActive(tree, 1, 'single');
-
-      act(() => {
-        fireTouch(button);
-        jest.runAllTimers();
-      });
-      await waitFor(() => {
-        expect(menu).not.toBeInTheDocument();
-      });
-    });
-
-    it('should focus the last item on Alt+ArrowUp if no selectedKeys specified', function () {
-      let tree = renderComponent({ trigger: 'longPress' });
-      let button = tree.getByRole('button');
-      fireEvent.keyDown(button, { key: 'ArrowUp', altKey: true });
-      expectMenuItemToBeActive(tree, -1, 'none');
-    });
-
-    it('should focus the first item on Alt+ArrowDown if no selectedKeys specified', function () {
-      let tree = renderComponent({ trigger: 'longPress' });
-      let button = tree.getByRole('button');
-      fireEvent.keyDown(button, { key: 'ArrowDown', altKey: true });
-      expectMenuItemToBeActive(tree, 0, 'none');
-    });
-  });
 });
 
 function renderComponent<T>(

design-system/pkg/src/menu/types.ts

@@ -1,6 +1,6 @@
 import type {
   AriaMenuProps,
-  MenuTriggerProps as _MenuTriggerProps,
+  MenuTriggerProps as AriaMenuTriggerProps,
 } from '@react-types/menu';
 import type {
   Alignment,
@@ -43,7 +43,7 @@ export type MenuTriggerProps = {
    * @default true
    */
   closeOnSelect?: boolean;
-} & _MenuTriggerProps;
+} & Omit<AriaMenuTriggerProps, 'trigger'>;
 
 export type ActionMenuProps<T> = {
   /** Whether the element should receive focus on render. */