diff --git a/app/(main)/[locale]/page.tsx b/app/(main)/[locale]/page.tsx index cd844fc7..a4627415 100644 --- a/app/(main)/[locale]/page.tsx +++ b/app/(main)/[locale]/page.tsx @@ -34,6 +34,7 @@ import { debug } from "@/lib/debug"; import { playNotificationSound } from "@/lib/notification-sound"; import { cn } from "@/lib/utils"; import { localizeMailboxName } from "@/lib/mailbox-label"; +import { KEYWORD_PREFIX, KEYWORD_PREFIX_LEGACY } from "@/lib/thread-utils"; import { ErrorBoundary, SidebarErrorFallback, @@ -1870,18 +1871,22 @@ export default function Home() { if (tagId === null) { // Remove all tag keywords Object.keys(keywords).forEach(key => { - if (key.startsWith("$label:") || key.startsWith("$color:")) { + if (key.startsWith(KEYWORD_PREFIX) || key.startsWith(KEYWORD_PREFIX_LEGACY)) { keywords[key] = false; } }); } else { - const jmapKey = `$label:${tagId}`; - if (keywords[jmapKey]) { - // Toggle off if already active - keywords[jmapKey] = false; + // Both prefixes name the same tag when read, so taking one off has to + // clear whichever spellings are actually set. + const activeKeys = [KEYWORD_PREFIX + tagId, KEYWORD_PREFIX_LEGACY + tagId] + .filter(key => keywords[key]); + if (activeKeys.length > 0) { + activeKeys.forEach(key => { + keywords[key] = false; + }); } else { // Add the tag without disturbing others - keywords[jmapKey] = true; + keywords[KEYWORD_PREFIX + tagId] = true; } } diff --git a/components/email/__tests__/tag-badge.test.tsx b/components/email/__tests__/tag-badge.test.tsx new file mode 100644 index 00000000..70775ff6 --- /dev/null +++ b/components/email/__tests__/tag-badge.test.tsx @@ -0,0 +1,41 @@ +import { render, screen, fireEvent } from '@testing-library/react'; +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { TagBadge } from '../tag-badge'; +import { useSettingsStore, type KeywordDefinition } from '@/stores/settings-store'; + +const TAGS: KeywordDefinition[] = [ + { id: 'work', label: 'Work', color: 'blue' }, + { id: 'work/clients', label: 'Clients', color: 'green' }, +]; + +describe('TagBadge', () => { + beforeEach(() => { + useSettingsStore.setState({ emailKeywords: TAGS, nestedTags: true }); + }); + + it('names the tag by its full path', () => { + render(); + expect(screen.getByText('Work/Clients')).toBeInTheDocument(); + }); + + it('names a tag it has no definition for by its id', () => { + render(); + expect(screen.getByText('from-elsewhere')).toBeInTheDocument(); + }); + + it('offers removal only when asked to', () => { + const onRemove = vi.fn(); + const { rerender } = render(); + expect(screen.queryByRole('button')).not.toBeInTheDocument(); + + rerender(); + fireEvent.click(screen.getByRole('button', { name: 'remove_tag' })); + expect(onRemove).toHaveBeenCalledOnce(); + }); + + it('leaves the dot alone, having nowhere to put the control', () => { + render( {}} />); + expect(screen.queryByRole('button')).not.toBeInTheDocument(); + expect(screen.getByLabelText('Work')).toBeInTheDocument(); + }); +}); diff --git a/components/email/__tests__/tag-picker.test.tsx b/components/email/__tests__/tag-picker.test.tsx index e7c7e664..048d9096 100644 --- a/components/email/__tests__/tag-picker.test.tsx +++ b/components/email/__tests__/tag-picker.test.tsx @@ -53,12 +53,28 @@ describe('TagPicker', () => { expect(onToggle).toHaveBeenCalledWith('work/clients'); }); - it('offers the clear-all row only while something is applied', () => { - const { rerender } = render( {}} onClearAll={() => {}} />); - expect(screen.queryByText('remove_tag')).not.toBeInTheDocument(); + it('lists a tag it has no definition for, so it can be taken off', () => { + const onToggle = vi.fn(); + const { rerender } = render(); - rerender( {}} onClearAll={() => {}} />); - expect(screen.getByText('remove_tag')).toBeInTheDocument(); + const row = screen.getByText('from-elsewhere').closest('button')!; + expect(row).toHaveAttribute('aria-checked', 'true'); + + fireEvent.click(row); + expect(onToggle).toHaveBeenCalledWith('from-elsewhere'); + + // Nothing but the message says it exists, so deselecting is the last of it. + rerender(); + expect(screen.queryByText('from-elsewhere')).not.toBeInTheDocument(); + }); + + it('counts undefined tags towards the filter box, and matches them', () => { + const strays = Array.from({ length: 8 }, (_, i) => `stray-${i}`); + const { container } = render( {}} />); + + fireEvent.change(screen.getByLabelText('tag_filter_placeholder'), { target: { value: 'stray-3' } }); + expect(within(container).getByText('stray-3')).toBeInTheDocument(); + expect(within(container).queryByText('Work')).not.toBeInTheDocument(); }); it('hides the filter box until the list is long enough to need one', () => { diff --git a/components/email/email-context-menu.tsx b/components/email/email-context-menu.tsx index 5c26667d..2cef2b5b 100644 --- a/components/email/email-context-menu.tsx +++ b/components/email/email-context-menu.tsx @@ -36,6 +36,7 @@ import { } from "lucide-react"; import { buildMailboxTree, MailboxNode } from "@/lib/utils"; import { localizeMailboxName } from "@/lib/mailbox-label"; +import { getEmailTagIds } from "@/lib/thread-utils"; import { TagPicker } from "./tag-picker"; interface Position { @@ -99,20 +100,6 @@ const getMailboxIcon = (role?: string) => { } }; -/** Every tag id set on a message, reading the current prefix and the legacy one. */ -const getCurrentTagIds = (keywords: Record | undefined): string[] => { - if (!keywords) return []; - const tags: string[] = []; - for (const key of Object.keys(keywords)) { - if ((key.startsWith("$label:") || key.startsWith("$color:")) && keywords[key] === true) { - tags.push( - key.startsWith("$label:") ? key.slice("$label:".length) : key.slice("$color:".length) - ); - } - } - return tags; -}; - export function EmailContextMenu({ email, position, @@ -155,7 +142,7 @@ export function EmailContextMenu({ const isStarred = email.keywords?.$flagged; const isPinned = email.keywords?.['$pinned'] === true; const isDraft = email.keywords?.['$draft'] === true; - const currentTagIds = getCurrentTagIds(email.keywords); + const currentTagIds = getEmailTagIds(email.keywords); const showBatchActions = isMultiSelect && selectedCount > 1; const isInJunkFolder = currentMailboxRole === 'junk'; // Marking your own outgoing mail as spam makes no sense - hide the action @@ -373,8 +360,7 @@ export function EmailContextMenu({
handleAction(() => onSetTag?.(tagId))} - onClearAll={() => handleAction(() => onSetTag?.(null))} + onToggle={(tagId) => onSetTag?.(tagId)} />
diff --git a/components/email/email-list.tsx b/components/email/email-list.tsx index 4333f72d..4329a53d 100644 --- a/components/email/email-list.tsx +++ b/components/email/email-list.tsx @@ -133,6 +133,14 @@ export function EmailList({ }, [emails, disableThreading, isScheduledView, threadEmailCounts]); const { contextMenu, openContextMenu, closeContextMenu, menuRef } = useContextMenu(); + /** + * The row the menu was opened on, as the list currently has it. The menu holds + * the message it was handed when it opened, but tags can be applied from + * inside it without dismissing it, so what it draws has to keep up. + */ + const contextMenuEmail = contextMenu.data + ? emails.find((email) => email.id === contextMenu.data!.id) ?? contextMenu.data + : null; const { dialogProps: confirmDialogProps, confirm: confirmDialog } = useConfirmDialog(); const [isProcessing, setIsProcessing] = useState(false); @@ -574,9 +582,9 @@ export function EmailList({ {/* Context Menu */} - {contextMenu.data && ( + {contextMenuEmail && ( onReply?.(contextMenu.data!)} - onReplyAll={() => onReplyAll?.(contextMenu.data!)} - onForward={() => onForward?.(contextMenu.data!)} - onForwardAsAttachment={() => onForwardAsAttachment?.(contextMenu.data!)} + onReply={() => onReply?.(contextMenuEmail!)} + onReplyAll={() => onReplyAll?.(contextMenuEmail!)} + onForward={() => onForward?.(contextMenuEmail!)} + onForwardAsAttachment={() => onForwardAsAttachment?.(contextMenuEmail!)} onMarkAsRead={(read) => onMarkAsRead?.(contextMenu.data!, read)} - onToggleStar={() => onToggleStar?.(contextMenu.data!)} - onTogglePinned={onTogglePinned ? () => onTogglePinned(contextMenu.data!) : undefined} - onDelete={() => onDelete?.(contextMenu.data!)} - onArchive={() => onArchive?.(contextMenu.data!)} - onSetTag={(color) => onSetTag?.(contextMenu.data!.id, color)} - onMoveToMailbox={(mailboxId) => onMoveToMailbox?.(contextMenu.data!.id, mailboxId)} - onMarkAsSpam={() => onMarkAsSpam?.(contextMenu.data!)} - onUndoSpam={() => onUndoSpam?.(contextMenu.data!)} - onEditDraft={() => onEditDraft?.(contextMenu.data!)} - onCancelScheduled={onCancelScheduled ? () => onCancelScheduled(contextMenu.data!) : undefined} - onCancelScheduledForEdit={onCancelScheduledForEdit ? () => onCancelScheduledForEdit(contextMenu.data!) : undefined} - onRescheduleScheduled={onRescheduleScheduled ? () => onRescheduleScheduled(contextMenu.data!) : undefined} + onToggleStar={() => onToggleStar?.(contextMenuEmail!)} + onTogglePinned={onTogglePinned ? () => onTogglePinned(contextMenuEmail!) : undefined} + onDelete={() => onDelete?.(contextMenuEmail!)} + onArchive={() => onArchive?.(contextMenuEmail!)} + onSetTag={(color) => onSetTag?.(contextMenuEmail!.id, color)} + onMoveToMailbox={(mailboxId) => onMoveToMailbox?.(contextMenuEmail!.id, mailboxId)} + onMarkAsSpam={() => onMarkAsSpam?.(contextMenuEmail!)} + onUndoSpam={() => onUndoSpam?.(contextMenuEmail!)} + onEditDraft={() => onEditDraft?.(contextMenuEmail!)} + onCancelScheduled={onCancelScheduled ? () => onCancelScheduled(contextMenuEmail!) : undefined} + onCancelScheduledForEdit={onCancelScheduledForEdit ? () => onCancelScheduledForEdit(contextMenuEmail!) : undefined} + onRescheduleScheduled={onRescheduleScheduled ? () => onRescheduleScheduled(contextMenuEmail!) : undefined} onBatchMarkAsRead={(read) => client && batchMarkAsRead(client, read)} onBatchDelete={() => client && batchDelete(client)} onBatchArchive={async () => { diff --git a/components/email/email-viewer.tsx b/components/email/email-viewer.tsx index 8883e444..bbdc0a76 100644 --- a/components/email/email-viewer.tsx +++ b/components/email/email-viewer.tsx @@ -16,6 +16,7 @@ import { TagBadge } from "./tag-badge"; import { TagPicker } from "./tag-picker"; import { useMeasuredTagDisplay } from "@/hooks/use-tag-display"; import { useKeywordFormat } from "@/hooks/use-keyword-format"; +import { getEmailTagIds } from "@/lib/thread-utils"; import { getSecurityStatus, extractListHeaders } from "@/lib/email-headers"; import { emailToReadView } from "@/lib/plugin-projection"; import { generateEmailSource } from "@/lib/email-source"; @@ -204,19 +205,6 @@ const getAttachmentDisplayName = (name: string | null | undefined, mimeType?: st return 'Attachment'; }; -const getCurrentTagIds = (keywords: Record | undefined): string[] => { - if (!keywords) return []; - const tags: string[] = []; - for (const key of Object.keys(keywords)) { - if ((key.startsWith("$label:") || key.startsWith("$color:")) && keywords[key] === true) { - tags.push( - key.startsWith("$label:") ? key.slice("$label:".length) : key.slice("$color:".length) - ); - } - } - return tags; -}; - // Helper function to format recipients with contextual display const _formatRecipients = ( recipients: Array<{ name?: string; email: string }> | undefined, @@ -818,7 +806,7 @@ export function EmailViewer({ const moveMenuRef = useRef(null); const toolbarRef = useRef(null); const [hiddenPriorities, setHiddenPriorities] = useState>(new Set()); - const currentTagIds = getCurrentTagIds(email?.keywords); + const currentTagIds = getEmailTagIds(email?.keywords); const sortedTagIds = sortTagIds(currentTagIds); // The header spans the reading pane, so it measures its own width rather than // inheriting the message list's answer. @@ -3011,8 +2999,7 @@ export function EmailViewer({
{ if (email) onSetTag?.(email.id, tagId); setTagMenuOpen(false); }} - onClearAll={() => { if (email) onSetTag?.(email.id, null); setTagMenuOpen(false); }} + onToggle={(tagId) => { if (email) onSetTag?.(email.id, tagId); }} />
)} @@ -3205,7 +3192,7 @@ export function EmailViewer({ )} {/* Overflow: tag - submenu */} - {emailKeywords.length > 0 && ( + {(emailKeywords.length > 0 || currentTagIds.length > 0) && (
setMoreMenuSub('tag')} onMouseLeave={() => setMoreMenuSub(null)} @@ -3222,8 +3209,7 @@ export function EmailViewer({
{ if (email) onSetTag?.(email.id, tagId); setMoreMenuOpen(false); setMoreMenuSub(null); }} - onClearAll={() => { if (email) onSetTag?.(email.id, null); setMoreMenuOpen(false); setMoreMenuSub(null); }} + onToggle={(tagId) => { if (email) onSetTag?.(email.id, tagId); }} />
)} @@ -3368,7 +3354,7 @@ export function EmailViewer({ {isStarred ? t('tooltips.unstar') : t('tooltips.star')} {/* Tag (opens sub-view) */} - {emailKeywords.length > 0 && ( + {(emailKeywords.length > 0 || currentTagIds.length > 0) && (
@@ -3551,7 +3536,12 @@ export function EmailViewer({ {sortedTagIds.length > 0 && (
{sortedTagIds.map((tagId) => ( - + onSetTag(email.id, tagId) : undefined} + /> ))}
)} diff --git a/components/email/tag-badge.tsx b/components/email/tag-badge.tsx index 9c61f6d8..ca36f35f 100644 --- a/components/email/tag-badge.tsx +++ b/components/email/tag-badge.tsx @@ -1,5 +1,7 @@ "use client"; +import { useTranslations } from "next-intl"; +import { X } from "lucide-react"; import { cn } from "@/lib/utils"; import { useKeywordFormat } from "@/hooks/use-keyword-format"; import { useShortenedText } from "@/hooks/use-shortened-text"; @@ -38,12 +40,19 @@ export const TAG_GROUP_CLASS = "flex shrink-0 items-center gap-1"; export function TagBadge({ tagId, variant, + onRemove, className, }: { tagId: string; variant: TagBadgeVariant; + /** + * Takes the tag off the message. Only the named form offers it - a dot is the + * size of the control it would have to hold. + */ + onRemove?: () => void; className?: string; }) { + const t = useTranslations("email_viewer"); const { tagName, tagNameCandidates, tagColor } = useKeywordFormat(); const [labelRef, shortenedName] = useShortenedText(tagNameCandidates(tagId)); const color = tagColor(tagId); @@ -61,10 +70,9 @@ export function TagBadge({ return ( - {shortenedName} + + {shortenedName} + + {onRemove && ( + + )} ); } diff --git a/components/email/tag-picker.tsx b/components/email/tag-picker.tsx index e2df5e1d..0f0948ef 100644 --- a/components/email/tag-picker.tsx +++ b/components/email/tag-picker.tsx @@ -2,7 +2,7 @@ import { useMemo, useState } from "react"; import { useTranslations } from "next-intl"; -import { Check, Search, X } from "lucide-react"; +import { Check, Search } from "lucide-react"; import { cn } from "@/lib/utils"; import { useSettingsStore } from "@/stores/settings-store"; import { buildKeywordTree, type KeywordNode } from "@/lib/keyword-nesting"; @@ -26,12 +26,10 @@ const SEARCH_THRESHOLD = 10; export function TagPicker({ selectedIds, onToggle, - onClearAll, touch = false, }: { selectedIds: string[]; onToggle: (tagId: string) => void; - onClearAll?: () => void; /** Larger hit areas for the mobile sheet. */ touch?: boolean; }) { @@ -42,15 +40,32 @@ export function TagPicker({ const [query, setQuery] = useState(""); const trimmedQuery = query.trim().toLowerCase(); - const showSearch = keywords.length >= SEARCH_THRESHOLD; + + /** + * Tags on the message this client has no definition for - set from another + * client, or outliving the tag they were made with. Listing them is the only + * way to take one off, and they leave the list as they are deselected because + * nothing but the message itself records that they exist. + */ + const unknownIds = useMemo( + () => + selectedIds + .filter((id) => !keywords.some((keyword) => keyword.id === id)) + .sort((a, b) => tagName(a).localeCompare(tagName(b))), + // `tagName` is rebuilt whenever the definitions or the nesting setting change. + [selectedIds, keywords, tagName], + ); + + const showSearch = keywords.length + unknownIds.length >= SEARCH_THRESHOLD; const matches = useMemo( () => trimmedQuery - ? keywords.filter((keyword) => tagName(keyword.id).toLowerCase().includes(trimmedQuery)) + ? [...keywords.map((keyword) => keyword.id), ...unknownIds].filter((id) => + tagName(id).toLowerCase().includes(trimmedQuery), + ) : [], - // `tagName` is rebuilt whenever the definitions or the nesting setting change. - [keywords, trimmedQuery, tagName], + [keywords, unknownIds, trimmedQuery, tagName], ); const tree = useMemo( @@ -111,28 +126,22 @@ export function TagPicker({
{trimmedQuery ? ( matches.length > 0 ? ( - matches.map((keyword) => renderRow(keyword.id, tagName(keyword.id))) + matches.map((id) => renderRow(id, tagName(id))) ) : (

{t("tag_no_matches")}

) ) : ( - renderBranch(tree) + <> + {renderBranch(tree)} + {unknownIds.length > 0 && ( + <> + {keywords.length > 0 &&
} + {unknownIds.map((id) => renderRow(id, tagName(id)))} + + )} + )}
- - {onClearAll && selectedIds.length > 0 && ( - <> -
- - - )} ); } diff --git a/components/pro/pro-email-tab-body.tsx b/components/pro/pro-email-tab-body.tsx index 2a412f7e..b37a5abe 100644 --- a/components/pro/pro-email-tab-body.tsx +++ b/components/pro/pro-email-tab-body.tsx @@ -16,6 +16,7 @@ import type { Email } from "@/lib/jmap/types"; import { buildReplySubject, buildForwardSubject } from "@/lib/subject-prefix"; import { getQuoteBodies } from "@/lib/email-composer-utils"; import { buildForwardAsAttachmentPayload } from "@/lib/forward-as-attachment"; +import { KEYWORD_PREFIX, KEYWORD_PREFIX_LEGACY } from "@/lib/thread-utils"; interface ProEmailTabBodyProps { tabId: string; @@ -57,7 +58,6 @@ export function ProEmailTabBody({ tabId, data }: ProEmailTabBodyProps) { const moveToMailbox = useEmailStore((s) => s.moveToMailbox); const setEmailKeywordsLocal = useEmailStore((s) => s.setEmailKeywordsLocal); const mailboxes = useEmailStore((s) => s.mailboxes); - const settingsKeywords = useSettingsStore((s) => s.emailKeywords); const identities = useIdentityStore((s) => s.identities); const multiAccountIdentities = useProMultiAccountIdentities(); @@ -238,18 +238,29 @@ export function ProEmailTabBody({ tabId, data }: ProEmailTabBodyProps) { const handleSetTag = useCallback((emailId: string, tagId: string | null) => { if (!email || email.id !== emailId) return; - // Drop existing color keywords, optionally add the new one. Matches the - // mail page's local optimistic update. + // Toggle one tag, or clear them all. Matches the mail page's local + // optimistic update, down to reaching tags this client cannot name. const keywords = { ...(email.keywords ?? {}) }; - for (const kw of settingsKeywords) { - delete keywords[`$label:${kw.id}`]; - } - if (tagId) { - keywords[`$label:${tagId}`] = true; + if (tagId === null) { + for (const key of Object.keys(keywords)) { + if (key.startsWith(KEYWORD_PREFIX) || key.startsWith(KEYWORD_PREFIX_LEGACY)) { + keywords[key] = false; + } + } + } else { + const activeKeys = [KEYWORD_PREFIX + tagId, KEYWORD_PREFIX_LEGACY + tagId] + .filter(key => keywords[key]); + if (activeKeys.length > 0) { + for (const key of activeKeys) { + keywords[key] = false; + } + } else { + keywords[KEYWORD_PREFIX + tagId] = true; + } } setEmailKeywordsLocal(emailId, keywords); setEmail({ ...email, keywords }); - }, [email, settingsKeywords, setEmailKeywordsLocal]); + }, [email, setEmailKeywordsLocal]); const handleMoveToMailbox = useCallback(async (mailboxId: string) => { if (!client || !email) return; diff --git a/lib/__tests__/thread-utils.test.ts b/lib/__tests__/thread-utils.test.ts index 5de19307..bc65d3e2 100644 --- a/lib/__tests__/thread-utils.test.ts +++ b/lib/__tests__/thread-utils.test.ts @@ -5,6 +5,7 @@ import { getThreadParticipants, mergeThreadEmails, getEmailTagId, + getEmailTagIds, getThreadTagId, getThreadTagIds, } from '../thread-utils'; @@ -246,6 +247,30 @@ describe('mergeThreadEmails', () => { }); }); +describe('getEmailTagIds', () => { + it('gathers every tag set on the message', () => { + expect(getEmailTagIds({ '$label:red': true, '$label:work': true, $seen: true })) + .toEqual(['red', 'work']); + }); + + it('reads the legacy prefix alongside the current one', () => { + expect(getEmailTagIds({ '$label:red': true, '$color:blue': true })).toEqual(['red', 'blue']); + }); + + it('reports a tag written under both prefixes once', () => { + expect(getEmailTagIds({ '$label:red': true, '$color:red': true })).toEqual(['red']); + }); + + it('ignores keywords set to false', () => { + expect(getEmailTagIds({ '$label:red': false, '$label:work': true })).toEqual(['work']); + }); + + it('is empty for an untagged message or none at all', () => { + expect(getEmailTagIds({ $seen: true })).toEqual([]); + expect(getEmailTagIds(undefined)).toEqual([]); + }); +}); + describe('getEmailTagId', () => { it('returns label from $label: keyword', () => { expect(getEmailTagId({ '$label:red': true, $seen: true })).toBe('red'); diff --git a/lib/thread-utils.ts b/lib/thread-utils.ts index 4a152568..7719a2e5 100644 --- a/lib/thread-utils.ts +++ b/lib/thread-utils.ts @@ -170,20 +170,21 @@ export const KEYWORD_PREFIX_LEGACY = "$color:"; /** * Gets every tag id set on a message. * Reads both the current $label: prefix and the legacy $color: prefix. + * A tag written under both spellings is one tag, so it is returned once. */ export function getEmailTagIds(keywords: Record | undefined): string[] { if (!keywords) return []; - const tags: string[] = []; + const tags = new Set(); for (const key of Object.keys(keywords)) { if ((key.startsWith(KEYWORD_PREFIX) || key.startsWith(KEYWORD_PREFIX_LEGACY)) && keywords[key] === true) { - tags.push( + tags.add( key.startsWith(KEYWORD_PREFIX) ? key.slice(KEYWORD_PREFIX.length) : key.slice(KEYWORD_PREFIX_LEGACY.length) ); } } - return tags; + return [...tags]; } /**