From bc4f9949bde1fae98e31575c2d71ba3e756cd923 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sun, 23 Aug 2026 20:00:06 -0700 Subject: [PATCH 1/2] improvement(ui): standardize modal default actions --- .claude/rules/emcn-components.md | 10 + apps/sim/app/(auth)/login/login-form.tsx | 3 - .../components/auth-modal/auth-modal.tsx | 13 +- .../delete-confirm-modal.tsx | 1 + .../components/share-modal/share-modal.tsx | 1 + .../workspace/[workspaceId]/files/files.tsx | 1 + .../document-tags-modal.tsx | 3 +- .../[workspaceId]/knowledge/[id]/base.tsx | 1 + .../base-tags-modal/base-tags-modal.tsx | 2 + .../delete-knowledge-base-modal.tsx | 1 + .../execution-snapshot/execution-snapshot.tsx | 36 +- .../snapshot-boundary.test.tsx | 26 +- .../execution-snapshot/snapshot-boundary.tsx | 44 +- .../settings/components/admin/admin.tsx | 1 + .../settings/components/browser/browser.tsx | 1 + .../components/byok/byok-key-manager.tsx | 4 - .../components/delete-account-modal.tsx | 1 + .../inbox-settings-tab/inbox-settings-tab.tsx | 2 +- .../mcp-server-form-modal.tsx | 5 +- .../transfer-ownership-dialog.tsx | 1 + .../[workspaceId]/tables/[tableId]/table.tsx | 2 + .../import-csv-dialog/import-csv-dialog.tsx | 1 + .../workspace/[workspaceId]/tables/tables.tsx | 1 + .../components/general/general.tsx | 53 +- .../components/deploy-modal/deploy-modal.tsx | 519 +++++++++--------- .../panel/components/deploy/deploy.tsx | 1 + .../w/[workflowId]/components/panel/panel.tsx | 1 + .../components/delete-modal/delete-modal.tsx | 6 + .../components/group-detail.tsx | 2 +- .../components/credential-group-details.tsx | 1 + .../components/custom-block-detail.tsx | 1 + .../fork-workspace-modal.tsx | 6 - .../components/chip-modal/chip-modal.test.tsx | 468 +++++++++++++++- .../src/components/chip-modal/chip-modal.tsx | 441 ++++++++++----- .../emcn/src/components/combobox/combobox.tsx | 5 + packages/emcn/src/components/index.ts | 3 + .../emcn/src/components/modal/auto-focus.ts | 6 +- 37 files changed, 1173 insertions(+), 501 deletions(-) diff --git a/.claude/rules/emcn-components.md b/.claude/rules/emcn-components.md index c5548d327df..0affdd95c02 100644 --- a/.claude/rules/emcn-components.md +++ b/.claude/rules/emcn-components.md @@ -33,6 +33,16 @@ The menu surface intentionally diverges from the pill: `dropdown-menu.tsx` items - **`ChipTimePicker`** — minute-granular time sibling of `ChipDatePicker`, a `ChipInput` that leniently parses typed input (`9:47`, `947`, `2:05pm`, `14:30`), commits on Enter/blur, and re-renders the canonical `9:47 AM` label. - **`DropdownMenu`** — the canonical context/action menu (Radix-backed). Not a chip, but the standard menu for command/action lists; reach for it instead of a hand-rolled popover. Its surface intentionally diverges from the chip pill (`text-small`, `gap-2`) — keep them distinct. For a pill that opens a value picker, use `ChipDropdown`/`ChipSelect` instead. +## Modal keyboard defaults + +Declare keyboard intent on the action-owning primitive; never add document-level or per-callsite Enter listeners. + +- `ChipModalFooter` defaults to `defaultAction='primary'`. A plain Enter in a canonical single-line field or a custom plain input invokes the enabled primary action. Use `'none'` when submission must require an explicit click, such as an irreversible destructive action or an editor whose nested control owns Enter. Use `'dismiss'` only when dismissal is genuinely the modal's default decision. +- `ChipConfirmModal` fails safe with `defaultAction='dismiss'`. Opt into `'confirm'` only for an audited, reversible or non-destructive decision. Use `'none'` for typed confirmations and severe account, ownership, or access changes. Button color never determines keyboard behavior. +- Textareas, native forms, buttons, links, comboboxes, menus, listboxes, tag/email inputs, IME composition, modified Enter, and disabled or pending actions retain their native behavior. A native form remains the sole submission path so browser validation is not bypassed. +- A custom field containing a search, token editor, or another input that owns Enter must set `submitOnEnter={false}` on `ChipModalField`. Do not attach a duplicate `onKeyDown` handler merely to call the footer action. +- Initial focus goes to the first visible editable text control. With no text control, the declared real button receives focus; `'none'` focuses the dialog surface. A safe dismiss default never turns Enter in a text field into data loss—the field simply does not publish a submit action. + ## Authoring principles - **One source of truth for shared chrome.** Compose from `chip-chrome.ts` / `chipVariants`; never duplicate the chrome string. diff --git a/apps/sim/app/(auth)/login/login-form.tsx b/apps/sim/app/(auth)/login/login-form.tsx index 380aeb89a42..d3c7d8bd6ed 100644 --- a/apps/sim/app/(auth)/login/login-form.tsx +++ b/apps/sim/app/(auth)/login/login-form.tsx @@ -464,9 +464,6 @@ export default function LoginPage({ title='Email' value={forgotPasswordEmail} onChange={(value) => setForgotPasswordEmail(value)} - onSubmit={() => { - if (!isSubmittingReset) void handleForgotPassword() - }} required placeholder='you@example.com' /> diff --git a/apps/sim/app/(landing)/components/auth-modal/auth-modal.tsx b/apps/sim/app/(landing)/components/auth-modal/auth-modal.tsx index f7a7872c093..da7f1e76324 100644 --- a/apps/sim/app/(landing)/components/auth-modal/auth-modal.tsx +++ b/apps/sim/app/(landing)/components/auth-modal/auth-modal.tsx @@ -48,6 +48,13 @@ const FALLBACK_STATUS: ProviderStatus = { const SOCIAL_BTN = 'relative flex h-[32px] w-full items-center justify-center rounded-[5px] border border-[var(--border-1)] text-[13.5px] text-[var(--text-primary)] transition-colors hover:bg-[var(--surface-hover)] disabled:cursor-not-allowed disabled:opacity-50' +/** Auth providers are peer choices, so opening the dialog must not arm one or dismissal. */ +function focusAuthDialog(event: Event): void { + event.preventDefault() + const content = event.currentTarget as HTMLElement | null + content?.focus() +} + function fetchProviderStatus(): Promise { if (fetchPromise) return fetchPromise fetchPromise = requestJson(getAuthProvidersContract, {}) @@ -155,7 +162,11 @@ export function AuthModal({ children, defaultView = 'login', source }: AuthModal return ( {children} - + {effectiveView === 'login' ? 'Log in' : 'Create account'} diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/delete-confirm-modal/delete-confirm-modal.tsx b/apps/sim/app/workspace/[workspaceId]/files/components/delete-confirm-modal/delete-confirm-modal.tsx index 8e02ebfcd69..2ac5a08bd99 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/delete-confirm-modal/delete-confirm-modal.tsx +++ b/apps/sim/app/workspace/[workspaceId]/files/components/delete-confirm-modal/delete-confirm-modal.tsx @@ -39,6 +39,7 @@ export const DeleteConfirmModal = memo(function DeleteConfirmModal({ onOpenChange={onOpenChange} srTitle={title} title={title} + defaultAction={totalCount === 1 && !hasFolders ? 'confirm' : 'dismiss'} text={[ 'Are you sure you want to delete ', fileName diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/share-modal/share-modal.tsx b/apps/sim/app/workspace/[workspaceId]/files/components/share-modal/share-modal.tsx index 26d9016c13d..d2ba8aeda37 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/share-modal/share-modal.tsx +++ b/apps/sim/app/workspace/[workspaceId]/files/components/share-modal/share-modal.tsx @@ -252,6 +252,7 @@ export function ShareModal({ !open && setExtractTargetId(null)} title='Unzip archive?' + defaultAction='confirm' text={[ 'This will unzip ', { text: extractTarget?.name ?? 'this archive', bold: true }, diff --git a/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/[documentId]/components/document-tags-modal/document-tags-modal.tsx b/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/[documentId]/components/document-tags-modal/document-tags-modal.tsx index e5fa53f5108..ff977135dc1 100644 --- a/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/[documentId]/components/document-tags-modal/document-tags-modal.tsx +++ b/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/[documentId]/components/document-tags-modal/document-tags-modal.tsx @@ -381,7 +381,7 @@ export function DocumentTagsModal({ handleClose(false)}>Document Tags - +
{documentTags.map((tag, index) => (
@@ -737,6 +737,7 @@ export function DocumentTagsModal({ handleClose(false)} + defaultAction='none' primaryAction={{ label: 'Close', onClick: () => handleClose(false) }} /> diff --git a/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/base.tsx b/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/base.tsx index 7e5d1e45483..53e2c99c80e 100644 --- a/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/base.tsx +++ b/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/base.tsx @@ -1372,6 +1372,7 @@ export function KnowledgeBase({ onOpenChange={setShowDeleteDialog} srTitle='Delete Knowledge Base' title='Delete Knowledge Base' + defaultAction='confirm' text={[ 'Are you sure you want to delete ', { text: knowledgeBaseName, bold: true }, diff --git a/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/base-tags-modal/base-tags-modal.tsx b/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/base-tags-modal/base-tags-modal.tsx index d82a9bd6937..ec640af722a 100644 --- a/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/base-tags-modal/base-tags-modal.tsx +++ b/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/base-tags-modal/base-tags-modal.tsx @@ -248,6 +248,7 @@ export function BaseTagsModal({ open, onOpenChange, knowledgeBaseId }: BaseTagsM Tags:{' '} @@ -389,6 +390,7 @@ export function BaseTagsModal({ open, onOpenChange, knowledgeBaseId }: BaseTagsM handleClose(false)} + defaultAction='none' primaryAction={{ label: 'Close', onClick: () => handleClose(false) }} /> diff --git a/apps/sim/app/workspace/[workspaceId]/knowledge/components/delete-knowledge-base-modal/delete-knowledge-base-modal.tsx b/apps/sim/app/workspace/[workspaceId]/knowledge/components/delete-knowledge-base-modal/delete-knowledge-base-modal.tsx index 5d99ed22be3..b37e80f5835 100644 --- a/apps/sim/app/workspace/[workspaceId]/knowledge/components/delete-knowledge-base-modal/delete-knowledge-base-modal.tsx +++ b/apps/sim/app/workspace/[workspaceId]/knowledge/components/delete-knowledge-base-modal/delete-knowledge-base-modal.tsx @@ -43,6 +43,7 @@ export const DeleteKnowledgeBaseModal = memo(function DeleteKnowledgeBaseModal({ onOpenChange={onClose} srTitle='Delete Knowledge Base' title='Delete Knowledge Base' + defaultAction='confirm' text={ knowledgeBaseName ? [ diff --git a/apps/sim/app/workspace/[workspaceId]/logs/components/log-details/components/execution-snapshot/execution-snapshot.tsx b/apps/sim/app/workspace/[workspaceId]/logs/components/log-details/components/execution-snapshot/execution-snapshot.tsx index b24a411d2b4..08d8c4095f2 100644 --- a/apps/sim/app/workspace/[workspaceId]/logs/components/log-details/components/execution-snapshot/execution-snapshot.tsx +++ b/apps/sim/app/workspace/[workspaceId]/logs/components/log-details/components/execution-snapshot/execution-snapshot.tsx @@ -1,8 +1,11 @@ 'use client' import type React from 'react' -import { useState } from 'react' +import { useId, useState } from 'react' import { + ChipModal, + ChipModalBody, + ChipModalHeader, cn, DropdownMenu, DropdownMenuContent, @@ -10,11 +13,6 @@ import { DropdownMenuTrigger, Duplicate, Loader, - Modal, - ModalBody, - ModalContent, - ModalDescription, - ModalHeader, } from '@sim/emcn' import { CircleAlert } from '@sim/emcn/icons' import { createPortal } from 'react-dom' @@ -62,6 +60,7 @@ export function ExecutionSnapshot({ onClose = () => {}, }: ExecutionSnapshotProps) { const { data, isLoading, error } = useExecutionSnapshot(executionId) + const modalDescriptionId = useId() const [isMenuOpen, setIsMenuOpen] = useState(false) const [menuPosition, setMenuPosition] = useState({ x: 0, y: 0 }) @@ -205,25 +204,26 @@ export function ExecutionSnapshot({ if (isModal) { return ( <> - { if (!open) { onClose() } }} + srTitle='Workflow State' + aria-describedby={modalDescriptionId} + size='full' + className='h-[90vh] [&>div]:h-full' > - - Workflow State - - - - View the workflow state snapshot for this execution - - {renderContent()} - - - + Workflow State + +

+ View the workflow state snapshot for this execution +

+ {renderContent()} +
+ {canvasContextMenu} ) diff --git a/apps/sim/app/workspace/[workspaceId]/logs/components/log-details/components/execution-snapshot/snapshot-boundary.test.tsx b/apps/sim/app/workspace/[workspaceId]/logs/components/log-details/components/execution-snapshot/snapshot-boundary.test.tsx index 221aa5892e4..a67b0cc4c16 100644 --- a/apps/sim/app/workspace/[workspaceId]/logs/components/log-details/components/execution-snapshot/snapshot-boundary.test.tsx +++ b/apps/sim/app/workspace/[workspaceId]/logs/components/log-details/components/execution-snapshot/snapshot-boundary.test.tsx @@ -11,27 +11,23 @@ const { mockToastError } = vi.hoisted(() => ({ vi.mock('@sim/emcn', () => ({ Loader: () =>