Skip to content

Commit

Permalink
remove unused "longpress" behaviour
Browse files Browse the repository at this point in the history
  • Loading branch information
@jossmac
jossmac committed Dec 4, 2024
1 parent a361b0a commit 70bbaed
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 247 deletions.
26 changes: 10 additions & 16 deletions design-system/pkg/src/menu/MenuTrigger.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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
);
Expand Down Expand Up @@ -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
Expand Down
7 changes: 0 additions & 7 deletions design-system/pkg/src/menu/stories/Menu.stories.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
222 changes: 0 additions & 222 deletions design-system/pkg/src/menu/test/MenuTrigger.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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>(
Expand Down
4 changes: 2 additions & 2 deletions design-system/pkg/src/menu/types.ts
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type {
AriaMenuProps,
MenuTriggerProps as _MenuTriggerProps,
MenuTriggerProps as AriaMenuTriggerProps,
} from '@react-types/menu';
import type {
Alignment,
Expand Down Expand Up @@ -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. */
Expand Down

0 comments on commit 70bbaed

Please sign in to comment.