react-spectrum icon indicating copy to clipboard operation
react-spectrum copied to clipboard

The onAction handler is called twice when passed to MenuItem.

Open GuoXiaoyang opened this issue 1 year ago โ€ข 1 comments

Provide a general summary of the issue here

After we upgraded our react-aria to 3.33.1, the onAction handler which passed to MenuItem directly was broken. It is called twice but not once only.

I noticed the change was introduced in this pull request: https://github.com/adobe/react-spectrum/pull/5874/files#diff-2237da4ee227fcc1c1be60b57ee78479c8cbac8679abfc5de478689e90e0adaf. And the added test case is passed unexpectedly: https://github.com/adobe/react-spectrum/pull/5874/files#r1658273241

๐Ÿค” Expected Behavior?

The onAction handler is called only once when passed to MenuItem.

๐Ÿ˜ฏ Current Behavior

The onAction handler is called twice when passed to MenuItem.

๐Ÿ’ Possible Solution

No response

๐Ÿ”ฆ Context

No response

๐Ÿ–ฅ๏ธ Steps to Reproduce

https://github.com/adobe/react-spectrum/pull/5874/files#r1658273241 image

Version

3.33.1

What browsers are you seeing the problem on?

Chrome

If other, please specify.

No response

What operating system are you using?

macOS 14.5

๐Ÿงข Your Company/Team

No response

๐Ÿ•ท Tracking Issue

No response

GuoXiaoyang avatar Jun 28 '24 07:06 GuoXiaoyang

Thanks for reporting, it looks like

item.props.onAction
&&
props.onAction

We should check if any of the other collections suffer from this. We may just be missing some prioritization on calling them, an else if, instead of just an if.

https://github.com/adobe/react-spectrum/blob/b46d23b9919eaec8ab1f621b52beced82e88b6ca/packages/%40react-aria/menu/src/useMenuItem.ts#L135

snowystinger avatar Jun 29 '24 03:06 snowystinger