Skip to content
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.

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

pressUp event is breaking the onClick / onPress callback #6224

Open
wingkwong opened this issue Apr 18, 2024 · 3 comments
Open

pressUp event is breaking the onClick / onPress callback #6224

wingkwong opened this issue Apr 18, 2024 · 3 comments

Comments

@wingkwong
Copy link

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

@snowystinger
Copy link
Member

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 #5291 ?

@wingkwong
Copy link
Author

@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 #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).

@snowystinger
Copy link
Member

Would #5874 address what you need? it enables onAction directly on MenuItems.

Is that what you're trying to do?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants