Skip to content

Commit

Permalink
feat(Menu): allow href and onClick to co-exist on a menu item (#1779)
Browse files Browse the repository at this point in the history
  • Loading branch information
booc0mtaco authored Oct 9, 2023
1 parent f9e77ea commit 971f189
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 6 deletions.
3 changes: 1 addition & 2 deletions src/components/Menu/Menu.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,7 @@ export const Default: StoryObj<MenuProps> = {
>
MDN: Menu
</Menu.Item>
{/* eslint-disable-next-line no-alert */}
<Menu.Item onClick={() => alert('Item clicked')}>
<Menu.Item href="#index" onClick={() => console.log('Item clicked')}>
Trigger Action
</Menu.Item>
<Menu.Item disabled href="https://example.org/" icon="warning">
Expand Down
24 changes: 24 additions & 0 deletions src/components/Menu/Menu.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ const { Default } = composeStories(stories);
const { Opened, ...staticStories } = stories;

describe('<Menu />', () => {
afterEach(() => {
jest.resetAllMocks();
});
generateSnapshots(staticStories, {
getElement: async () => {
const user = userEvent.setup();
Expand Down Expand Up @@ -46,6 +49,27 @@ describe('<Menu />', () => {
expect(menuContainer.getAttribute('aria-activedescendant')).not.toBeNull();
});

it('handles onclick events when there is an href present', async () => {
// create a spy on the `log` method, and avoid calling it by setting the mock implementation to nothing
const consoleSpy = jest.spyOn(console, 'log').mockImplementation();
const user = userEvent.setup();
render(<Default />);
const triggerButton = await screen.findByRole('button');
await act(async () => {
await user.click(triggerButton);
});

await act(async () => {
await user.keyboard('{arrowdown}{arrowdown}{arrowdown}');
});

await act(async () => {
await user.keyboard('{enter}');
});

expect(consoleSpy).toHaveBeenCalledTimes(1);
});

it('should close menu on keyboard escape key', async () => {
const user = userEvent.setup();
render(<Default />);
Expand Down
6 changes: 2 additions & 4 deletions src/components/Menu/Menu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export type MenuItemProps = ExtractProps<typeof HeadlessMenu.Item> & {
*/
icon?: IconName;
/**
* Configurable action for the menu item action. If both `href` and `onClick` are used, `onClick` takes precedent.
* Configurable action for the menu item upon click
*/
onClick?: MouseEventHandler<HTMLAnchorElement>;
};
Expand Down Expand Up @@ -189,8 +189,6 @@ const MenuItem = ({
onClick,
...other
}: MenuItemProps) => {
// If we have an event handler, avoid navigation by discarding the href
const destinationUrl = !onClick ? href : undefined;
return (
<HeadlessMenu.Item {...other}>
{({ active, disabled }) => {
Expand All @@ -209,7 +207,7 @@ const MenuItem = ({
) : (
<a
className={clsx(styles['menu__item'])}
href={destinationUrl}
href={href}
onClick={onClick}
>
{listItemView}
Expand Down

0 comments on commit 971f189

Please sign in to comment.