Added events to dialogue onClose prop method
Motivation
To provide more information to the onClose function to allow developers to add their own custom logic based on the event fired. Currently, there seems to be no great way to do this. An example scenario where this is necessary is working with react-toastify.
While wrapping the <ToastContainer .../> with <Portal></Portal> (as suggested here) tags allows the toasts to show up on top as expected, anytime they are clicked, the dialog is dismissed, which can lead to unexpected behaviour.
Here is an older discussion on this, note that this solution is not ideal as per MDN's recommendation to avoid using window.event.
Changes Made
New type is declared and exported:
export type DialogEventType = ReactMouseEvent | PointerEvent | FocusEvent | TouchEvent | KeyboardEvent
And it is used as an optional parameter for onClose
export type DialogProps<TTag extends ElementType = typeof DEFAULT_DIALOG_TAG> = Props<
TTag,
DialogRenderPropArg,
DialogPropsWeControl,
PropsForFeatures<typeof DialogRenderFeatures> & {
open?: boolean
onClose(value: boolean, event?: DialogEventType): void // <--- Added here
initialFocus?: MutableRefObject<HTMLElement | null>
role?: 'dialog' | 'alertdialog'
autoFocus?: boolean
__demoMode?: boolean
}
>
The latest updates on your projects. Learn more about Vercel for Git ↗︎
| Name | Status | Preview | Comments | Updated (UTC) |
|---|---|---|---|---|
| headlessui-react | ✅ Ready (Inspect) | Visit Preview | 💬 Add feedback | Jan 25, 2024 0:45am |
| headlessui-vue | ✅ Ready (Inspect) | Visit Preview | 💬 Add feedback | Jan 25, 2024 0:45am |
@RobinMalfait Sorry to ping you specifically, but it would be nice if I can get this PR through or get some feedback on this. Thanks.
I was actually just looking at doing something similar, but for a more limited scope. i.e. Having a close reason rather than a boolean (which is always false) would be very useful for scenarios when we want to disable Esc or outsideClick handling. This proposed solution would enable the same type of logic while also fixing other scenarios when things do appear on top of/outside of the dialog that get clicked.
@parkerholladay I agree, it would be nice if the good people maintaining this project provided some feedback or approved this PR. It has been sitting here for a while now...
Hey!
This has come up a few times and we do want to provide some information to the users, so I 100% get why you made the PR which I appreciate! However, we want to tackle this in a slightly different way and do this for most components so that the APIs are consistent.
I will open a PR where we do this for most components and make sure to link to the various PRs that wanted to introduce this as well so that they are all in a single spot.
Thanks regardless, I appreciate the contribution 👍
@RobinMalfait Thanks for getting to this. I understand, I figured you guys would rather make it consistent with everything else. Happy to help with this if you need me :)