From 94b1f5aa48cd6bc4cfe9c4fa8df7d6980b496070 Mon Sep 17 00:00:00 2001 From: Stefan Hildebrandt <695494+hildebrandttk@users.noreply.github.com> Date: Mon, 15 Jun 2026 18:20:51 +0200 Subject: [PATCH 1/4] refactor: model composer recipients as arrays instead of delimited strings Alternative to the quote-aware string fix: represent committed To/Cc/Bcc recipients as Recipient[] ({name?, email}) with a separate input-text string per field, instead of a single comma-joined string parsed with split(','). Structured recipients can never be torn apart on a delimiter, so a display name containing a comma ("Doo, John ", as produced on Reply-All) stays a single chip. - email-composer-utils: add Recipient type, parseRecipient/formatRecipient, and parseRecipientList/formatRecipientList for the (de)serialization boundary (ComposerDraftData stays a string; quoting keeps it lossless). Remove the now-unused string-chip helpers. - email-composer: to/cc/bcc are Recipient[]; toInput/ccInput/bccInput hold the in-progress text. Reply/forward init, autocomplete, chip edit, drag & drop (payload now carries the structured recipient), send/draft/validation and template paths all operate on arrays. withInput() folds uncommitted typed text into the send/validation set. - Tests updated for the array contract; add comma-in-name chip coverage. --- .../__tests__/recipient-chip-drag.test.tsx | 26 +- components/email/email-composer.tsx | 359 +++++++++--------- lib/__tests__/email-composer-utils.test.ts | 105 +++-- lib/email-composer-utils.ts | 101 +++-- 4 files changed, 337 insertions(+), 254 deletions(-) diff --git a/components/email/__tests__/recipient-chip-drag.test.tsx b/components/email/__tests__/recipient-chip-drag.test.tsx index 517e317c..68ac3083 100644 --- a/components/email/__tests__/recipient-chip-drag.test.tsx +++ b/components/email/__tests__/recipient-chip-drag.test.tsx @@ -237,7 +237,7 @@ describe('RecipientChipInput drag and drop', () => { expect(chipSpan).toHaveAttribute('draggable', 'true'); }); - it('onDragStart encodes chip value and source field into dataTransfer', async () => { + it('onDragStart encodes the recipient and source field into dataTransfer', async () => { render(); const chipText = await screen.findByText('alice@example.com'); @@ -247,7 +247,21 @@ describe('RecipientChipInput drag and drop', () => { fireEvent.dragStart(chipSpan, { dataTransfer: dt }); const payload = JSON.parse(dt.getData('application/x-recipient-chip')); - expect(payload).toEqual({ chip: 'alice@example.com', fromField: 'to' }); + expect(payload).toEqual({ recipient: { email: 'alice@example.com' }, fromField: 'to' }); + }); + + it('keeps a display name with a comma in a single chip (array model)', async () => { + render(, ' }} />); + + // One chip, displayed as "Doo, John (john@doo.org)" — not split on the comma. + const chip = await screen.findByText('Doo, John (john@doo.org)'); + const chipSpan = chip.closest('[draggable]') as HTMLElement; + expect(chipSpan).not.toBeNull(); + + const dt = new MockDataTransfer(); + fireEvent.dragStart(chipSpan, { dataTransfer: dt }); + const payload = JSON.parse(dt.getData('application/x-recipient-chip')); + expect(payload).toEqual({ recipient: { name: 'Doo, John', email: 'john@doo.org' }, fromField: 'to' }); }); it('onDragEnd clears the opacity class on the chip', async () => { @@ -277,7 +291,7 @@ describe('RecipientChipInput drag and drop', () => { if (!ccContainer) return; const dt = new MockDataTransfer(); - dt.setData('application/x-recipient-chip', JSON.stringify({ chip: 'alice@example.com', fromField: 'to' })); + dt.setData('application/x-recipient-chip', JSON.stringify({ recipient: { email: 'alice@example.com' }, fromField: 'to' })); fireEvent.dragOver(ccContainer, { dataTransfer: dt }); expect(ccContainer.className).toContain('ring-primary'); @@ -297,7 +311,7 @@ describe('RecipientChipInput drag and drop', () => { if (!toContainer || !ccContainer) return; const dt = new MockDataTransfer(); - dt.setData('application/x-recipient-chip', JSON.stringify({ chip: 'alice@example.com', fromField: 'to' })); + dt.setData('application/x-recipient-chip', JSON.stringify({ recipient: { email: 'alice@example.com' }, fromField: 'to' })); fireEvent.dragOver(ccContainer, { dataTransfer: dt }); act(() => { fireEvent.drop(ccContainer, { dataTransfer: dt }); @@ -319,7 +333,7 @@ describe('RecipientChipInput drag and drop', () => { const toContainer = allContainers.find(el => el.querySelector('[draggable]')) as HTMLElement; const dt = new MockDataTransfer(); - dt.setData('application/x-recipient-chip', JSON.stringify({ chip: 'alice@example.com', fromField: 'to' })); + dt.setData('application/x-recipient-chip', JSON.stringify({ recipient: { email: 'alice@example.com' }, fromField: 'to' })); fireEvent.dragOver(toContainer, { dataTransfer: dt }); act(() => { fireEvent.drop(toContainer, { dataTransfer: dt }); @@ -336,7 +350,7 @@ describe('RecipientChipInput drag and drop', () => { const ccButton = screen.getByRole('button', { name: 'Cc' }); const dt = new MockDataTransfer(); - dt.setData('application/x-recipient-chip', JSON.stringify({ chip: 'alice@example.com', fromField: 'to' })); + dt.setData('application/x-recipient-chip', JSON.stringify({ recipient: { email: 'alice@example.com' }, fromField: 'to' })); fireEvent.dragOver(ccButton, { dataTransfer: dt }); act(() => { fireEvent.drop(ccButton, { dataTransfer: dt }); diff --git a/components/email/email-composer.tsx b/components/email/email-composer.tsx index 12314cb5..42e8eaab 100644 --- a/components/email/email-composer.tsx +++ b/components/email/email-composer.tsx @@ -45,8 +45,11 @@ import { computeReplyThreadingHeaders } from "@/lib/email-threading"; import { rewriteCidImagesForEditor, replaceInlineImagePlaceholders, - removeChipFromFieldValue, - addChipToFieldValue, + formatRecipient, + parseRecipient, + parseRecipientList, + formatRecipientList, + type Recipient, } from "@/lib/email-composer-utils"; import { RichTextEditor } from "@/components/email/rich-text-editor"; import type { Editor } from "@tiptap/react"; @@ -280,46 +283,34 @@ export function EmailComposer({ const shouldEmbedSignatureInNewMail = mode === 'compose' && hasInitialSignature; // Format a single EmailAddress for display in the composer input - const formatAddr = (r: { name?: string; email?: string }) => - r.email ? (r.name && r.name !== r.email ? `${r.name} <${r.email}>` : r.email) : ""; - - // Parse a recipient string that may be "Name " or bare "email" - const parseRecipient = (s: string): { name?: string; email: string } => { - const trimmed = s.trim(); - const angleMatch = trimmed.match(/^(.+?)\s*<([^>]+)>$/); - if (angleMatch) { - return { name: angleMatch[1].trim(), email: angleMatch[2].trim() }; - } - return { email: trimmed }; - }; + const toRecipient = (r: { name?: string; email?: string }): Recipient => + ({ name: r.name && r.name !== r.email ? r.name : undefined, email: r.email ?? "" }); // Initialize with reply/forward data if provided - const getInitialTo = () => { - if (!replyTo) return ""; + const getInitialTo = (): Recipient[] => { + if (!replyTo) return []; // RFC 5322: use Reply-To header if present, otherwise fall back to From const replyTarget = replyTo.replyToAddresses?.length - ? replyTo.replyToAddresses.filter(r => r.email).map(formatAddr).join(", ") - : (replyTo.from?.[0] ? formatAddr(replyTo.from[0]) : ""); + ? replyTo.replyToAddresses.filter(r => r.email).map(toRecipient) + : (replyTo.from?.[0]?.email ? [toRecipient(replyTo.from[0])] : []); if (mode === 'reply') { - return replyTarget ? replyTarget + ', ' : ""; + return replyTarget; } else if (mode === 'replyAll') { const ownEmails = new Set(identities.map(i => i.email?.trim().toLowerCase()).filter(Boolean)); - const originalTo = replyTo.to - ?.filter(r => r.email && !ownEmails.has(r.email.trim().toLowerCase())) - .map(formatAddr).join(", ") || ""; - const combined = [replyTarget, originalTo].filter(Boolean).join(", "); - return combined ? combined + ', ' : ""; + const originalTo = (replyTo.to ?? []) + .filter(r => r.email && !ownEmails.has(r.email.trim().toLowerCase())) + .map(toRecipient); + return [...replyTarget, ...originalTo]; } - return ""; + return []; }; - const getInitialCc = () => { - if (!replyTo || mode !== 'replyAll') return ""; + const getInitialCc = (): Recipient[] => { + if (!replyTo || mode !== 'replyAll') return []; const ownEmails = new Set(identities.map(i => i.email?.trim().toLowerCase()).filter(Boolean)); - const cc = replyTo.cc - ?.filter(r => r.email && !ownEmails.has(r.email.trim().toLowerCase())) - .map(formatAddr).join(", ") || ""; - return cc ? cc + ', ' : ""; + return (replyTo.cc ?? []) + .filter(r => r.email && !ownEmails.has(r.email.trim().toLowerCase())) + .map(toRecipient); }; const getInitialSubject = () => { @@ -459,13 +450,25 @@ export function EmailComposer({ return prefix; }; - const [to, setTo] = useState(initialData?.to ?? getInitialTo()); - const [cc, setCc] = useState(initialData?.cc ?? getInitialCc()); - const [bcc, setBcc] = useState(initialData?.bcc ?? ""); + // Committed recipients are structured arrays; the in-progress text the user + // is typing lives in a separate `*Input` string per field. This keeps a + // display name containing a comma (e.g. "Doo, John") intact instead of + // tearing it apart on a delimiter. + const [to, setTo] = useState(initialData ? parseRecipientList(initialData.to) : getInitialTo()); + const [cc, setCc] = useState(initialData ? parseRecipientList(initialData.cc) : getInitialCc()); + const [bcc, setBcc] = useState(initialData ? parseRecipientList(initialData.bcc) : []); + const [toInput, setToInput] = useState(''); + const [ccInput, setCcInput] = useState(''); + const [bccInput, setBccInput] = useState(''); const [subject, setSubject] = useState(initialData?.subject ?? getInitialSubject()); const [body, setBody] = useState(initialData?.body ?? getInitialBody()); - const [showCc, setShowCc] = useState(initialData?.showCc ?? !!getInitialCc()); + const [showCc, setShowCc] = useState(initialData?.showCc ?? getInitialCc().length > 0); const [showBcc, setShowBcc] = useState(initialData?.showBcc ?? false); + // Committed recipients plus any not-yet-committed text the user has typed. + // Send/validation/draft paths treat a typed-but-uncommitted address as a + // real recipient, matching the previous string-based behavior. + const withInput = (chips: Recipient[], input: string): Recipient[] => + input.trim() ? [...chips, parseRecipient(input)] : chips; const [isDraggingChipOverCc, setIsDraggingChipOverCc] = useState(false); const [isDraggingChipOverBcc, setIsDraggingChipOverBcc] = useState(false); const [requestReadReceipt, setRequestReadReceipt] = useState(requestReadReceiptDefault); @@ -794,10 +797,11 @@ export function EmailComposer({ const canSmimeSign = !!smimeKeyRecord; const canSmimeEncrypt = (() => { if (!smimeKeyRecord) return false; - const toAddrs = to.split(',').map(e => e.trim()).filter(Boolean); - const ccAddrs = cc.split(',').map(e => e.trim()).filter(Boolean); - const bccAddrs = bcc.split(',').map(e => e.trim()).filter(Boolean); - const allRecipients = [...toAddrs, ...ccAddrs, ...bccAddrs]; + const allRecipients = [ + ...withInput(to, toInput), + ...withInput(cc, ccInput), + ...withInput(bcc, bccInput), + ].map(r => r.email); if (allRecipients.length === 0) return false; const { missing } = smimeStore.getRecipientCerts(allRecipients); return missing.length === 0; @@ -813,15 +817,21 @@ export function EmailComposer({ // eslint-disable-next-line react-hooks/exhaustive-deps }, [currentSmimeIdentityId]); + // Serialized recipient strings for ComposerDraftData (string-shaped) and for + // by-value dirty comparison. Folds in any uncommitted typed text. + const toStr = formatRecipientList(withInput(to, toInput)); + const ccStr = formatRecipientList(withInput(cc, ccInput)); + const bccStr = formatRecipientList(withInput(bcc, bccInput)); + // Keep a ref to current state for the unmount save - const stateRef = useRef({ to, cc, bcc, subject, body, showCc, showBcc, selectedIdentityId, subAddressTag, draftId, fromOverrideEnabled, fromOverrideEmail, fromOverrideName }); - stateRef.current = { to, cc, bcc, subject, body, showCc, showBcc, selectedIdentityId, subAddressTag, draftId, fromOverrideEnabled, fromOverrideEmail, fromOverrideName }; + const stateRef = useRef({ to: toStr, cc: ccStr, bcc: bccStr, subject, body, showCc, showBcc, selectedIdentityId, subAddressTag, draftId, fromOverrideEnabled, fromOverrideEmail, fromOverrideName }); + stateRef.current = { to: toStr, cc: ccStr, bcc: bccStr, subject, body, showCc, showBcc, selectedIdentityId, subAddressTag, draftId, fromOverrideEnabled, fromOverrideEmail, fromOverrideName }; // Track initial values for dirty detection (captured once on first render) - const initialValuesRef = useRef({ to, cc, bcc, subject, body, attachmentCount: attachments.length }); + const initialValuesRef = useRef({ to: toStr, cc: ccStr, bcc: bccStr, subject, body, attachmentCount: attachments.length }); const isDirtyRef = useRef(false); - isDirtyRef.current = to !== initialValuesRef.current.to || cc !== initialValuesRef.current.cc || - bcc !== initialValuesRef.current.bcc || subject !== initialValuesRef.current.subject || + isDirtyRef.current = toStr !== initialValuesRef.current.to || ccStr !== initialValuesRef.current.cc || + bccStr !== initialValuesRef.current.bcc || subject !== initialValuesRef.current.subject || body !== initialValuesRef.current.body || attachments.length > initialValuesRef.current.attachmentCount; // Ref to latest saveDraft for use in event handlers with stale closures @@ -898,21 +908,26 @@ export function EmailComposer({ } }, [plainTextMode]); - const handleMoveChip = useCallback((chip: string, fromField: 'to' | 'cc' | 'bcc', toField: 'to' | 'cc' | 'bcc') => { + const handleMoveChip = useCallback((recipient: Recipient, fromField: 'to' | 'cc' | 'bcc', toField: 'to' | 'cc' | 'bcc') => { + if (fromField === toField) return; const setters = { to: setTo, cc: setCc, bcc: setBcc }; - setters[fromField](prev => removeChipFromFieldValue(prev, chip)); - setters[toField](prev => addChipToFieldValue(prev, chip)); + const sameRecipient = (a: Recipient, b: Recipient) => a.email === b.email && (a.name ?? '') === (b.name ?? ''); + setters[fromField](prev => { + const idx = prev.findIndex(r => sameRecipient(r, recipient)); + return idx === -1 ? prev : prev.filter((_, i) => i !== idx); + }); + setters[toField](prev => prev.some(r => sameRecipient(r, recipient)) ? prev : [...prev, recipient]); if (toField === 'cc') setShowCc(true); if (toField === 'bcc') setShowBcc(true); }, [setTo, setCc, setBcc, setShowCc, setShowBcc]); - const handleAutocomplete = useCallback((value: string, field: 'to' | 'cc' | 'bcc') => { + const handleAutocomplete = useCallback((inputText: string, field: 'to' | 'cc' | 'bcc') => { if (autocompleteTimeoutRef.current) { clearTimeout(autocompleteTimeoutRef.current); } - const lastPart = value.split(',').pop()?.trim() || ''; - if (lastPart.length < 1) { + const query = inputText.trim(); + if (query.length < 1) { setAutocompleteResults([]); setActiveAutoField(null); setAutoSelectedIndex(-1); @@ -920,10 +935,10 @@ export function EmailComposer({ } autocompleteTimeoutRef.current = setTimeout(async () => { - const localResults = getAutocomplete(lastPart); + const localResults = getAutocomplete(query); // Let plugins contribute extra suggestions (Slack handles, GitHub, CRM, …). const initial: RecipientSuggestion[] = localResults.map(r => ({ name: r.name, email: r.email })); - const merged = await contactHooks.onProvideRecipientSuggestions.transform(initial, { query: lastPart }); + const merged = await contactHooks.onProvideRecipientSuggestions.transform(initial, { query }); setAutocompleteResults(merged.map(s => ({ name: s.name, email: s.email }))); setActiveAutoField(merged.length > 0 ? field : null); setAutoSelectedIndex(-1); @@ -932,17 +947,10 @@ export function EmailComposer({ const insertAutocomplete = (suggestion: { name: string; email: string }, field: 'to' | 'cc' | 'bcc') => { const setter = field === 'to' ? setTo : field === 'cc' ? setCc : setBcc; - const getter = field === 'to' ? to : field === 'cc' ? cc : bcc; + const inputSetter = field === 'to' ? setToInput : field === 'cc' ? setCcInput : setBccInput; - const parts = getter.split(',').map(s => s.trim()).filter(Boolean); - if (!getter.trimEnd().endsWith(',') && parts.length > 0) { - parts.pop(); - } - const formatted = suggestion.name && suggestion.name !== suggestion.email - ? `${suggestion.name} <${suggestion.email}>` - : suggestion.email; - parts.push(formatted); - setter(parts.join(', ') + ', '); + setter(prev => [...prev, toRecipient(suggestion)]); + inputSetter(''); setAutocompleteResults([]); setActiveAutoField(null); setAutoSelectedIndex(-1); @@ -999,14 +1007,14 @@ export function EmailComposer({ setSubject(filledSubject); setBody(bodyContent); if (template.defaultRecipients?.to?.length) { - setTo(template.defaultRecipients.to.join(', ') + ', '); + setTo(template.defaultRecipients.to.map(parseRecipient)); } if (template.defaultRecipients?.cc?.length) { - setCc(template.defaultRecipients.cc.join(', ') + ', '); + setCc(template.defaultRecipients.cc.map(parseRecipient)); setShowCc(true); } if (template.defaultRecipients?.bcc?.length) { - setBcc(template.defaultRecipients.bcc.join(', ') + ', '); + setBcc(template.defaultRecipients.bcc.map(parseRecipient)); setShowBcc(true); } } else { @@ -1233,9 +1241,9 @@ export function EmailComposer({ const saveDraftOnce = async (): Promise => { if (!client || !composerClient) return null; - const toAddresses = to.split(",").map(e => e.trim()).filter(Boolean); - const ccAddresses = cc.split(",").map(e => e.trim()).filter(Boolean); - const bccAddresses = bcc.split(",").map(e => e.trim()).filter(Boolean); + const toAddresses = withInput(to, toInput).map(r => formatRecipient(r.name, r.email)); + const ccAddresses = withInput(cc, ccInput).map(r => formatRecipient(r.name, r.email)); + const bccAddresses = withInput(bcc, bccInput).map(r => formatRecipient(r.name, r.email)); if (!toAddresses.length && !subject && !(plainTextMode ? body.trim() : htmlToPlainText(body).trim())) { return null; @@ -1358,9 +1366,9 @@ export function EmailComposer({ saveTimeoutRef.current = null; // Plugin observers (AI assist, grammar, …) get a debounced snapshot here. emailHooks.onDraftChange.emit({ - to: to.split(',').map(s => s.trim()).filter(Boolean), - cc: cc.split(',').map(s => s.trim()).filter(Boolean), - bcc: bcc.split(',').map(s => s.trim()).filter(Boolean), + to: withInput(to, toInput).map(r => formatRecipient(r.name, r.email)), + cc: withInput(cc, ccInput).map(r => formatRecipient(r.name, r.email)), + bcc: withInput(bcc, bccInput).map(r => formatRecipient(r.name, r.email)), subject, htmlBody: plainTextMode ? '' : body, textBody: plainTextMode ? body : htmlToPlainText(body), @@ -1379,7 +1387,7 @@ export function EmailComposer({ } }; // eslint-disable-next-line react-hooks/exhaustive-deps -- saveDraft reads current state when called, not when effect is set up - }, [to, cc, bcc, subject, body, attachments]); + }, [toStr, ccStr, bccStr, subject, body, attachments]); useEffect(() => { return () => { @@ -1389,7 +1397,7 @@ export function EmailComposer({ }; }, []); - const toAddresses = to.split(",").map(e => e.trim()).filter(Boolean); + const toAddresses = withInput(to, toInput); const bodyPlainText = plainTextMode ? body.trim() : htmlToPlainText(body).trim(); const hasContent = bodyPlainText || attachments.some(att => att.blobId && !att.uploading); const canSend = toAddresses.length > 0 && !!subject && hasContent; @@ -1473,8 +1481,8 @@ export function EmailComposer({ }; const handleSend = async (skipAttachmentCheck = false, delayedUntil?: string) => { - const ccAddresses = cc.split(",").map(e => e.trim()).filter(Boolean); - const bccAddresses = bcc.split(",").map(e => e.trim()).filter(Boolean); + const ccAddresses = withInput(cc, ccInput); + const bccAddresses = withInput(bcc, bccInput); if (!canSend) { const errors: { to?: boolean; subject?: boolean; body?: boolean } = {}; @@ -1596,9 +1604,9 @@ export function EmailComposer({ // guards, etc.). Returning false from any handler aborts before either // the S/MIME or standard JMAP path runs. const sendablePreview: OutgoingEmail = { - to: toAddresses, - cc: ccAddresses, - bcc: bccAddresses, + to: toAddresses.map(r => formatRecipient(r.name, r.email)), + cc: ccAddresses.map(r => formatRecipient(r.name, r.email)), + bcc: bccAddresses.map(r => formatRecipient(r.name, r.email)), subject, htmlBody: finalHtmlBody || '', textBody: finalBody, @@ -1686,9 +1694,9 @@ export function EmailComposer({ : undefined; const mimeBytes = buildMimeMessage({ from: { name: currentIdentity.name || undefined, email: fromEmail || currentIdentity.email }, - to: toAddresses.map(parseRecipient), - cc: ccAddresses.length > 0 ? ccAddresses.map(parseRecipient) : undefined, - bcc: bccAddresses.length > 0 ? bccAddresses.map(parseRecipient) : undefined, + to: toAddresses, + cc: ccAddresses.length > 0 ? ccAddresses : undefined, + bcc: bccAddresses.length > 0 ? bccAddresses : undefined, subject, inReplyTo: mimeInReplyTo, references: mimeReferences, @@ -1701,8 +1709,8 @@ export function EmailComposer({ const smimeHeaders = { from: { name: currentIdentity.name || undefined, email: fromEmail || currentIdentity.email }, - to: toAddresses.map(parseRecipient), - cc: ccAddresses.length > 0 ? ccAddresses.map(parseRecipient) : undefined, + to: toAddresses, + cc: ccAddresses.length > 0 ? ccAddresses : undefined, subject, inReplyTo: mimeInReplyTo, references: mimeReferences, @@ -1724,7 +1732,7 @@ export function EmailComposer({ // 6. Encrypt if enabled if (smimeEncrypt_ && smimeKeyRecord) { - const allRecipients = [...toAddresses, ...ccAddresses, ...bccAddresses].map(s => parseRecipient(s).email); + const allRecipients = [...toAddresses, ...ccAddresses, ...bccAddresses].map(r => r.email); const { found, missing } = smimeStore.getRecipientCerts(allRecipients); if (missing.length > 0) { throw new Error(`Missing certificates for: ${missing.join(', ')}`); @@ -1741,7 +1749,7 @@ export function EmailComposer({ } // 7. Send via raw email path - const result = await sendRawEmail(client, payload, currentIdentity.id, effectiveDelayedUntil, [...toAddresses, ...ccAddresses, ...bccAddresses].map(s => parseRecipient(s).email)); + const result = await sendRawEmail(client, payload, currentIdentity.id, effectiveDelayedUntil, [...toAddresses, ...ccAddresses, ...bccAddresses].map(r => r.email)); if (effectiveDelayedUntil && finalDraftId) { client.deleteEmail(finalDraftId).catch(err => { debug.warn('email', 'Scheduled S/MIME send created, but plaintext draft cleanup failed:', err); @@ -1762,9 +1770,9 @@ export function EmailComposer({ // Let plugins (signatures, link-rewriting, encryption, AI rewrite, …) // transform the outgoing message immediately before submission. const transformInput: OutgoingEmail = { - to: toAddresses, - cc: ccAddresses, - bcc: bccAddresses, + to: toAddresses.map(r => formatRecipient(r.name, r.email)), + cc: ccAddresses.map(r => formatRecipient(r.name, r.email)), + bcc: bccAddresses.map(r => formatRecipient(r.name, r.email)), subject, htmlBody: finalHtmlBody || '', textBody: finalBody, @@ -1817,9 +1825,12 @@ export function EmailComposer({ } } - setTo(""); - setCc(""); - setBcc(""); + setTo([]); + setCc([]); + setBcc([]); + setToInput(""); + setCcInput(""); + setBccInput(""); setSubject(""); setBody(""); draftIdRef.current = null; @@ -2102,7 +2113,7 @@ export function EmailComposer({ ? identities.find(id => id.id === selectedIdentityId)?.email : primaryIdentity?.email) || '' } - recipientEmails={to.split(',').map(e => e.trim()).filter(Boolean)} + recipientEmails={withInput(to, toInput).map(r => r.email)} onSelectTag={setSubAddressTag} /> )} @@ -2147,11 +2158,13 @@ export function EmailComposer({
{t('to')}: { - setTo(v); + chips={to} + onChipsChange={(next) => { + setTo(next); if (validationErrors.to) setValidationErrors(prev => ({ ...prev, to: false })); }} + inputText={toInput} + onInputChange={setToInput} inputRef={toInputRef} placeholder={t('to_placeholder')} field="to" @@ -2186,8 +2199,8 @@ export function EmailComposer({ setIsDraggingChipOverCc(false); const raw = e.dataTransfer.getData('application/x-recipient-chip'); if (!raw) return; - const { chip, fromField } = JSON.parse(raw) as { chip: string; fromField: 'to' | 'cc' | 'bcc' }; - if (fromField !== 'cc') handleMoveChip(chip, fromField, 'cc'); + const { recipient, fromField } = JSON.parse(raw) as { recipient: Recipient; fromField: 'to' | 'cc' | 'bcc' }; + if (fromField !== 'cc') handleMoveChip(recipient, fromField, 'cc'); setShowCc(true); }} > @@ -2210,8 +2223,8 @@ export function EmailComposer({ setIsDraggingChipOverBcc(false); const raw = e.dataTransfer.getData('application/x-recipient-chip'); if (!raw) return; - const { chip, fromField } = JSON.parse(raw) as { chip: string; fromField: 'to' | 'cc' | 'bcc' }; - if (fromField !== 'bcc') handleMoveChip(chip, fromField, 'bcc'); + const { recipient, fromField } = JSON.parse(raw) as { recipient: Recipient; fromField: 'to' | 'cc' | 'bcc' }; + if (fromField !== 'bcc') handleMoveChip(recipient, fromField, 'bcc'); setShowBcc(true); }} > @@ -2225,8 +2238,10 @@ export function EmailComposer({
{t('cc_label')} {t('bcc_label')} s.trim()).filter(Boolean), - cc: cc.split(',').map(s => s.trim()).filter(Boolean), - bcc: bcc.split(',').map(s => s.trim()).filter(Boolean), + to: withInput(to, toInput).map(r => formatRecipient(r.name, r.email)), + cc: withInput(cc, ccInput).map(r => formatRecipient(r.name, r.email)), + bcc: withInput(bcc, bccInput).map(r => formatRecipient(r.name, r.email)), }} onSave={(data) => { addTemplate(data); @@ -2790,8 +2807,10 @@ const AutocompleteDropdown = React.forwardRef void; + chips: Recipient[]; + onChipsChange: (chips: Recipient[]) => void; + inputText: string; + onInputChange: (text: string) => void; inputRef: React.RefObject; placeholder: string; field: 'to' | 'cc' | 'bcc'; - onAutocomplete: (value: string, field: 'to' | 'cc' | 'bcc') => void; + onAutocomplete: (inputText: string, field: 'to' | 'cc' | 'bcc') => void; onAutoKeyDown: (e: React.KeyboardEvent, field: 'to' | 'cc' | 'bcc') => void; onAutoBlur: (e: React.FocusEvent, field: 'to' | 'cc' | 'bcc') => void; activeAutoField: 'to' | 'cc' | 'bcc' | null; @@ -2824,22 +2845,17 @@ function RecipientChipInput({ validationError?: boolean; validationMessage?: string; onTab?: () => void; - onMoveChip: (chip: string, fromField: 'to' | 'cc' | 'bcc', toField: 'to' | 'cc' | 'bcc') => void; + onMoveChip: (recipient: Recipient, fromField: 'to' | 'cc' | 'bcc', toField: 'to' | 'cc' | 'bcc') => void; }) { const t = useTranslations('email_composer'); const tCommon = useTranslations('common'); - const { contextMenu, openContextMenu, closeContextMenu, menuRef } = useContextMenu<{ index: number; chip: string }>(); - const [editingChip, setEditingChip] = useState<{ index: number; chip: string; editType: 'email' | 'name' } | null>(null); + const { contextMenu, openContextMenu, closeContextMenu, menuRef } = useContextMenu<{ index: number; recipient: Recipient }>(); + const [editingChip, setEditingChip] = useState<{ index: number; editType: 'email' | 'name' } | null>(null); const [editValue, setEditValue] = useState(''); const [isDragOver, setIsDragOver] = useState(false); const [draggingIndex, setDraggingIndex] = useState(null); const editInputRef = useRef(null); - const allParts = value.split(',').map(s => s.trim()).filter(Boolean); - const hasTrailingComma = value.trimEnd().endsWith(','); - const chips = hasTrailingComma ? allParts : allParts.slice(0, -1); - const inputText = hasTrailingComma ? '' : (allParts[allParts.length - 1] || ''); - // Focus edit input when editing starts useEffect(() => { if (editingChip) { @@ -2852,70 +2868,50 @@ function RecipientChipInput({ } }, [editingChip]); - // Parse a chip string to extract name and email - const parseChip = (chip: string): { name?: string; email: string } => { - const angleMatch = chip.match(/^(.+?)\s*<([^>]+)>$/); - if (angleMatch) { - return { name: angleMatch[1].trim(), email: angleMatch[2].trim() }; - } - return { email: chip }; - }; - - // Format a chip for display - const formatChipDisplay = (chip: string): string => { - const parsed = parseChip(chip); - if (parsed.name && parsed.name !== parsed.email) { - return `${parsed.name} (${parsed.email})`; - } - return parsed.email; - }; + // Format a recipient for display in a chip / context menu + const formatChipDisplay = (r: Recipient): string => + r.name && r.name !== r.email ? `${r.name} (${r.email})` : r.email; // Handle saving an edited chip const handleSaveEdit = (newValue: string) => { if (!editingChip) return; const { index, editType } = editingChip; const chip = chips[index]; - const parsed = parseChip(chip); + if (!chip) { + setEditingChip(null); + return; + } + const trimmedNew = newValue.trim(); - let newChip: string; + let newChip: Recipient; if (editType === 'email') { - // Update email, keep name - const trimmedNew = newValue.trim(); + // Update email, keep name. Empty email is a no-op (can't drop the email). if (!trimmedNew) { setEditingChip(null); return; } - newChip = parsed.name ? `${parsed.name} <${trimmedNew}>` : trimmedNew; + newChip = { name: chip.name, email: trimmedNew }; } else { - // Update name, keep email - const trimmedNew = newValue.trim(); - if (trimmedNew) { - newChip = `${trimmedNew} <${parsed.email}>`; - } else { - // Name cleared, remove from format - newChip = parsed.email; - } + // Update name, keep email. Empty name clears the display name. + newChip = { name: trimmedNew || undefined, email: chip.email }; } - // Replace the chip in the value const newChips = [...chips]; newChips[index] = newChip; - onChange(newChips.join(', ') + ', '); + onChipsChange(newChips); setEditingChip(null); }; const handleInputChange = (e: React.ChangeEvent) => { const newInputText = e.target.value; - const chipPart = chips.length > 0 ? chips.join(', ') + ', ' : ''; - const newValue = chipPart + newInputText; - onChange(newValue); - onAutocomplete(newValue, field); + onInputChange(newInputText); + onAutocomplete(newInputText, field); }; const commitCurrentInput = () => { if (inputText.trim()) { - const newChips = [...chips, inputText.trim()]; - onChange(newChips.join(', ') + ', '); + onChipsChange([...chips, parseRecipient(inputText)]); + onInputChange(''); } }; @@ -2946,45 +2942,39 @@ function RecipientChipInput({ return; } + // Backspace on an empty input pulls the last chip back into the input for + // quick editing. if (e.key === 'Backspace' && !inputText && chips.length > 0) { const lastChip = chips[chips.length - 1]; - const remainingChips = chips.slice(0, -1); - const chipPart = remainingChips.length > 0 ? remainingChips.join(', ') + ', ' : ''; - onChange(chipPart + lastChip); + onChipsChange(chips.slice(0, -1)); + onInputChange(formatRecipient(lastChip.name, lastChip.email)); return; } }; const handleChipRemove = (index: number, e: React.MouseEvent) => { e.stopPropagation(); - const remainingChips = chips.filter((_, i) => i !== index); - if (remainingChips.length > 0) { - onChange(remainingChips.join(', ') + ', ' + inputText); - } else { - onChange(inputText); - } + onChipsChange(chips.filter((_, i) => i !== index)); }; - const handleContextMenu = (e: React.MouseEvent, index: number, chip: string) => { - openContextMenu(e, { index, chip }); + const handleContextMenu = (e: React.MouseEvent, index: number, recipient: Recipient) => { + openContextMenu(e, { index, recipient }); }; const handleEditEmail = () => { if (!contextMenu.data) return; - const { index, chip } = contextMenu.data; - const parsed = parseChip(chip); + const { index, recipient } = contextMenu.data; closeContextMenu(); - setEditValue(parsed.email); - setEditingChip({ index, chip, editType: 'email' }); + setEditValue(recipient.email); + setEditingChip({ index, editType: 'email' }); }; const handleEditName = () => { if (!contextMenu.data) return; - const { index, chip } = contextMenu.data; - const parsed = parseChip(chip); + const { index, recipient } = contextMenu.data; closeContextMenu(); - setEditValue(parsed.name || ''); - setEditingChip({ index, chip, editType: 'name' }); + setEditValue(recipient.name || ''); + setEditingChip({ index, editType: 'name' }); }; const handleBlur = (e: React.FocusEvent) => { @@ -2992,10 +2982,7 @@ function RecipientChipInput({ if (relatedTarget && dropdownRef.current?.contains(relatedTarget)) { return; } - if (inputText.trim()) { - const newChips = [...chips, inputText.trim()]; - onChange(newChips.join(', ') + ', '); - } + commitCurrentInput(); onAutoBlur(e, field); }; @@ -3017,9 +3004,9 @@ function RecipientChipInput({ setIsDragOver(false); const raw = e.dataTransfer.getData('application/x-recipient-chip'); if (!raw) return; - const { chip: draggedChip, fromField } = JSON.parse(raw) as { chip: string; fromField: 'to' | 'cc' | 'bcc' }; + const { recipient, fromField } = JSON.parse(raw) as { recipient: Recipient; fromField: 'to' | 'cc' | 'bcc' }; if (fromField === field) return; - onMoveChip(draggedChip, fromField, field); + onMoveChip(recipient, fromField, field); }; return ( @@ -3040,14 +3027,14 @@ function RecipientChipInput({ const chipDisplay = formatChipDisplay(chip); return ( { e.stopPropagation(); e.dataTransfer.effectAllowed = 'move'; - e.dataTransfer.setData('application/x-recipient-chip', JSON.stringify({ chip, fromField: field })); + e.dataTransfer.setData('application/x-recipient-chip', JSON.stringify({ recipient: chip, fromField: field })); // Show the address while dragging, matching the email-list drag preview. - const dragPreview = createChipDragPreview(parseChip(chip).email); + const dragPreview = createChipDragPreview(chip.email); e.dataTransfer.setDragImage(dragPreview, 0, 0); requestAnimationFrame(() => dragPreview.remove()); setDraggingIndex(i); @@ -3159,7 +3146,7 @@ function RecipientChipInput({ {contextMenu.data && ( <>
- {formatChipDisplay(contextMenu.data.chip)} + {formatChipDisplay(contextMenu.data.recipient)}
diff --git a/lib/__tests__/email-composer-utils.test.ts b/lib/__tests__/email-composer-utils.test.ts index 9b5a407c..9fdcbde2 100644 --- a/lib/__tests__/email-composer-utils.test.ts +++ b/lib/__tests__/email-composer-utils.test.ts @@ -4,8 +4,11 @@ import { rewriteCidImagesForEditor, replaceInlineImagePlaceholders, INLINE_IMAGE_PLACEHOLDER, - removeChipFromFieldValue, - addChipToFieldValue, + splitRecipients, + formatRecipient, + parseRecipient, + parseRecipientList, + formatRecipientList, } from "../email-composer-utils"; describe("plainTextToComposerBody", () => { @@ -115,59 +118,83 @@ describe("replaceInlineImagePlaceholders", () => { }); }); -describe("removeChipFromFieldValue", () => { - it("removes the target chip and preserves others", () => { - const result = removeChipFromFieldValue("alice@example.com, bob@example.com, ", "alice@example.com"); - expect(result).toBe("bob@example.com, "); +describe("splitRecipients", () => { + it("splits a plain comma-separated list", () => { + expect(splitRecipients("alice@x.com, bob@x.com")).toEqual([ + "alice@x.com", + "bob@x.com", + ]); }); - it("removes a chip with a display name", () => { - const result = removeChipFromFieldValue("Alice , bob@example.com, ", "Alice "); - expect(result).toBe("bob@example.com, "); + it("trims whitespace and drops empty segments", () => { + expect(splitRecipients(" alice@x.com ,, bob@x.com ,")).toEqual([ + "alice@x.com", + "bob@x.com", + ]); }); - it("handles removing the only chip", () => { - const result = removeChipFromFieldValue("alice@example.com, ", "alice@example.com"); - expect(result).toBe(""); + it("keeps a quoted display name containing a comma intact", () => { + expect(splitRecipients('"Doo, John" , alice@x.com')).toEqual([ + '"Doo, John" ', + "alice@x.com", + ]); }); - it("returns the value unchanged when chip is not found", () => { - const value = "alice@example.com, bob@example.com, "; - expect(removeChipFromFieldValue(value, "carol@example.com")).toBe(value); + it("does not split on a comma inside angle brackets", () => { + expect(splitRecipients("Group , c@x.com")).toEqual([ + "Group ", + "c@x.com", + ]); }); - it("preserves in-progress input text after removing a chip", () => { - const result = removeChipFromFieldValue("alice@example.com, bob@example.com, car", "alice@example.com"); - expect(result).toBe("bob@example.com, car"); - }); - - it("handles an empty field value", () => { - expect(removeChipFromFieldValue("", "alice@example.com")).toBe(""); - }); - - it("removes only the first occurrence when chip appears multiple times", () => { - const result = removeChipFromFieldValue("alice@example.com, alice@example.com, bob@example.com, ", "alice@example.com"); - expect(result).toBe("alice@example.com, bob@example.com, "); + it("returns an empty array for an empty string", () => { + expect(splitRecipients("")).toEqual([]); }); }); -describe("addChipToFieldValue", () => { - it("appends a chip to a field with existing chips", () => { - const result = addChipToFieldValue("alice@example.com, ", "bob@example.com"); - expect(result).toBe("alice@example.com, bob@example.com, "); +describe("formatRecipient / parseRecipient", () => { + it("returns a bare email when there is no name", () => { + expect(formatRecipient(undefined, "a@x.com")).toBe("a@x.com"); }); - it("appends a chip to an empty field", () => { - expect(addChipToFieldValue("", "alice@example.com")).toBe("alice@example.com, "); + it("returns a bare email when the name equals the email", () => { + expect(formatRecipient("a@x.com", "a@x.com")).toBe("a@x.com"); }); - it("preserves in-progress input text when appending", () => { - const result = addChipToFieldValue("alice@example.com, bob", "carol@example.com"); - expect(result).toBe("alice@example.com, carol@example.com, bob"); + it("formats a simple name without quoting", () => { + expect(formatRecipient("Alice", "a@x.com")).toBe("Alice "); }); - it("appends a chip with a display name", () => { - const result = addChipToFieldValue("alice@example.com, ", "Bob "); - expect(result).toBe("alice@example.com, Bob , "); + it("quotes a name containing a comma", () => { + expect(formatRecipient("Doo, John", "john@doo.org")).toBe( + '"Doo, John" ' + ); + }); + + it("parses a bare email", () => { + expect(parseRecipient("a@x.com")).toEqual({ email: "a@x.com" }); + }); + + it("parses and unquotes a quoted comma name", () => { + expect(parseRecipient('"Doo, John" ')).toEqual({ + name: "Doo, John", + email: "john@doo.org", + }); + }); +}); + +describe("parseRecipientList / formatRecipientList", () => { + it("round-trips a comma-name recipient through serialize + parse", () => { + const list = [ + { name: "Doo, John", email: "john@doo.org" }, + { email: "alice@x.com" }, + ]; + const serialized = formatRecipientList(list); + expect(serialized).toBe('"Doo, John" , alice@x.com'); + expect(parseRecipientList(serialized)).toEqual(list); + }); + + it("parses an empty string to an empty array", () => { + expect(parseRecipientList("")).toEqual([]); }); }); diff --git a/lib/email-composer-utils.ts b/lib/email-composer-utils.ts index ed1f2d13..4f869074 100644 --- a/lib/email-composer-utils.ts +++ b/lib/email-composer-utils.ts @@ -52,37 +52,92 @@ export function rewriteCidImagesForEditor(html: string): string { return touched ? doc.body.innerHTML : html; } +/** A composer recipient. Display name is optional; email is required. */ +export type Recipient = { name?: string; email: string }; + /** - * Parses the chip array and trailing in-progress input text from a - * comma-separated recipient field value (e.g. "Alice , bob@x.com, b"). - * A trailing comma means "bob@x.com" is a committed chip and "b" is the live input. + * Splits a comma-separated recipient string into individual entries. Commas + * inside a quoted display name (`"Doo, John" `) or angle brackets + * (``) are treated as literal, not separators. Only used at the + * (de)serialization boundary — the live composer state is an array, so the UI + * never round-trips through this. Trims each part and drops empties. */ -function parseFieldValue(fieldValue: string): { chips: string[]; inputText: string } { - const allParts = fieldValue.split(',').map(s => s.trim()).filter(Boolean); - const hasTrailingComma = fieldValue.trimEnd().endsWith(','); - const chips = hasTrailingComma ? allParts : allParts.slice(0, -1); - const inputText = hasTrailingComma ? '' : (allParts[allParts.length - 1] ?? ''); - return { chips, inputText }; +export function splitRecipients(value: string): string[] { + const result: string[] = []; + let current = ''; + let inQuotes = false; + let inAngle = false; + for (const ch of value) { + if (ch === '"') { + inQuotes = !inQuotes; + current += ch; + } else if (ch === '<' && !inQuotes) { + inAngle = true; + current += ch; + } else if (ch === '>' && !inQuotes) { + inAngle = false; + current += ch; + } else if (ch === ',' && !inQuotes && !inAngle) { + const trimmed = current.trim(); + if (trimmed) result.push(trimmed); + current = ''; + } else { + current += ch; + } + } + const trimmed = current.trim(); + if (trimmed) result.push(trimmed); + return result; } -function buildFieldValue(chips: string[], inputText: string): string { - if (chips.length === 0) return inputText; - return chips.join(', ') + ', ' + inputText; +// Display names containing any of these must be wrapped in a quoted-string so +// they survive comma-splitting at the serialization boundary and round-trip. +const NAME_NEEDS_QUOTING = /[,<>"@;:]/; + +/** + * Formats a recipient as a string. Bare email when there's no distinct name; + * otherwise `Name `, RFC 5322 quoting the name when it contains a comma + * or other special character. + */ +export function formatRecipient(name: string | undefined, email: string): string { + const trimmedName = name?.trim(); + if (!trimmedName || trimmedName === email) return email; + const quoted = NAME_NEEDS_QUOTING.test(trimmedName) + ? `"${trimmedName.replace(/(["\\])/g, '\\$1')}"` + : trimmedName; + return `${quoted} <${email}>`; } -/** Removes the first occurrence of `chip` from a recipient field value string. */ -export function removeChipFromFieldValue(fieldValue: string, chip: string): string { - const { chips, inputText } = parseFieldValue(fieldValue); - const idx = chips.indexOf(chip); - if (idx === -1) return fieldValue; - const remaining = chips.filter((_, i) => i !== idx); - return buildFieldValue(remaining, inputText); +/** Strips a surrounding quoted-string (and its escapes) from a display name. */ +function unquoteName(name: string): string { + const trimmed = name.trim(); + if (trimmed.length >= 2 && trimmed.startsWith('"') && trimmed.endsWith('"')) { + return trimmed.slice(1, -1).replace(/\\(["\\])/g, '$1'); + } + return trimmed; } -/** Appends `chip` as a committed entry to a recipient field value string. */ -export function addChipToFieldValue(fieldValue: string, chip: string): string { - const { chips, inputText } = parseFieldValue(fieldValue); - return buildFieldValue([...chips, chip], inputText); +/** + * Parses a single recipient string (`Name `, `"Quoted, Name" `, + * or bare `email`) into a {@link Recipient}. The display name is unquoted. + */ +export function parseRecipient(s: string): Recipient { + const trimmed = s.trim(); + const angleMatch = trimmed.match(/^(.+?)\s*<([^>]+)>$/); + if (angleMatch) { + return { name: unquoteName(angleMatch[1]), email: angleMatch[2].trim() }; + } + return { email: trimmed }; +} + +/** Parses a serialized comma-separated recipient string into an array. */ +export function parseRecipientList(value: string): Recipient[] { + return splitRecipients(value).map(parseRecipient); +} + +/** Serializes a recipient array into a comma-separated string. */ +export function formatRecipientList(recipients: Recipient[]): string { + return recipients.map((r) => formatRecipient(r.name, r.email)).join(', '); } /** From 84aced7b4eaded56308dc4b298233b9ec3e18bd9 Mon Sep 17 00:00:00 2001 From: Stefan Hildebrandt <695494+hildebrandttk@users.noreply.github.com> Date: Mon, 15 Jun 2026 18:33:03 +0200 Subject: [PATCH 2/4] test(dev-mock): use comma display names in a mock email Give email-002 ("Project Update - Q1 Review") a sender and CC with "Lastname, Firstname" display names so Reply/Reply-All in dev mode exercises the comma-in-name recipient case end to end. --- app/api/dev-jmap/[...path]/route.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/api/dev-jmap/[...path]/route.ts b/app/api/dev-jmap/[...path]/route.ts index 74d5971b..351c3c72 100644 --- a/app/api/dev-jmap/[...path]/route.ts +++ b/app/api/dev-jmap/[...path]/route.ts @@ -121,9 +121,9 @@ const emails: MockEmail[] = [ }, { id: 'email-002', threadId: 'thread-002', mailboxIds: { 'mb-inbox': true }, keywords: { $seen: true, $flagged: true, '$label:blue': true }, size: 5100, receivedAt: daysAgo(1), - from: [{ name: 'Pierre Dubois', email: 'pierre@dubois.example' }], + from: [{ name: 'Dubois, Pierre', email: 'pierre@dubois.example' }], to: [{ name: 'Dev User', email: 'dev@localhost' }], - cc: [{ name: 'Karel de Vries', email: 'karel@devries.example' }], + cc: [{ name: 'de Vries, Karel', email: 'karel@devries.example' }], subject: 'Project Update - Q1 Review', preview: 'Salut team, I wanted to share the latest project numbers. We are on track to meet our targets for Q1.', hasAttachment: true, From f7d4f9d53c45900d9f83e8aebe840c9ec90b8b6e Mon Sep 17 00:00:00 2001 From: Max Hao Date: Tue, 9 Jun 2026 17:07:18 -0400 Subject: [PATCH 3/4] fix: prevent draft emails from being marked as unread --- lib/jmap/client.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/jmap/client.ts b/lib/jmap/client.ts index 501de5bb..4bf02286 100644 --- a/lib/jmap/client.ts +++ b/lib/jmap/client.ts @@ -2175,7 +2175,7 @@ export class JMAPClient implements IJMAPClient { cc: cc?.length ? cc.map(email => ({ email })) : undefined, bcc: bcc?.length ? bcc.map(email => ({ email })) : undefined, subject, - keywords: { "$draft": true }, + keywords: { "$seen": true, "$draft": true }, mailboxIds: { [draftsMailbox.id]: true }, bodyValues: htmlBody ? { "text": { value: body }, "html": { value: htmlBody } } @@ -6259,6 +6259,7 @@ export class JMAPClient implements IJMAPClient { const update: Record = { [`mailboxIds/${draftMailboxId}`]: true, 'keywords/$draft': true, + 'keywords/$seen': true, }; if (sentMailboxId) { update[`mailboxIds/${sentMailboxId}`] = null; From fd700f412e30b999a2b424ba7f764f1345b8274c Mon Sep 17 00:00:00 2001 From: Max Hao Date: Thu, 11 Jun 2026 19:49:12 -0400 Subject: [PATCH 4/4] fix: fix inconsistent behavior with threading email messages in the inbox/folders --- app/(main)/[locale]/page.tsx | 39 ++++- components/email/email-list.tsx | 10 +- components/pro/pro-compose-tab-body.tsx | 20 ++- lib/demo/demo-client.ts | 10 ++ lib/jmap/client-interface.ts | 1 + lib/jmap/client.ts | 18 ++ lib/thread-utils.ts | 12 +- stores/email-store.ts | 212 +++++++++++++++++++++++- 8 files changed, 309 insertions(+), 13 deletions(-) diff --git a/app/(main)/[locale]/page.tsx b/app/(main)/[locale]/page.tsx index 1d081eb9..a575d1e2 100644 --- a/app/(main)/[locale]/page.tsx +++ b/app/(main)/[locale]/page.tsx @@ -335,6 +335,7 @@ export default function Home() { viewingAccountId, selectAccountMailbox, setViewingAccount, + refreshCurrentMailbox, } = useEmailStore(); // Pro shell: populate per-account mailbox cache so the sidebar can render @@ -1168,7 +1169,26 @@ export default function Home() { } // Refresh the current mailbox to update the UI - if (!isScheduledView) await fetchEmails(client, selectedMailbox); + if (!isScheduledView) { + await refreshCurrentMailbox(client); + // Re-fetch the replied thread's cross-folder data so the expanded + // view shows the newly sent reply without collapsing. + if (originalEmailId) { + const emailState = useEmailStore.getState(); + const repliedEmail = emailState.emails.find(e => e.id === originalEmailId); + if (repliedEmail?.threadId && emailState.expandedThreadIds.has(repliedEmail.threadId)) { + const accountId = client.getAccountId(); + const fullEmails = await client.getThreadEmails(repliedEmail.threadId, accountId); + if (fullEmails.length > 0) { + useEmailStore.setState((state) => { + const c = new Map(state.threadEmailsCache); + c.set(repliedEmail.threadId!, fullEmails); + return { threadEmailsCache: c }; + }); + } + } + } + } } catch (error) { console.error("Failed to send email:", error); } @@ -2201,7 +2221,22 @@ export default function Home() { } // Refresh emails to show the sent reply - await fetchEmails(client, selectedMailbox); + await refreshCurrentMailbox(client); + // Re-fetch the replied thread's cross-folder data so the expanded + // view shows the newly sent reply without collapsing. + const emailState = useEmailStore.getState(); + const repliedEmail = emailState.emails.find(e => e.id === originalEmailId); + if (repliedEmail?.threadId && emailState.expandedThreadIds.has(repliedEmail.threadId)) { + const accountId = client.getAccountId(); + const fullEmails = await client.getThreadEmails(repliedEmail.threadId, accountId); + if (fullEmails.length > 0) { + useEmailStore.setState((state) => { + const c = new Map(state.threadEmailsCache); + c.set(repliedEmail.threadId!, fullEmails); + return { threadEmailsCache: c }; + }); + } + } }; // Show loading state while checking auth diff --git a/components/email/email-list.tsx b/components/email/email-list.tsx index 80753f58..5a781e85 100644 --- a/components/email/email-list.tsx +++ b/components/email/email-list.tsx @@ -100,6 +100,8 @@ export function EmailList({ isLoadingThread, toggleThreadExpansion, fetchThreadEmails, + markThreadAsRead, + threadEmailCounts, searchFilters, setSearchFilters, clearSearchFilters, @@ -110,9 +112,9 @@ export function EmailList({ const disableThreading = useSettingsStore((state) => state.disableThreading); const threadGroups = useMemo(() => { - const groups = groupEmailsByThread(emails, disableThreading || isScheduledView); + const groups = groupEmailsByThread(emails, disableThreading || isScheduledView, threadEmailCounts); return sortThreadGroups(groups); - }, [emails, disableThreading, isScheduledView]); + }, [emails, disableThreading, isScheduledView, threadEmailCounts]); const { contextMenu, openContextMenu, closeContextMenu, menuRef } = useContextMenu(); const { dialogProps: confirmDialogProps, confirm: confirmDialog } = useConfirmDialog(); @@ -250,10 +252,12 @@ export function EmailList({ if (!isExpanded && client) { toggleThreadExpansion(threadId); await fetchThreadEmails(client, threadId); + // Mark all unread emails in this thread as read + void markThreadAsRead(client, threadId); } else { toggleThreadExpansion(threadId); } - }, [client, expandedThreadIds, toggleThreadExpansion, fetchThreadEmails]); + }, [client, expandedThreadIds, toggleThreadExpansion, fetchThreadEmails, markThreadAsRead]); // Range-based load more: trigger when last visible item is near the end. // Debounce to prevent rapid cascade when thread grouping reduces item diff --git a/components/pro/pro-compose-tab-body.tsx b/components/pro/pro-compose-tab-body.tsx index ecbca751..49dd46d1 100644 --- a/components/pro/pro-compose-tab-body.tsx +++ b/components/pro/pro-compose-tab-body.tsx @@ -26,6 +26,7 @@ export function ProComposeTabBody({ tabId, data }: ProComposeTabBodyProps) { const client = useAuthStore((s) => s.client); const sendEmail = useEmailStore((s) => s.sendEmail); const fetchEmails = useEmailStore((s) => s.fetchEmails); + const refreshCurrentMailbox = useEmailStore((s) => s.refreshCurrentMailbox); const fetchScheduledEmails = useEmailStore((s) => s.fetchScheduledEmails); const refreshScheduledMetadata = useEmailStore((s) => s.refreshScheduledMetadata); const selectedMailbox = useEmailStore((s) => s.selectedMailbox); @@ -92,7 +93,24 @@ export function ProComposeTabBody({ tabId, data }: ProComposeTabBodyProps) { // Refresh the currently-active mail list so the new sent message / // updated keyword status shows up. - await fetchEmails(client, selectedMailbox); + await refreshCurrentMailbox(client); + // Re-fetch the replied thread's cross-folder data so the expanded + // view shows the newly sent reply without collapsing. + if (data.sourceEmailId) { + const emailState = useEmailStore.getState(); + const repliedEmail = emailState.emails.find(e => e.id === data.sourceEmailId); + if (repliedEmail?.threadId && emailState.expandedThreadIds.has(repliedEmail.threadId)) { + const accountId = client.getAccountId(); + const fullEmails = await client.getThreadEmails(repliedEmail.threadId, accountId); + if (fullEmails.length > 0) { + useEmailStore.setState((state) => { + const c = new Map(state.threadEmailsCache); + c.set(repliedEmail.threadId!, fullEmails); + return { threadEmailsCache: c }; + }); + } + } + } closeTab(tabIdRef.current); } catch (error) { console.error('Failed to send email:', error); diff --git a/lib/demo/demo-client.ts b/lib/demo/demo-client.ts index 2288490d..cc20d52d 100644 --- a/lib/demo/demo-client.ts +++ b/lib/demo/demo-client.ts @@ -388,6 +388,16 @@ export class DemoJMAPClient implements IJMAPClient { return { id: threadId, emailIds: emails.map(e => e.id) }; } + async getThreads(threadIds: string[]): Promise { + return threadIds + .map(tid => { + const emails = this.data.emails.filter(e => e.threadId === tid); + if (emails.length === 0) return null; + return { id: tid, emailIds: emails.map(e => e.id) }; + }) + .filter((t): t is Thread => t !== null); + } + async getThreadEmails(threadId: string): Promise { return this.data.emails .filter(e => e.threadId === threadId) diff --git a/lib/jmap/client-interface.ts b/lib/jmap/client-interface.ts index 6a55a8f5..0624861e 100644 --- a/lib/jmap/client-interface.ts +++ b/lib/jmap/client-interface.ts @@ -117,6 +117,7 @@ export interface IJMAPClient { // ── Threads ─────────────────────────────────────────────────── getThread(threadId: string, accountId?: string): Promise; + getThreads(threadIds: string[], accountId?: string): Promise; getThreadEmails(threadId: string, accountId?: string): Promise; // ── Compose / Send ──────────────────────────────────────────── diff --git a/lib/jmap/client.ts b/lib/jmap/client.ts index 4bf02286..7440d54c 100644 --- a/lib/jmap/client.ts +++ b/lib/jmap/client.ts @@ -1904,6 +1904,24 @@ export class JMAPClient implements IJMAPClient { } } + async getThreads(threadIds: string[], accountId?: string): Promise { + if (threadIds.length === 0) return []; + try { + const targetAccountId = accountId || this.accountId; + const response = await this.request([ + ["Thread/get", { accountId: targetAccountId, ids: threadIds }, "0"], + ]); + + if (response.methodResponses?.[0]?.[0] === "Thread/get") { + return (response.methodResponses[0][1].list || []) as Thread[]; + } + return []; + } catch (error) { + console.error('Failed to get threads:', error); + return []; + } + } + async getThreadEmails(threadId: string, accountId?: string): Promise { try { const targetAccountId = accountId || this.accountId; diff --git a/lib/thread-utils.ts b/lib/thread-utils.ts index 65dfbae0..45ced51d 100644 --- a/lib/thread-utils.ts +++ b/lib/thread-utils.ts @@ -5,8 +5,16 @@ import type { Email, ThreadGroup } from "./jmap/types"; * Single-email threads are still returned as ThreadGroups with emailCount=1. * When disableThreading is true, each email is placed into its own group using * its message ID as the key, so the list shows individual messages. + * + * @param threadEmailCounts - Optional map of threadId → total email count across + * all folders (from Thread/get). When provided, emailCount reflects the full + * thread size rather than just the emails in the current folder. */ -export function groupEmailsByThread(emails: Email[], disableThreading = false): ThreadGroup[] { +export function groupEmailsByThread( + emails: Email[], + disableThreading = false, + threadEmailCounts?: Map, +): ThreadGroup[] { if (!emails || emails.length === 0) { return []; } @@ -53,7 +61,7 @@ export function groupEmailsByThread(emails: Email[], disableThreading = false): hasAttachment, hasAnswered, hasForwarded, - emailCount: sortedEmails.length, + emailCount: threadEmailCounts?.get(threadId) ?? sortedEmails.length, }); } diff --git a/stores/email-store.ts b/stores/email-store.ts index 53b25249..498e6173 100644 --- a/stores/email-store.ts +++ b/stores/email-store.ts @@ -60,6 +60,8 @@ interface EmailStore { expandedThreadIds: Set; threadEmailsCache: Map; isLoadingThread: string | null; + // Full thread email counts (from Thread/get across all folders) + threadEmailCounts: Map; // Keyword/tag filter selectedKeyword: string | null; @@ -199,6 +201,8 @@ interface EmailStore { fetchThreadEmails: (client: IJMAPClient, threadId: string) => Promise; collapseAllThreads: () => void; updateThreadCache: (threadId: string, emails: Email[]) => void; + fetchThreadEmailCounts: (client: IJMAPClient) => Promise; + markThreadAsRead: (client: IJMAPClient, threadId: string) => Promise; // Mailbox management createMailbox: (client: IJMAPClient, name: string, parentId?: string) => Promise; @@ -519,6 +523,7 @@ export const useEmailStore = create((set, get) => ({ expandedThreadIds: new Set(), threadEmailsCache: new Map(), isLoadingThread: null, + threadEmailCounts: new Map(), // Keyword/tag filter selectedKeyword: null, @@ -565,6 +570,7 @@ export const useEmailStore = create((set, get) => ({ selectedKeyword: null, expandedThreadIds: new Set(), threadEmailsCache: new Map(), + threadEmailCounts: new Map(), isLoadingThread: null, }), fetchAccountMailboxes: async (client, accountId) => { @@ -595,6 +601,7 @@ export const useEmailStore = create((set, get) => ({ selectedEmailIds: new Set(), expandedThreadIds: new Set(), threadEmailsCache: new Map(), + threadEmailCounts: new Map(), }), fetchTagCounts: async (client) => { try { @@ -617,6 +624,7 @@ export const useEmailStore = create((set, get) => ({ selectedKeyword: null, expandedThreadIds: new Set(), threadEmailsCache: new Map(), + threadEmailCounts: new Map(), isLoadingThread: null, }), setLoading: (loading) => set({ isLoading: loading }), @@ -785,8 +793,14 @@ export const useEmailStore = create((set, get) => ({ emails: annotateScheduledEmails(result.emails, get().scheduledSubmissionByEmailId), hasMoreEmails: result.hasMore, totalEmails: result.total, + // Clear thread caches since the email list was fully replaced + threadEmailsCache: new Map(), + expandedThreadIds: new Set(), + isLoadingThread: null, isLoading: false }); + // Fetch full thread counts in the background (non-blocking) + void get().fetchThreadEmailCounts(client); } catch (error) { console.error('Failed to fetch emails:', error); set({ @@ -923,6 +937,10 @@ export const useEmailStore = create((set, get) => ({ totalEmails: result.total, isLoadingMore: false }); + // Fetch full thread counts for newly loaded threads in the background + if (newEmails.length > 0) { + void get().fetchThreadEmailCounts(client); + } } catch (error) { console.error('Failed to load more emails:', error); set({ @@ -1214,7 +1232,23 @@ export const useEmailStore = create((set, get) => ({ ? { ...state.selectedEmail, keywords: { ...state.selectedEmail.keywords, $seen: read } } : state.selectedEmail, mailboxes: updatedMailboxes, - processingReadStatus: newProcessingSet + processingReadStatus: newProcessingSet, + // Also update threadEmailsCache so expanded dropdowns reflect the change + threadEmailsCache: (() => { + let updated = false; + const newCache = new Map(state.threadEmailsCache); + for (const [tid, cachedEmails] of newCache) { + const idx = cachedEmails.findIndex(e => e.id === emailId); + if (idx !== -1) { + newCache.set(tid, cachedEmails.map((e, i) => + i === idx ? { ...e, keywords: { ...e.keywords, $seen: read } } : e + )); + updated = true; + break; + } + } + return updated ? newCache : state.threadEmailsCache; + })(), }; }); } catch (error) { @@ -2327,11 +2361,74 @@ export const useEmailStore = create((set, get) => ({ // hasMore should reflect whether there are still more emails beyond // what we have loaded, using the fresh total from the server. const hasMore = merged.length < (result.total || 0); - set({ - emails: merged, - hasMoreEmails: hasMore, - totalEmails: result.total, + + // Invalidate thread email caches for threads whose composition changed + // so expanded threads pick up new/removed emails. + const prevThreadIds = new Set(currentEmails.map(e => e.threadId)); + const nextThreadIds = new Set(merged.map(e => e.threadId)); + const changedThreadIds = new Set(); + for (const tid of prevThreadIds) { + if (!nextThreadIds.has(tid)) changedThreadIds.add(tid); + } + for (const tid of nextThreadIds) { + if (!prevThreadIds.has(tid)) changedThreadIds.add(tid); + } + // Also check threads where the set of email IDs changed + const prevEmailsByThread = new Map>(); + for (const e of currentEmails) { + if (!prevEmailsByThread.has(e.threadId)) prevEmailsByThread.set(e.threadId, new Set()); + prevEmailsByThread.get(e.threadId)!.add(e.id); + } + const nextEmailsByThread = new Map>(); + for (const e of merged) { + if (!nextEmailsByThread.has(e.threadId)) nextEmailsByThread.set(e.threadId, new Set()); + nextEmailsByThread.get(e.threadId)!.add(e.id); + } + for (const [tid, nextIds] of nextEmailsByThread) { + const prevIds = prevEmailsByThread.get(tid); + if (!prevIds || prevIds.size !== nextIds.size) { + changedThreadIds.add(tid); + } else { + for (const id of nextIds) { + if (!prevIds.has(id)) { changedThreadIds.add(tid); break; } + } + } + } + + set((state) => { + const newCache = new Map(state.threadEmailsCache); + for (const tid of changedThreadIds) { + newCache.delete(tid); + } + return { + emails: merged, + hasMoreEmails: hasMore, + totalEmails: result.total, + threadEmailsCache: newCache, + }; }); + + // Re-fetch cross-folder thread data for any currently expanded threads + // so they show the complete conversation (not just current-folder emails). + const expandedNow = get().expandedThreadIds; + if (expandedNow.size > 0) { + const effectiveClient2 = resolveActionClient(client); + const accountId = effectiveClient2.getAccountId(); + for (const tid of expandedNow) { + void effectiveClient2.getThreadEmails(tid, accountId).then((fullEmails) => { + if (fullEmails.length > 0) { + set((state) => { + const c = new Map(state.threadEmailsCache); + c.set(tid, fullEmails); + return { threadEmailsCache: c }; + }); + } + }); + } + } + + // Fetch full thread counts in the background (non-blocking) + void get().fetchThreadEmailCounts(client); } } catch (error) { console.error('Failed to refresh current mailbox:', error); @@ -2401,6 +2498,90 @@ export const useEmailStore = create((set, get) => ({ } }, + markThreadAsRead: async (client, threadId) => { + const state = get(); + const threadEmails = state.threadEmailsCache.get(threadId) ?? []; + const mainEmails = state.emails.filter(e => e.threadId === threadId); + + // Combine unique emails from both sources + const allEmailMap = new Map(); + for (const e of mainEmails) allEmailMap.set(e.id, e); + for (const e of threadEmails) allEmailMap.set(e.id, e); + + const unreadIds = Array.from(allEmailMap.values()) + .filter(e => !e.keywords?.$seen) + .map(e => e.id); + + if (unreadIds.length === 0) return; + + // Group by account for unified view support + const emailsById = new Map(); + for (const e of allEmailMap.values()) emailsById.set(e.id, e); + + // Group unread IDs by account client + const groups = new Map(); + for (const id of unreadIds) { + const email = emailsById.get(id)!; + const { client: actionClient } = resolveEmailActionContext(email, client); + if (!groups.has(actionClient)) groups.set(actionClient, []); + groups.get(actionClient)!.push(id); + } + + // Mark all as read on the server + try { + await Promise.all( + Array.from(groups.entries()).map(([actionClient, emailIds]) => + actionClient.batchMarkAsRead(emailIds, true) + ) + ); + } catch (error) { + console.error('Failed to mark thread as read:', error); + return; + } + + // Update local state + set((state) => { + const unreadSet = new Set(unreadIds); + + const updatedEmails = state.emails.map(e => + unreadSet.has(e.id) ? { ...e, keywords: { ...e.keywords, $seen: true } } : e + ); + + // Update threadEmailsCache + const newCache = new Map(state.threadEmailsCache); + const cached = newCache.get(threadId); + if (cached) { + newCache.set(threadId, cached.map(e => + unreadSet.has(e.id) ? { ...e, keywords: { ...e.keywords, $seen: true } } : e + )); + } + + // Update mailbox unread counters + const affectedEmails = state.emails.filter(e => unreadSet.has(e.id)); + const updatedMailboxes = state.mailboxes.map(mailbox => { + let delta = 0; + for (const email of affectedEmails) { + if (email.mailboxIds?.[mailbox.id]) delta -= 1; + } + if (delta === 0) return mailbox; + return { + ...mailbox, + unreadEmails: Math.max(0, mailbox.unreadEmails + delta), + unreadThreads: Math.max(0, mailbox.unreadThreads + delta), + }; + }); + + return { + emails: updatedEmails, + threadEmailsCache: newCache, + mailboxes: updatedMailboxes, + selectedEmail: state.selectedEmail && unreadSet.has(state.selectedEmail.id) + ? { ...state.selectedEmail, keywords: { ...state.selectedEmail.keywords, $seen: true } } + : state.selectedEmail, + }; + }); + }, + collapseAllThreads: () => { set({ expandedThreadIds: new Set(), @@ -2414,6 +2595,27 @@ export const useEmailStore = create((set, get) => ({ set({ threadEmailsCache: newCache }); }, + fetchThreadEmailCounts: async (client) => { + const { emails } = get(); + if (emails.length === 0) return; + + const uniqueThreadIds = [...new Set(emails.map(e => e.threadId).filter(Boolean))]; + if (uniqueThreadIds.length === 0) return; + + try { + const effectiveClient = resolveActionClient(client); + const threads = await effectiveClient.getThreads(uniqueThreadIds); + + const newCounts = new Map(get().threadEmailCounts); + for (const thread of threads) { + newCounts.set(thread.id, thread.emailIds?.length ?? 0); + } + set({ threadEmailCounts: newCounts }); + } catch { + // Non-critical — fall back to inbox-only counts + } + }, + // Mailbox management createMailbox: async (client, name, parentId) => { try {