Skip to content

feat(menu): add MenuItem action support for onPress, href, and to #1528

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Marcosld merged 6 commits into from
May 11, 2026
Merged

feat(menu): add MenuItem action support for onPress, href, and to #1528

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
aceaaf0
feat(Menu): add href, to and onPress support to MenuItem using action…
MurilloLeoni Apr 6, 2026
c465bb4
refactor(menu): migrate MenuItem from action prop to onPress href and to
MurilloLeoni Apr 9, 2026
d7f99b8
refactor(menu): simplify MenuItem logic and remove duplicated content
MurilloLeoni Apr 13, 2026
ad268f9
Merge branch 'master' into feat/menu-item-action-props
yceballost Apr 15, 2026
0375b09
Merge branch 'master' into feat/menu-item-action-props
yceballost Apr 27, 2026
3dcbb30
Merge branch 'master' into feat/menu-item-action-props
MurilloLeoni May 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions playroom/snippets.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,22 +27,22 @@ const menuSnippet = {
renderMenu={({ ref, className }) => (
<div ref={ref} className={className}>
<MenuSection>
<MenuItem label="option 1" onPress={() => {}} />
<MenuItem label="option 1" action={{onPress: () => {}}} />
<MenuItem
label="option 2"
onPress={() => setState("option 2", !getState("option 2", false))}
action={{onPress: () => setState("option 2", !getState("option 2", false))}}
Comment thread
MurilloLeoni marked this conversation as resolved.
Outdated
controlType="checkbox"
checked={getState("option 2", false)}
/>
<MenuItem label="option 3" disabled onPress={() => {}} />
<MenuItem label="option 3" disabled action={{onPress: () => {}}} />
</MenuSection>

<MenuSection>
<MenuItem
label="option 4"
destructive
Icon={IconLightningRegular}
onPress={() => {}}
action={{onPress: () => {}}}
/>
</MenuSection>

Expand All @@ -51,11 +51,11 @@ const menuSnippet = {
label="option 5"
disabled
Icon={IconLightningRegular}
onPress={() => {}}
action={{onPress: () => {}}}
/>
<MenuItem
label="An option with a really long text to verify overflow"
onPress={() => {}}
action={{onPress: () => {}}}
/>
</MenuSection>
</div>
Expand Down
4 changes: 2 additions & 2 deletions src/__private_stories__/components-in-portals-story.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ export const Default: StoryComponent = () => {
)}
renderMenu={({ref, className}) => (
<div ref={ref} className={className}>
<MenuItem key="option 1" label="Option 1" onPress={() => {}} />
<MenuItem key="option 2" label="Option 2" onPress={() => {}} />
<MenuItem key="option 1" label="Option 1" action={{onPress: () => {}}} />
<MenuItem key="option 2" label="Option 2" action={{onPress: () => {}}} />
</div>
)}
/>
Expand Down
21 changes: 13 additions & 8 deletions src/__stories__/menu-story.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,12 +85,17 @@ export const Default: StoryComponent<MenuArgs> = ({
<MenuItem
key={optionIndex}
label={`Option ${optionIndex + 1}`}
onPress={(item) => {
if (checkbox) {
setValues(item);
} else {
alert({title: `Item ${item + 1}`, message: 'pressed'});
}
action={{
onPress: () => {
if (checkbox) {
setValues(optionIndex);
} else {
alert({
title: `Item ${optionIndex + 1}`,
message: 'pressed',
});
}
},
}}
{...(checkbox && {
controlType: 'checkbox' as const,
Expand All @@ -104,7 +109,7 @@ export const Default: StoryComponent<MenuArgs> = ({
<MenuItem
key="closingOption"
label="Click to close the menu"
onPress={() => {}}
action={{onPress: () => {}}}
destructive
/>
</MenuSection>
Expand Down Expand Up @@ -171,7 +176,7 @@ export const InsideCard: StoryComponent = () => {
<MenuItem
key={optionIndex + 1}
label={`Option ${optionIndex + 1}`}
onPress={() => null}
action={{onPress: () => {}}}
/>
))}
</div>
Expand Down
107 changes: 106 additions & 1 deletion src/__tests__/menu-test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,26 @@ import userEvent from '@testing-library/user-event';
import {render, screen, waitFor} from '@testing-library/react';
import {ThemeContextProvider, Touchable, Text3, Menu, MenuSection, MenuItem} from '..';
import {makeTheme} from './test-utils';
import {redirect as redirectSpy} from '../utils/browser';
import {MemoryRouter, useLocation, Link as ReactRouterLink} from 'react-router-dom';

import type {ThemeConfig} from '../theme';

const Link: ThemeConfig['Link'] = ({innerRef, ...props}) => <ReactRouterLink {...props} ref={innerRef} />;

jest.mock('../utils/browser', () => ({
...jest.requireActual('../utils/browser'),
redirect: jest.fn(),
}));

beforeEach(() => {
(redirectSpy as any).mockReset();
});

const options = ['Option 1', 'Option 2', 'Option 3'];

test('Menu closes after pressing an option', async () => {
const onPressSpy = jest.fn();
render(
<ThemeContextProvider theme={makeTheme()}>
<Menu
Expand All @@ -19,7 +35,7 @@ test('Menu closes after pressing an option', async () => {
<div ref={ref} className={className}>
<MenuSection>
{options.map((option) => (
<MenuItem key={option} label={option} onPress={() => {}} />
<MenuItem key={option} label={option} action={{onPress: onPressSpy}} />
))}
</MenuSection>
</div>
Expand All @@ -39,6 +55,8 @@ test('Menu closes after pressing an option', async () => {

await userEvent.click(screen.getByText('Option 1'));

expect(onPressSpy).toHaveBeenCalledTimes(1);

/** We have to wait until the CSS transition finishes when closing the menu */
await waitFor(() => {
expect(screen.getByText('menu is close')).toBeInTheDocument();
Expand All @@ -47,3 +65,90 @@ test('Menu closes after pressing an option', async () => {
expect(screen.queryByText('Option 3')).not.toBeInTheDocument();
});
});

test('Menu closes after clicking an href option', async () => {
render(
<ThemeContextProvider theme={makeTheme()}>
<Menu
renderTarget={({ref, onPress, isMenuOpen}) => (
<Touchable ref={ref} onPress={onPress}>
<Text3 regular>{isMenuOpen ? 'menu is open' : 'menu is close'}</Text3>
</Touchable>
)}
renderMenu={({ref, className}) => (
<div ref={ref} className={className}>
<MenuSection>
<MenuItem label="External link" action={{href: 'https://example.com'}} />
</MenuSection>
</div>
)}
/>
</ThemeContextProvider>
);

await userEvent.click(screen.getByText('menu is close'));

expect(screen.getByText('menu is open')).toBeInTheDocument();

const menuItem = screen.getByRole('menuitem', {name: 'External link'});
expect(menuItem.tagName).toBe('A');
expect(menuItem).toHaveAttribute('href', 'https://example.com');

await userEvent.click(menuItem);

await waitFor(() => {
expect(redirectSpy).toHaveBeenCalledWith('https://example.com', false, false);
});

/** We have to wait until the CSS transition finishes when closing the menu */
await waitFor(() => {
expect(screen.getByText('menu is close')).toBeInTheDocument();
});
});

test('Menu closes after clicking a "to" option', async () => {
const to = '/interna';

const CurrentPath = () => <div>Current path: {useLocation().pathname}</div>;

render(
<ThemeContextProvider theme={makeTheme({Link})}>
<MemoryRouter>
<Menu
renderTarget={({ref, onPress, isMenuOpen}) => (
<Touchable ref={ref} onPress={onPress}>
<Text3 regular>{isMenuOpen ? 'menu is open' : 'menu is close'}</Text3>
</Touchable>
)}
renderMenu={({ref, className}) => (
<div ref={ref} className={className}>
<MenuSection>
<MenuItem label="Internal link" action={{to}} />
</MenuSection>
</div>
)}
/>
<CurrentPath />
</MemoryRouter>
</ThemeContextProvider>
);

await userEvent.click(screen.getByText('menu is close'));

expect(screen.getByText('menu is open')).toBeInTheDocument();

const menuItem = screen.getByRole('menuitem', {name: 'Internal link'});
expect(menuItem.tagName).toBe('A');
expect(menuItem).toHaveAttribute('href', to);

expect(screen.getByText('Current path: /')).toBeInTheDocument();

await userEvent.click(menuItem);

expect(screen.getByText(`Current path: ${to}`)).toBeInTheDocument();

/** We have to wait until the CSS transition finishes when closing the menu */
await waitFor(() => {
expect(screen.getByText('menu is close')).toBeInTheDocument();
});
});
1 change: 1 addition & 0 deletions src/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ export {default as CoverHero} from './cover-hero';
export {Table} from './table';
export {default as Divider} from './divider';
export {Menu, MenuItem, MenuSection} from './menu';
export type {MenuItemAction} from './menu';
export {default as EmptyState} from './empty-state';
export {default as EmptyStateCard} from './empty-state-card';
export {default as Callout} from './callout';
Expand Down
29 changes: 19 additions & 10 deletions src/menu.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,21 +51,24 @@ const getItemIndexInMenu = (menu: HTMLElement | null, item: HTMLElement | null):
return itemIndex < 0 ? null : itemIndex;
};

export type MenuItemAction = {onPress: () => void} | {href: string} | {to: string};
Comment thread
MurilloLeoni marked this conversation as resolved.
Outdated

interface MenuItemBaseProps {
label: string;
Icon?: (props: IconProps) => JSX.Element;
destructive?: boolean;
disabled?: boolean;
onPress: (item: number) => void;
Comment thread
MurilloLeoni marked this conversation as resolved.
dataAttributes?: DataAttributes;
}

interface MenuItemWithoutControlProps extends MenuItemBaseProps {
action: MenuItemAction;
controlType?: undefined;
checked?: undefined;
}

interface MenuItemWithControlProps extends MenuItemBaseProps {
action: {onPress: () => void};
controlType?: 'checkbox';
checked?: boolean;
}
Expand All @@ -77,7 +80,7 @@ export const MenuItem = ({
Icon,
destructive,
disabled,
onPress,
action,
controlType,
checked,
dataAttributes,
Expand All @@ -100,8 +103,8 @@ export const MenuItem = ({
name={label}
checked={checked}
onChange={() => {
if (isMenuOpen && itemIndex !== null) {
onPress(itemIndex);
if (isMenuOpen) {
action.onPress();
closeMenu();
}
}}
Expand Down Expand Up @@ -129,12 +132,18 @@ export const MenuItem = ({
) : (
<Touchable
ref={itemRef}
onPress={() => {
if (isMenuOpen && itemIndex !== null) {
onPress(itemIndex);
closeMenu();
}
}}
{...('href' in action
? {href: action.href, onNavigate: closeMenu}
: 'to' in action
? {to: action.to, onNavigate: closeMenu}
: {
onPress: () => {
if (isMenuOpen) {
action.onPress();
closeMenu();
}
},
})}
disabled={disabled}
role="menuitem"
dataAttributes={menuItemDataAttributes}
Expand Down
Loading