adobe / react-spectrum

A collection of libraries and tools that help you build adaptive, accessible, and robust user experiences.
https://react-spectrum.adobe.com
Apache License 2.0
12.58k stars 1.09k forks source link

react-aria focus management causes conflicts with other libraries #5799

Open cssinate opened 7 months ago

cssinate commented 7 months ago

Provide a general summary of the issue here

Perhaps this is just a case of "Yes, this is the way it is, and you should use our ecosystem as much as possible to prevent these sorts of issues," but I thought I'd point it out anyway. We have used a lot of radix-ui before react-aria really had its feet under it. As a result, a lot of our components are already from that library. But they're missing some components that react-aria provides, in this particular case, the useColorX hooks. I tried putting a useColorArea inside of a radix-ui Popover, and now it requires two clicks to close the Popover.

๐Ÿค” Expected Behavior?

I should have the option, when using react-aria components to not have the focus management change from other libraries.

๐Ÿ˜ฏ Current Behavior

When react-aria takes over focus, the radix-ui Popover component requires two clicks to close.

๐Ÿ’ Possible Solution

No response

๐Ÿ”ฆ Context

I'm not in a place where I'm prepared to switch our Popover component out for the one that comes with react-aria, but it does feel like there should be a way to make the two libraries play nice with each other.

๐Ÿ–ฅ๏ธ Steps to Reproduce

Steps are listed in the rendered view of the example: https://codesandbox.io/p/sandbox/react-aria-colorpicker-mwj453?file=%2Fsrc%2FApp.tsx

Version

"@react-aria/color": "~3.0.0-beta.29",

What browsers are you seeing the problem on?

Chrome

If other, please specify.

No response

What operating system are you using?

Windows 11

๐Ÿงข Your Company/Team

No response

๐Ÿ•ท Tracking Issue

No response

reidbarber commented 7 months ago

I'm not able to access that codesandbox. Can you double check that it's public?

cssinate commented 7 months ago

I'm not able to access that codesandbox. Can you double check that it's public?

A thousand apologies. Please try again now.

reidbarber commented 7 months ago

This looks like a bug, and I don't think it's focus management related, because the issue isn't there if you are focused via keyboard. I threw the useMove docs example into the Popover and saw the same issue (useColorArea uses that hook), so I think the issue is in there, and related to how mouse events are handled.

jaknas commented 7 months ago

We noticed the same issue happening when combining radix-ui popover with Calendar component from RAC. After narrowing down, the issue seems to happen after clicking any react-aria button inside the popover. See this narrowed down example.

Additionally, passing onPressStart={e => e.continuePropagation() seems to be fixing that problem. However that workaround is not possible when using for example CalendarCell.

Hopefully that will help y'all with finding the root cause.

snowystinger commented 7 months ago

I think this is an issue with radix-ui, not us. We should be able to stopPropagation on a button's pointerDown.

https://codesandbox.io/p/sandbox/react-aria-colorpicker-forked-3tmpt2?file=%2Fsrc%2FApp.tsx%3A28%2C35

possibly an issue here? https://github.com/radix-ui/primitives/blob/c31c97274ff357aea99afe6c01c1c8c58b6356e0/packages/react/dismissable-layer/src/DismissableLayer.tsx#L296 and compared here https://github.com/radix-ui/primitives/blob/c31c97274ff357aea99afe6c01c1c8c58b6356e0/packages/react/dismissable-layer/src/DismissableLayer.tsx#L233 there is no matching pointerDown after the capture to clear that flag?

I do understand the other side to this as well. We've run into instances where people do something to an event we rely on. This one seems easy enough to deal with, though that I'm ok pushing back on it. I doubt we're the only ones calling stopPropagation on a click event in a component. It prevents the components above it from handling an event we've already handled.