Skip to content

Commit 947df6d

Browse files
committed
improvement(emcn): let a modal refuse every dismissal while an action runs
ChipConfirmModal's docs promised "a single dismiss path shared by the header X / dismiss button / Escape … and disabling dismiss while the confirm is in flight". Only the dismiss button was ever guarded — Escape, outside-click and the header X all still closed a confirmation mid-delete. Two knowledge-base connector modals had the same shape: they guarded onOpenChange against a pending save, then handed the header X a direct onOpenChange(false) that skipped the guard. A modal now states the interlock once, as `dismissDisabled` on ChipModal or ModalContent, and the primitive holds all four exits shut. ModalContent owns the Radix paths because `{...props}` is spread after its own handlers, so a consumer-passed onEscapeKeyDown/onInteractOutside would silently drop the floating-layer guard; it publishes the flag through a context that ChipModalHeader, ChipModalFooter and ModalHeader read. The two narrow props compose with `||`, so an explicit `true` still disables a single button and an explicit `false` cannot punch a hole in the root's guarantee. Also turns on `turbo run type-check` for every workspace. packages/emcn, packages/utils, apps/desktop and apps/docs had no type check in CI at all — only @sim/realtime did — and apps/sim's source was covered solely as a side effect of `next build`. All 23 workspaces pass today, so it lands green.
1 parent 6c10ac2 commit 947df6d

8 files changed

Lines changed: 322 additions & 53 deletions

File tree

.github/workflows/test-build.yml

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -178,8 +178,14 @@ jobs:
178178
fi
179179
bun run check:migrations "$BASE_REF"
180180
181-
- name: Type-check realtime server
182-
run: bunx turbo run type-check --filter=@sim/realtime
181+
# Every workspace, not just realtime. packages/emcn, packages/utils,
182+
# apps/desktop and apps/docs had no type check in CI at all; apps/sim's
183+
# source was covered only as a side effect of `next build` in the separate
184+
# Build App job. Note this does NOT cover apps/sim's tests — its tsconfig
185+
# excludes *.test.ts(x), and including them today surfaces ~2.2k errors,
186+
# so that is its own cleanup rather than a gate to switch on here.
187+
- name: Type-check all workspaces
188+
run: bunx turbo run type-check
183189

184190
# cloud-review-tools.test.ts runs the real helper on the runner, which shells
185191
# out to rg. Blacksmith's image ships it, GitHub's doesn't.

apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/add-connector-modal/add-connector-modal.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -235,9 +235,10 @@ export function AddConnectorModal({
235235
<>
236236
<ChipModal
237237
open={open}
238-
onOpenChange={(val) => !isCreating && onOpenChange(val)}
238+
onOpenChange={onOpenChange}
239239
srTitle={step === 'select-type' ? 'Connect Source' : `Configure ${connectorConfig?.name}`}
240240
size='md'
241+
dismissDisabled={isCreating}
241242
>
242243
<ChipModalHeader onClose={() => onOpenChange(false)}>
243244
{step === 'configure' ? (
@@ -428,7 +429,6 @@ export function AddConnectorModal({
428429
{step === 'configure' && (
429430
<ChipModalFooter
430431
onCancel={() => onOpenChange(false)}
431-
cancelDisabled={isCreating}
432432
primaryAction={{
433433
label: isCreating ? 'Connecting…' : 'Connect & Sync',
434434
onClick: handleSubmit,

apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/edit-connector-modal/edit-connector-modal.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -269,9 +269,10 @@ export function EditConnectorModal({
269269
return (
270270
<ChipModal
271271
open={open}
272-
onOpenChange={(val) => !isSaving && onOpenChange(val)}
272+
onOpenChange={onOpenChange}
273273
srTitle={`Edit ${displayName}`}
274274
size='md'
275+
dismissDisabled={isSaving}
275276
>
276277
<ChipModalHeader icon={Icon ?? null} onClose={() => onOpenChange(false)}>
277278
Edit {displayName}
@@ -312,7 +313,6 @@ export function EditConnectorModal({
312313
{activeTab === 'settings' && (
313314
<ChipModalFooter
314315
onCancel={() => onOpenChange(false)}
315-
cancelDisabled={isSaving}
316316
primaryAction={{
317317
label: isSaving ? 'Saving…' : 'Save',
318318
onClick: handleSave,
Lines changed: 187 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,187 @@
1+
/**
2+
* @vitest-environment jsdom
3+
*/
4+
import { act, type ReactNode } from 'react'
5+
import { createRoot, type Root } from 'react-dom/client'
6+
import { afterEach, describe, expect, it, vi } from 'vitest'
7+
import { ChipConfirmModal, ChipModal, ChipModalFooter, ChipModalHeader } from './chip-modal'
8+
9+
vi.mock('next/navigation', () => ({
10+
usePathname: () => '/workspace/workspace-1/home',
11+
}))
12+
13+
let root: Root | null = null
14+
let container: HTMLDivElement | null = null
15+
16+
function mount(ui: ReactNode) {
17+
;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true
18+
container = document.createElement('div')
19+
document.body.appendChild(container)
20+
root = createRoot(container)
21+
act(() => root?.render(ui))
22+
}
23+
24+
afterEach(() => {
25+
if (root) act(() => root?.unmount())
26+
container?.remove()
27+
root = null
28+
container = null
29+
})
30+
31+
/** The dialog panel Radix renders, which owns the Escape/outside-click handlers. */
32+
function dialog(): HTMLElement {
33+
const node = document.querySelector<HTMLElement>('[role="dialog"]')
34+
if (!node) throw new Error('Dialog did not render')
35+
return node
36+
}
37+
38+
function buttonByText(text: string): HTMLButtonElement {
39+
const match = Array.from(document.querySelectorAll('button')).find((button) =>
40+
button.textContent?.includes(text)
41+
)
42+
if (!match) throw new Error(`No button containing "${text}"`)
43+
return match as HTMLButtonElement
44+
}
45+
46+
function closeButton(): HTMLButtonElement {
47+
const match = Array.from(document.querySelectorAll('button')).find((button) =>
48+
button.querySelector('.sr-only')?.textContent?.includes('Close')
49+
)
50+
if (!match) throw new Error('Close button did not render')
51+
return match as HTMLButtonElement
52+
}
53+
54+
function pressEscape() {
55+
act(() => {
56+
dialog().dispatchEvent(
57+
new KeyboardEvent('keydown', { key: 'Escape', bubbles: true, cancelable: true })
58+
)
59+
})
60+
}
61+
62+
function Harness({
63+
onOpenChange,
64+
dismissDisabled,
65+
}: {
66+
onOpenChange: (open: boolean) => void
67+
dismissDisabled?: boolean
68+
}) {
69+
return (
70+
<ChipModal
71+
open
72+
onOpenChange={onOpenChange}
73+
srTitle='Test modal'
74+
dismissDisabled={dismissDisabled}
75+
>
76+
<ChipModalHeader onClose={() => onOpenChange(false)}>Title</ChipModalHeader>
77+
<ChipModalFooter
78+
onCancel={() => onOpenChange(false)}
79+
primaryAction={{ label: 'Save', onClick: () => {} }}
80+
/>
81+
</ChipModal>
82+
)
83+
}
84+
85+
describe('ChipModal dismissDisabled', () => {
86+
it('closes through every path when not set', () => {
87+
const onOpenChange = vi.fn()
88+
mount(<Harness onOpenChange={onOpenChange} />)
89+
90+
expect(closeButton().disabled).toBe(false)
91+
expect(buttonByText('Cancel').disabled).toBe(false)
92+
93+
pressEscape()
94+
expect(onOpenChange).toHaveBeenCalledWith(false)
95+
})
96+
97+
// Outside-click is guarded by the same flag but jsdom cannot drive Radix's
98+
// outside-interaction path, so asserting it here could never fail.
99+
it('blocks the close button, Cancel and Escape when set', () => {
100+
const onOpenChange = vi.fn()
101+
mount(<Harness onOpenChange={onOpenChange} dismissDisabled />)
102+
103+
expect(closeButton().disabled).toBe(true)
104+
expect(buttonByText('Cancel').disabled).toBe(true)
105+
106+
pressEscape()
107+
expect(onOpenChange).not.toHaveBeenCalled()
108+
})
109+
110+
// Either flag disables: an explicit `false` must not re-enable a button whose
111+
// click Radix has already been told to ignore.
112+
it('cannot be re-enabled by an explicit closeDisabled or cancelDisabled of false', () => {
113+
const onOpenChange = vi.fn()
114+
mount(
115+
<ChipModal open onOpenChange={onOpenChange} srTitle='Test modal' dismissDisabled>
116+
<ChipModalHeader onClose={() => onOpenChange(false)} closeDisabled={false}>
117+
Title
118+
</ChipModalHeader>
119+
<ChipModalFooter
120+
onCancel={() => onOpenChange(false)}
121+
cancelDisabled={false}
122+
primaryAction={{ label: 'Save', onClick: () => {} }}
123+
/>
124+
</ChipModal>
125+
)
126+
127+
expect(closeButton().disabled).toBe(true)
128+
expect(buttonByText('Cancel').disabled).toBe(true)
129+
})
130+
131+
it('still lets an explicit true disable a button on its own', () => {
132+
const onOpenChange = vi.fn()
133+
mount(
134+
<ChipModal open onOpenChange={onOpenChange} srTitle='Test modal'>
135+
<ChipModalHeader onClose={() => onOpenChange(false)} closeDisabled>
136+
Title
137+
</ChipModalHeader>
138+
<ChipModalFooter
139+
onCancel={() => onOpenChange(false)}
140+
primaryAction={{ label: 'Save', onClick: () => {} }}
141+
/>
142+
</ChipModal>
143+
)
144+
145+
expect(closeButton().disabled).toBe(true)
146+
expect(buttonByText('Cancel').disabled).toBe(false)
147+
})
148+
})
149+
150+
describe('ChipConfirmModal pending', () => {
151+
it('holds every exit shut while the confirm runs', () => {
152+
const onOpenChange = vi.fn()
153+
mount(
154+
<ChipConfirmModal
155+
open
156+
onOpenChange={onOpenChange}
157+
title='Delete key'
158+
text='This cannot be undone.'
159+
confirm={{ label: 'Delete', onClick: () => {}, pending: true, pendingLabel: 'Deleting...' }}
160+
/>
161+
)
162+
163+
expect(closeButton().disabled).toBe(true)
164+
expect(buttonByText('Cancel').disabled).toBe(true)
165+
expect(buttonByText('Deleting...').disabled).toBe(true)
166+
167+
pressEscape()
168+
expect(onOpenChange).not.toHaveBeenCalled()
169+
})
170+
171+
it('dismisses normally when the confirm is idle', () => {
172+
const onOpenChange = vi.fn()
173+
mount(
174+
<ChipConfirmModal
175+
open
176+
onOpenChange={onOpenChange}
177+
title='Delete key'
178+
text='This cannot be undone.'
179+
confirm={{ label: 'Delete', onClick: () => {} }}
180+
/>
181+
)
182+
183+
expect(closeButton().disabled).toBe(false)
184+
act(() => closeButton().click())
185+
expect(onOpenChange).toHaveBeenCalledWith(false)
186+
})
187+
})

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

Lines changed: 52 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ import { ChipInput } from '../chip-input/chip-input'
5151
import { ChipSwitch } from '../chip-switch/chip-switch'
5252
import { ChipTextarea } from '../chip-textarea/chip-textarea'
5353
import { Label } from '../label/label'
54-
import { Modal, ModalContent } from '../modal/modal'
54+
import { Modal, ModalContent, useModalDismissDisabled } from '../modal/modal'
5555
import { Tooltip } from '../tooltip/tooltip'
5656

5757
/**
@@ -108,6 +108,14 @@ export interface ChipModalProps {
108108
size?: 'sm' | 'md' | 'lg' | 'xl' | 'full'
109109
/** Optional className forwarded to the outer panel ring. */
110110
className?: string
111+
/**
112+
* Refuses every exit while an action is in flight — Escape, outside-click,
113+
* the header close button, and the footer Cancel. Stating it once here is the
114+
* point: disabling only the buttons leaves Escape and outside-click open,
115+
* which reads as handled without being handled.
116+
* @default false
117+
*/
118+
dismissDisabled?: boolean
111119
children?: React.ReactNode
112120
}
113121

@@ -124,13 +132,20 @@ function ChipModal({
124132
srTitle = 'Dialog',
125133
size = 'md',
126134
className,
135+
dismissDisabled = false,
127136
children,
128137
}: ChipModalProps) {
129138
const submitRef = React.useRef<ChipModalSubmit | null>(null)
130139
return (
131140
<ChipModalSubmitContext.Provider value={submitRef}>
132141
<Modal open={open} onOpenChange={onOpenChange}>
133-
<ModalContent bare showClose={false} srTitle={srTitle} size={size}>
142+
<ModalContent
143+
bare
144+
showClose={false}
145+
srTitle={srTitle}
146+
size={size}
147+
dismissDisabled={dismissDisabled}
148+
>
134149
<div
135150
className={cn(
136151
'flex min-h-0 w-full flex-col rounded-xl border border-[var(--border-muted)] bg-[var(--surface-4)] p-[3px] shadow-[var(--shadow-overlay)] dark:bg-[var(--surface-5)]',
@@ -154,7 +169,11 @@ export interface ChipModalHeaderProps extends React.HTMLAttributes<HTMLDivElemen
154169
icon?: React.ComponentType<{ className?: string }> | null
155170
/** Invoked when the trailing close button is activated. Always rendered. */
156171
onClose: () => void
157-
/** Disables the trailing close button while an operation is in flight. */
172+
/**
173+
* Disables the trailing close button. Combines with
174+
* {@link ChipModalProps.dismissDisabled}, which also blocks Escape and
175+
* outside-click — prefer that for an in-flight operation.
176+
*/
158177
closeDisabled?: boolean
159178
/** Accessible label for the close button. */
160179
closeAriaLabel?: string
@@ -171,32 +190,35 @@ const ChipModalHeader = React.forwardRef<HTMLDivElement, ChipModalHeaderProps>(
171190
children,
172191
icon: Icon = null,
173192
onClose,
174-
closeDisabled = false,
193+
closeDisabled,
175194
closeAriaLabel = 'Close',
176195
...props
177196
},
178197
ref
179-
) => (
180-
<div ref={ref} className={cn('flex flex-col', className)} {...props}>
181-
<div className='flex min-w-0 items-center justify-between gap-2 px-4 pt-3'>
182-
<div className='flex min-w-0 items-center gap-2'>
183-
{Icon ? <Icon className={chipContentIconClass} /> : null}
184-
<span className={chipContentLabelClass}>{children}</span>
198+
) => {
199+
const dismissDisabled = useModalDismissDisabled()
200+
return (
201+
<div ref={ref} className={cn('flex flex-col', className)} {...props}>
202+
<div className='flex min-w-0 items-center justify-between gap-2 px-4 pt-3'>
203+
<div className='flex min-w-0 items-center gap-2'>
204+
{Icon ? <Icon className={chipContentIconClass} /> : null}
205+
<span className={chipContentLabelClass}>{children}</span>
206+
</div>
207+
<Button
208+
type='button'
209+
variant='ghost'
210+
onClick={onClose}
211+
disabled={closeDisabled || dismissDisabled}
212+
className='relative size-[14px] flex-shrink-0 p-0 before:absolute before:inset-[-14px] before:content-[""]'
213+
>
214+
<X className='size-[14px] text-[var(--text-icon)]' />
215+
<span className='sr-only'>{closeAriaLabel}</span>
216+
</Button>
185217
</div>
186-
<Button
187-
type='button'
188-
variant='ghost'
189-
onClick={onClose}
190-
disabled={closeDisabled}
191-
className='relative size-[14px] flex-shrink-0 p-0 before:absolute before:inset-[-14px] before:content-[""]'
192-
>
193-
<X className='size-[14px] text-[var(--text-icon)]' />
194-
<span className='sr-only'>{closeAriaLabel}</span>
195-
</Button>
218+
<ChipModalSeparator className='mt-3' />
196219
</div>
197-
<ChipModalSeparator className='mt-3' />
198-
</div>
199-
)
220+
)
221+
}
200222
)
201223

202224
ChipModalHeader.displayName = 'ChipModalHeader'
@@ -924,6 +946,10 @@ export interface ChipModalFooterProps {
924946
* Disables the Cancel button. Set this while a primary/secondary action is
925947
* in flight (e.g. an async delete or save) so the user cannot dismiss the
926948
* modal and assume the operation was aborted while the mutation keeps running.
949+
*
950+
* This covers the Cancel button only. For an in-flight operation reach for
951+
* {@link ChipModalProps.dismissDisabled} instead, which also blocks Escape,
952+
* outside-click and the header's X.
927953
* @default false
928954
*/
929955
cancelDisabled?: boolean
@@ -1025,6 +1051,7 @@ function ChipModalFooter({
10251051
primaryAdjacentAction,
10261052
secondaryActions,
10271053
}: ChipModalFooterProps) {
1054+
const dismissDisabled = useModalDismissDisabled()
10281055
const showsDisabledTooltip = Boolean(primaryAction.disabled && primaryAction.disabledTooltip)
10291056

10301057
/**
@@ -1079,7 +1106,7 @@ function ChipModalFooter({
10791106
}
10801107
>
10811108
{hideCancel ? null : (
1082-
<Chip onClick={onCancel} disabled={cancelDisabled}>
1109+
<Chip onClick={onCancel} disabled={cancelDisabled || dismissDisabled}>
10831110
Cancel
10841111
</Chip>
10851112
)}
@@ -1303,6 +1330,7 @@ function ChipConfirmModal({
13031330
onOpenChange={onOpenChange}
13041331
size={size}
13051332
srTitle={srTitle ?? (typeof title === 'string' ? title : 'Confirm')}
1333+
dismissDisabled={confirm.pending}
13061334
>
13071335
<ChipModalHeader icon={icon} onClose={dismiss}>
13081336
{title}

0 commit comments

Comments
 (0)