`onClick` propagation with `usePress`

[alexichristakis]

📝 Feedback

We rely on onClick events propagating through our dom structure. Sometimes the source of the event will be from a component using react-aria's useButton hook, but it appears that usePress stops the propagation. This forces us to stub stopPropagation for all events passed into react-aria.

We'd like some context on why usePress implements this behavior.

🔦 Context

Event bubbling is necessary in Retool's editor to support selecting interactive components. We've been very happy leveraging react-aria to improve our accessibility, but are confused by the decision to prevent events from bubbling.

💻 Code Sample

https://github.com/adobe/react-spectrum/blob/main/packages/@react-aria/interactions/src/usePress.ts#L242

[devongovett]

Yeah, there are a number of reasons. Mainly we've found that in general relying on bubbling is more problematic than not, i.e. there are more cases where you don't want bubbling than when you do. We've found that in most cases an event should be handled in a single place, and allowing events to bubble after they've been handled leads to bugs. I wrote a twitter thread a while back with some examples and context: https://twitter.com/devongovett/status/1371549410245640195.

One workaround we usually recommend is to use capture phase listeners (i.e. onClickCapture) rather than relying on bubbling. This will allow the outer component to handle the event first, before it gets to the child. Would that work for you?

I'm curious to hear more about your use case. We do support a continuePropagation method on some of our events, but not yet in usePress. We'd consider adding it if necessary (i.e. capture phase listeners don't work for you). Could you give us more details?

[cawabunga]

Some 3rd party services rely on bubbling. E.g. walkthrough providers track clicking on element to transit to the next tour step. I totally understand and share your point, but sometimes it might be convenient to have an option for propagation.

Here is a temporary solution:

import { useCallback, MouseEventHandler, useRef } from 'react'

export const usePropagatableClickHandler = <T>(onClick: MouseEventHandler<T> | undefined) => {
  const isEventRealRef = useRef(true)

  const handleClick: MouseEventHandler<T> = useCallback((event) => {
    const { target, nativeEvent } = event
    const clonedNativeEvent = new MouseEvent('click', nativeEvent)

    if (!isEventRealRef.current) {
      isEventRealRef.current = true
      return
    }

    onClick?.(event)

    if (event.isPropagationStopped()) {
      isEventRealRef.current = false
      target.dispatchEvent(clonedNativeEvent)
    }
  }, [onClick])

  return handleClick
}

// usage
const { buttonProps } = useButton(props, ref)
const onClick = usePropagatableClickHandler(buttonProps.onClick)
<button ref={ref} {...buttonProps} onClick={onClick} />

[cscheffauer]

@devongovett It would be also useful for use cases we are facing. Propagation of events to the top is definitely something where 3rd party services are relying on, so stopping the propagation blocks us from using the library in combination with other libraries and doesn't allow for flexibility in terms of event handling.
Is this something you are looking into working on? :)

[tgelu]

Just want to contribute with another real-world use case: We're using this library to detect when a user becomes idle: https://idletimer.dev/ which works by tracking a bunch of events on the document. Everything works fine except that all components implemented with usePress eat up the click. All clicks o those components are therefore not captured as user activity and we can have idle users that kept clicking buttons/checkboxes etc. :)

Of course, it isn't react-aria's problem that this occurs, but I think this illustrates that event propagation is enough of a foundational web thing that interfering with it at a large scale leads to unexpected results in real life.

[snowystinger]

@tgelu have you considered using capture phase for events? That way it shouldn't matter what we do, you should be unaffected.

[tgelu]

I think my message was a mistake. That library does use capture for events. I had a different issue that caused this 😅 - so, nevermind, carry on capturing

[aramvr]

@devongovett I understand that there may be reasons to stop event propagation, but if there is any reason why not to, then it should not be the default behavior.

One real-life scenario where I have a major issue with stopping propagation is when dealing with complex canvas functionalities that rely on detecting outside clicks via the mousedown event on the document. I'm sure there are other similar cases where stopping propagation causes problems.

You may suggest handling events inside the onMouseDown function of the button, but this approach may not be feasible in cases where the application can be embedded in various contexts with multiple spectrum buttons.

Moreover, if React does not stop event propagation, then it is not clear why Spectrum should do so. Why have e.continuePropagation() if there are already e.stopPropagation that could be used when necessary?

In fact, I have not seen this behavior in any other design system that I am aware of, so I kindly request you to reconsider this decision. If you are willing to do so, I would be happy to open MRs to discontinue stopping propagation by default.

[devongovett]

One real-life scenario where I have a major issue with stopping propagation is when dealing with complex canvas functionalities that rely on detecting outside clicks via the mousedown event on the document.

You should use a capturing listener for this so the document level event fires before the event handler on the target. Otherwise you will have all sorts of problems if anything, spectrum or otherwise, stops propagation. Propagation can be stopped for any number of good reasons, by any element on the page, so relying on it not being stopped at a global event will be very brittle. Capturing listeners solve this.

[aramvr]

at this point, it's not possible to listen mousedown event with capturing when the target element is spectrum button, because it stops propagation inside pointerdown event, sօ seems it does not trigger mousedown event afterward.