Skip to content

Commit 03f6449

Browse files
committed
fix(emcn): compose consumer dismiss handlers instead of replacing the guard
ModalContent's own onEscapeKeyDown/onInteractOutside sit before the `{...props}` spread, so a consumer passing either replaced them — dropping both the dismissDisabled interlock and the floating-layer guard that keeps a popper dismissal from closing the modal and freezing the page. The TSDoc argued the guard had to live here for exactly that reason, then left the same spread able to defeat it. Both handlers are now destructured out of props and invoked after the guard, so the guard always runs and a consumer can still observe or extend the event. No consumer passes either today, so this was latent rather than live.
1 parent 947df6d commit 03f6449

2 files changed

Lines changed: 39 additions & 4 deletions

File tree

packages/emcn/src/components/chip-modal/chip-modal.test.tsx

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import { act, type ReactNode } from 'react'
55
import { createRoot, type Root } from 'react-dom/client'
66
import { afterEach, describe, expect, it, vi } from 'vitest'
7+
import { Modal, ModalContent, ModalHeader } from '../modal/modal'
78
import { ChipConfirmModal, ChipModal, ChipModalFooter, ChipModalHeader } from './chip-modal'
89

910
vi.mock('next/navigation', () => ({
@@ -147,6 +148,37 @@ describe('ChipModal dismissDisabled', () => {
147148
})
148149
})
149150

151+
describe('ModalContent dismissDisabled', () => {
152+
it('runs a consumer escape handler without letting it drop the guard', () => {
153+
const onOpenChange = vi.fn()
154+
const onEscapeKeyDown = vi.fn()
155+
mount(
156+
<Modal open onOpenChange={onOpenChange}>
157+
<ModalContent srTitle='Guarded' dismissDisabled onEscapeKeyDown={onEscapeKeyDown}>
158+
<input aria-label='Field' />
159+
</ModalContent>
160+
</Modal>
161+
)
162+
163+
pressEscape()
164+
expect(onEscapeKeyDown).toHaveBeenCalled()
165+
expect(onOpenChange).not.toHaveBeenCalled()
166+
})
167+
168+
it('disables the built-in ModalHeader close button', () => {
169+
const onOpenChange = vi.fn()
170+
mount(
171+
<Modal open onOpenChange={onOpenChange}>
172+
<ModalContent srTitle='Guarded' dismissDisabled>
173+
<ModalHeader>Title</ModalHeader>
174+
</ModalContent>
175+
</Modal>
176+
)
177+
178+
expect(closeButton().disabled).toBe(true)
179+
})
180+
})
181+
150182
describe('ChipConfirmModal pending', () => {
151183
it('holds every exit shut while the confirm runs', () => {
152184
const onOpenChange = vi.fn()

packages/emcn/src/components/modal/modal.tsx

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -469,10 +469,9 @@ export interface ModalContentProps
469469
* clicking outside, and `ModalHeader`'s close button. Descendants read it via
470470
* {@link useModalDismissDisabled}.
471471
*
472-
* The Radix paths are handled here rather than through a consumer-passed
473-
* `onEscapeKeyDown` / `onInteractOutside` because `{...props}` is spread after
474-
* this component's own handlers, so a consumer overriding either would
475-
* silently drop the floating-layer guard below.
472+
* A consumer's own `onEscapeKeyDown` / `onInteractOutside` runs after this
473+
* guard rather than replacing it, so neither the interlock nor the
474+
* floating-layer guard can be dropped by passing a handler.
476475
* @default false
477476
*/
478477
dismissDisabled?: boolean
@@ -497,6 +496,8 @@ const ModalContent = React.forwardRef<
497496
dismissDisabled = false,
498497
style,
499498
onOpenAutoFocus,
499+
onEscapeKeyDown,
500+
onInteractOutside,
500501
'aria-describedby': ariaDescribedBy,
501502
...props
502503
},
@@ -601,6 +602,7 @@ const ModalContent = React.forwardRef<
601602
// Radix reads `defaultPrevented`; stopPropagation alone would not block it.
602603
if (dismissDisabled) e.preventDefault()
603604
e.stopPropagation()
605+
onEscapeKeyDown?.(e)
604606
}}
605607
onPointerDown={(e) => {
606608
e.stopPropagation()
@@ -626,6 +628,7 @@ const ModalContent = React.forwardRef<
626628
if (dismissDisabled || hasOpenFloatingLayer()) {
627629
e.preventDefault()
628630
}
631+
onInteractOutside?.(e)
629632
}}
630633
onOpenAutoFocus={(event) => {
631634
// Radix fires this once when the (still invisible) Content

0 commit comments

Comments
 (0)