Add `capture` option for event handler?

[ingver]

I faced the problem of inconsistent event handling. It appears in my SPA, when I navigate from route with component with dropdown that uses useOnClickOutside hook, and then navigate back. Component is being totally unmount during navigation.

When it is mounted back, useOnClickOutside adds event listener document again. I click on dropdown-list option, state of component changes, then list completely rerendered. And somehow mousedown event for dropdown option fired before mousedown event on document. Between these to events dom node is being removed, so onClickOutside triggered.

I tried to provide { capture: true } to mousedown event listener for document and now it's ok. So I think capture option should be provided by default, or at least via optional argument to hook. Is there any downside of this solution?

[Andarist]

Could you prepare a runnable repro case for this? I'd like to understand the problem first before rushing into changing the implementation

[ingver]

Here it is. I've told that my case was that useOnClickOutside was triggered while interacting with component only after navigating out from page with that component and then navigating back. But I've been able to reproduce the same behavior without need of navigating around. The component, MultiSelect, changes its available options list through state variable, so when you click on option, it causes a rerender, then useOnClickOutside is triggered.

[Andarist]

Hm, I see your problem - by the time onClickOutside handler get called your node is already detached from the DOM. I'm not sure if using capture phase by default would make things much better - probably other use cases would break.

From the library's point of view the only thing I see I could do is to give away control over adding/removing the document listeners to the user (optionally). It would make this lib way more complex that it probably should be though 🤔

On the other hand - why do u use onMouseDown in your MultiSelect? I believe it really should be onClick, it would give better UX and would support touch devices better out of the box.

[ingver]

From the library's point of view the only thing I see I could do is to give away control over adding/removing the document listeners to the user (optionally). It would make this lib way more complex that it probably should be though 🤔

I've thought about adding optional argument to hook, where user could specify options for event-listeners (not adding/removing listeners themeselves). It's would be useful for me, but I don't know about others, though :)

On the other hand - why do u use onMouseDown in your MultiSelect? I believe it really should be onClick, it would give better UX and would support touch devices better out of the box.

That's because I need to manage focus (I don't do that in the example anyway). When option is clicked, "input"-div is blurred, and I can't prevent this, because click-event is fired before blur-event.

[addlistener]

I also need it. Because there are heavy mouse interactions in the app and events are stopPropagated everywhere. Events just cannot reach the document

Compatible update, easy as a one-liner?