improvement(emcn): share one emails/domains chip input across share and deploy modals - #6226
Conversation
…nd deploy modals Extracts the emails chip lifecycle out of ChipModalField type='emails' into a standalone ChipEmailsInput, and points both the file share modal and the deploy modal's chat tab at it instead of their hand-rolled TagInput wiring. - add ChipEmailsInput (dedupe, normalize, format gate, paste, per-chip errors) with an allowDomains opt-in for bare @domain.tld entries - share modal and deploy modal chat tab now use it; drop both hand-rolled add/remove/validate implementations and the dead emailError state - move the shared allowlist policy into validateAllowlistEntry - drop the "Add specific emails or whole domains" hint text - give OutputSelect a size prop; the deploy modal chat tab uses the 30px chip trigger so it lines up with the Title field above it - drop overflow-y-auto from the chat deploy form, which was promoting overflow-x to auto and rendering a stray horizontal scrollbar
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview File share and chat deploy modals use
Reviewed by Cursor Bugbot for commit a4ba018. Configure here. |
Greptile SummaryThe PR consolidates email and domain chip handling into a shared format-aware component used by file sharing and chat deployment.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| packages/emcn/src/components/chip-emails-input/chip-emails-input.tsx | Introduces the shared controlled email/domain chip input with ref-backed synchronous composition and external-value reconciliation. |
| packages/emcn/src/components/chip-modal/chip-modal.tsx | Replaces the modal-specific email lifecycle with a thin wrapper around ChipEmailsInput. |
| apps/sim/app/workspace/[workspaceId]/files/components/share-modal/share-modal.tsx | Migrates file-share allowlists to the shared email/domain field and centralized policy validator. |
| apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/chat/chat.tsx | Migrates chat deployment allowlists to ChipEmailsInput and aligns the output selector with neighboring form controls. |
| apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/chat/components/output-select/output-select.tsx | Adds a medium ChipCombobox trigger option while preserving compact Combobox behavior by default. |
| apps/sim/lib/messaging/email/validation.ts | Reuses shared syntax validation and exposes the application-level allowlist policy callback. |
| packages/utils/src/string.ts | Adds bounded format-only validation for full email addresses and optional bare-domain allowlist entries. |
Reviews (3): Last reviewed commit: "test(email): pin the allowlist entry rul..." | Re-trigger Greptile
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…eholder regex Audit follow-ups on the emails chip input. - move EMAIL_SYNTAX_REGEX and the new @Domain pattern into @sim/utils/string as isValidEmailSyntax, so emcn and lib/messaging/email/validation.ts stop keeping byte-identical copies of the RFC 5322 regex - allow single-label domains (@intranet) again — requiring a dot rejected entries the old startsWith('@') check accepted, which self-hosted deployments use. A lone @ and malformed labels stay rejected - replace derivePlaceholderWithTags' /^Enter\s+(.+?)s?$/i with string ops; CodeQL flagged it as polynomial backtracking (js/redos). Verified identical output across the placeholder shapes in use - forward the emails control's props explicitly instead of underscore-discard destructuring, matching ChipModalFileControl in the same file
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 0a53c88. Configure here.
Covers isValidEmailSyntax's allowDomains branch (single-label domains stay
valid, malformed bare domains that the old startsWith('@') check accepted do
not), the 254-character cap, the DNS label limit, and validateAllowlistEntry
waiving address-level policy for bare domains. Verified both new rules fail
when the behavior is reverted.
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit a4ba018. Configure here.
2 participants
Summary
ChipModalField type='emails'into a standaloneChipEmailsInput— dedupe, normalize, format gate, paste, per-chip error tooltips — with anallowDomainsopt-in for bare@domain.tldentriesTagInputimplementations (add/remove/validate handlers,TagItemmapping, ref mirrors, and the deademailErrorstate that could only ever be'')validateAllowlistEntry, so disposable-domain/suspicious-pattern checks stay in the app and the design system stays format-onlyOutputSelectasizeprop; the deploy modal's Chat tab uses the 30px chip trigger so Output lines up with the Title field above it. Other call sites (API tab, chat panel toolbar) keep the compact pilloverflow-y-autofrom the chat deploy form — withoverflow-xstillvisible, CSS promoted it toauto, which is where the stray horizontal scrollbar came from.ModalBodyalready owns the vertical scroll, so the class bought nothingType of Change
Testing
bun run lint:check(23/23),tsc --noEmit,check:api-validation, and the 1644 workspace tests all pass. Not yet verified in a running browser.Note: the emails field in both modals is now the taller
blockchip surface (the same one the workspace invite modal uses) rather than the previous single-row input.Checklist