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

pressUp event is breaking the onClick / onPress callback

Open wingkwong opened this issue 1 year ago โ€ข 3 comments

Provide a general summary of the issue here

Hello. I'm creating this issue on behalf of NextUI. Recently we've upgraded react-aria to the latest versions. For some reasons, the pressUp event is breaking the onClick / onPress callback.

๐Ÿค” Expected Behavior?

onPress / onClick events can be triggered

๐Ÿ˜ฏ Current Behavior

onPress / onClick events are not working on menu based components like Listbox / Dropdown / Menu / etc

๐Ÿ’ Possible Solution

To fix this we had to implement our own use-menu-item hook as a workaround and control the pressUp event, in addition, we passed down the properties to control the press events directly. The full PR can be found here.

See use-menu-item.ts

image

๐Ÿ”ฆ Context

No response

๐Ÿ–ฅ๏ธ Steps to Reproduce

https://codesandbox.io/p/sandbox/admiring-gagarin-2y9qgk?file=%2Fsrc%2FApp.js%3A14%2C11

Version

"@react-aria/menu": "^3.13.1"

What browsers are you seeing the problem on?

Firefox, Chrome, Safari, Microsoft Edge

If other, please specify.

No response

What operating system are you using?

MacOS

๐Ÿงข Your Company/Team

NextUI

๐Ÿ•ท Tracking Issue

No response

wingkwong avatar Apr 18 '24 05:04 wingkwong

Hey, thanks for the issue. I'm not entirely sure what the bug is. There seem to be a few things happening and not much in the way of reproduction steps. It'd be helpful to outline what you're trying to accomplish.

One thing I see is that you're attaching onPress directly to Item, which we don't support. Please use onSelectionChange.

It also looks like you're implementing a closeOnSelect. You shouldn't need to do that. If selectionMode is 'none' or 'single', this is already the behavior. If it's multiple selection, then it should stay open even when selection changes.

Is this related to https://github.com/adobe/react-spectrum/pull/5291 ?

snowystinger avatar Apr 18 '24 06:04 snowystinger

@snowystinger Thanks for the response. The original issue from our side is that the onPress function in menu item (i.e. NextUI DropdownItem) is not working suddenly after upgrading react-aria.

One thing I see is that you're attaching onPress directly to Item, which we don't support. Please use onSelectionChange.

My bad. I've removed the onPress in Item to avoid confusion. The onPress should be in in MenuItem instead.

It also looks like you're implementing a closeOnSelect. You shouldn't need to do that. If selectionMode is 'none' or 'single', this is already the behavior. If it's multiple selection, then it should stay open even when selection changes.

We didn't implement new stuff but just added the timeout function on top of your useMenuItem.ts implementation as shown in above screenshot.

Is this related to https://github.com/adobe/react-spectrum/pull/5291 ?

Based on the findings, we previously identified the root case was coming from useMenuItem. We've switched to the latest @react-aria/interactions as well so it may not relate to this PR. But currently we removed usePress from our code and passed down the properties to control the press events directly in our own customised useMenuItem (ref: here).

wingkwong avatar Apr 18 '24 06:04 wingkwong

Would https://github.com/adobe/react-spectrum/pull/5874 address what you need? it enables onAction directly on MenuItems.

Is that what you're trying to do?

snowystinger avatar Apr 18 '24 20:04 snowystinger