radix-ui / primitives

Radix Primitives is an open-source UI component library for building high-quality, accessible design systems and web apps. Maintained by @workos.
https://radix-ui.com/primitives
MIT License
15.44k stars 775 forks source link

[Toast] Ability to remove `document` event listeners #1547

Open jorroll opened 2 years ago

jorroll commented 2 years ago

Feature request

Overview

Currently, the Toast package adds an event listener to the document for managing hotkeys internally.

https://github.com/radix-ui/primitives/blob/3e5644edadcded918db177368c3fe9654ce9885f/packages/react/toast/src/Toast.tsx#L169-L170

I'd like the ability to disable this so that I have full control over Toast hotkey handling. One option would be simply passing an empty array to the hotkeys prop. Ideally, Toast would check the length of the hotkey prop before adding an event handler to document and, if hotkey was length 0, then no handler would be added.

Note: the hotkey listeners were implemented using Array#every() which returns true for an empty array.

Who does this impact? Who is this for?

Users of the Toast package.

Additional context

In my application, I have a sophisticated, custom service which is responsible for handling all hotkeys. I'd like to manage toast hotkeys that way.

jorroll commented 2 years ago

Additionally, it looks like there's no ability to disable the Toast package's handling of the Escape key.

Basically, I'm requesting the ability to stop the Toast package from adding any event listeners outside of the Viewport component.

benoitgrelard commented 2 years ago

Hey @jorroll,

What is the reason you need to take over this? You would have to re-implement the hotkey and focus behaviour yourself if you opted out.

Additionally, it looks like there's no ability to disable the Toast package's handling of the Escape key.

There's an onEscapeKeydown prop in which you can event.preventDefault().

jorroll commented 2 years ago

What is the reason you need to take over this?

I have a sophisticated service which handles all document event listeners in my application. The Toast component was conflicting with it. For example, I would try to press escape to perform some navigation within the app but then the Toast notifications would disappear.

There's an onEscapeKeydown prop in which you can event.preventDefault()

Prevent default isn't an option since that would affect other hotkeys and listeners subscribed to Escape.

As a side note, the Radix UI packages are, in general, excellent. I'm especially fond of Slot. Thanks for all the work that's gone into this library!

benoitgrelard commented 2 years ago

Oh ya, just tested. It works the way I thought it did (i.e. it affects other listeners). Your comment made me think that maybe I've had a huge misunderstanding of javascript event listeners for several years now 😅.

No you're right, it's just that in other places some of these cancellable events we have are custom events we raise, so those wouldn't affect any other events. In this case we are piggy-backing on the keyboard event so it would.

jjenzz commented 2 years ago

In this case we are piggy-backing on the keyboard event so it would.

oh, was that a mistake on our part? seems it should be a CustomEvent no?