fix: keep plugin chips named by their filter, draw the team icons at once and pass attributes to icons still loading

This commit is contained in:
Linus Rath
2026-10-28 08:16:52 +01:00
parent c00c7080a1
commit 23f67a5e72
8 changed files with 157 additions and 25 deletions
@@ -0,0 +1,84 @@
import { act, fireEvent, render, screen } from '@testing-library/react';
import { beforeEach, describe, expect, it, vi } from 'vitest';
import React from 'react';
import { EmailList } from '../email-list';
import { useEmailStore } from '@/stores/email-store';
import { resetListFilters, setPluginListFilters } from '@/lib/plugin-sandbox/list-filters';
import { confirmGroups, resetConfirmedGroups } from '@/lib/plugin-sandbox/account-kind';
import type { Mailbox } from '@/lib/jmap/types';
// The plugins' list filter chips in the message list: shown in a team
// folder's own list, loading the list again when one is turned on, and not
// in a search, a tag view or the merged views.
const client = vi.hoisted(() => ({
getAccountId: () => 'ci',
getAccountInfo: (id: string) => (id === 'ch' ? { name: 'org-team@example.org', isPersonal: false, isReadOnly: false, accountCapabilities: {} } : null),
getSharedAccounts: () => [
{ id: 'ci', name: 'org-alice@example.org', isPrimary: true, capabilities: { mail: true } },
{ id: 'ch', name: 'org-team@example.org', isPrimary: false, capabilities: { mail: true } },
],
getPrincipalTypes: async () => ({ ch: 'group' }),
}));
vi.mock('@/hooks/use-email-drag', () => ({
useEmailDrag: () => ({ dragHandlers: {}, isDragging: false }),
}));
vi.mock('@/components/ui/avatar', () => ({ Avatar: () => null }));
vi.mock('@/stores/auth-store', async () => {
const { create } = await import('zustand');
return {
useAuthStore: create(() => ({
client, identities: [], username: null, activeAccountId: 'acct-1',
getClientForAccount: () => client, getAllConnectedClients: () => new Map(),
})),
};
});
vi.mock('@/stores/account-store', () => {
const state = { getAccountById: () => undefined };
return {
useAccountStore: Object.assign(
(selector?: (s: typeof state) => unknown) => (selector ? selector(state) : state),
{ getState: () => state },
),
};
});
const teamInbox = { id: 'ch:a', originalId: 'a', name: 'Inbox', role: 'inbox', isShared: true, accountId: 'ch', totalEmails: 0, unreadEmails: 0, sortOrder: 0 } as unknown as Mailbox;
const initialEmailState = useEmailStore.getState();
const fetchEmails = vi.fn(async () => {});
beforeEach(async () => {
resetListFilters();
resetConfirmedGroups();
fetchEmails.mockClear();
useEmailStore.setState(initialEmailState, true);
useEmailStore.setState({ selectedMailbox: 'ch:a', mailboxes: [teamInbox], fetchEmails });
setPluginListFilters('team-inbox', 'Team inbox', [{ id: 'open', label: 'Open', notKeyword: '$done', accounts: ['group'] }]);
await confirmGroups(client as never);
});
describe('plugin list filters in the message list', () => {
it('shows the chip in a team folder and loads the list when it is turned on and off', async () => {
render(<EmailList emails={[]} />);
const chip = await screen.findByRole('button', { name: 'Open' });
expect(fetchEmails).not.toHaveBeenCalled();
act(() => { fireEvent.click(chip); });
expect(chip).toHaveAttribute('aria-pressed', 'true');
expect(fetchEmails).toHaveBeenCalledTimes(1);
expect(fetchEmails).toHaveBeenCalledWith(client);
act(() => { fireEvent.click(chip); });
expect(fetchEmails).toHaveBeenCalledTimes(2);
});
it.each([
['a search', { searchQuery: 'invoice' }],
['a tag view', { selectedKeyword: 'work' }],
['the merged views', { isUnifiedView: true }],
])('leaves %s without chips', async (_name, state) => {
useEmailStore.setState(state);
render(<EmailList emails={[]} />);
await act(async () => {});
expect(screen.queryByTestId('plugin-list-filters')).toBeNull();
});
});
@@ -37,8 +37,10 @@ describe('PluginListFilters', () => {
render(<PluginListFilters mailboxRole="inbox" accountKind="group" onChange={onChange} />);
fireEvent.click(screen.getByRole('button', { name: 'Open' }));
const active = screen.getByRole('button', { name: 'remove_filter' });
// The name stays the label; pressed says it's on, the tooltip how to remove it.
const active = screen.getByRole('button', { name: 'Open' });
expect(active).toHaveAttribute('aria-pressed', 'true');
expect(active).toHaveAttribute('title', 'remove_filter');
expect(active.querySelector('svg')).not.toBeNull();
expect(onChange).toHaveBeenCalledTimes(1);
expect(activeListFilterCondition(TEAM)).toEqual({ notKeyword: '$done' });
+4 -3
View File
@@ -19,7 +19,7 @@ const NONE = new Set<string>();
/**
* The keyword filters plugins offer for this folder (api.ui.setListFilters),
* as chips in the list header. A chip turns its filter on; an active chip
* shows a cross and turns it off again. `onChange` loads the list again
* is pressed, shows a cross and turns it off again. `onChange` loads the list again
* whenever the applied filters change, also when a plugin drops one that
* was on.
*/
@@ -65,8 +65,9 @@ export function PluginListFilters({
key={filter.key}
type="button"
aria-pressed={on}
aria-label={on ? t("remove_filter", { label: filter.label }) : filter.label}
title={filter.pluginName}
// The name stays the filter's label; aria-pressed says it is on,
// and the tooltip says a click removes it.
title={on ? t("remove_filter", { label: filter.label }) : filter.pluginName}
data-plugin-list-filter={filter.key}
onClick={() => setListFilterActive(filter.key)}
className={cn(
+2 -3
View File
@@ -6,6 +6,7 @@ import type { EmailListBadge } from "@/lib/plugin-types";
import { renderHooks } from "@/lib/plugin-hooks";
import { emailSummary } from "@/lib/plugin-projection";
import { rowBadgeGeneration, subscribeRowBadges } from "@/lib/plugin-sandbox/row-badges";
import { PLUGIN_ICON_RE } from "@/lib/plugin-sandbox/actions";
import { cn } from "@/lib/utils";
import { iconForName } from "@/components/icons";
import { TAG_GROUP_CLASS, TAG_LOZENGE_CLASS } from "./tag-badge";
@@ -41,8 +42,6 @@ function cacheKey(email: Email, listeners: number, generation: number): string {
// variable or a colour name. Anything else (url(), expressions, several
// declarations) is dropped and the badge keeps the theme colours.
const SAFE_COLOR = /^(#[0-9a-f]{3,8}|(rgb|rgba|hsl|hsla)\([\d\s.,%/+-]+\)|var\(--[a-z0-9-]+\)|[a-z]{3,30})$/i;
// A host icon name, as for plugin actions.
const ICON_NAME = /^[A-Za-z0-9:-]{1,40}$/;
// eslint-disable-next-line no-control-regex -- removing control characters is the point
const CONTROL_CHARS = /[\u0000-\u001f\u007f]/g;
@@ -71,7 +70,7 @@ export function normalizeBadges(raw: unknown): EmailListBadge[] {
const title = cleanText(candidate.title, MAX_TITLE_LENGTH);
if (title) badge.title = title;
const icon = cleanText(candidate.icon, 40);
if (icon && ICON_NAME.test(icon)) badge.icon = icon;
if (icon && PLUGIN_ICON_RE.test(icon)) badge.icon = icon;
out.push(badge);
}
return out;
@@ -0,0 +1,25 @@
import { describe, expect, it } from 'vitest';
import { render } from '@testing-library/react';
import { iconForName } from '../named';
describe('icons by name', () => {
it('draws the team inbox plugin\'s icons at once, without loading the whole set', () => {
for (const name of ['user', 'check', 'user-check', 'user-x', 'circle-check', 'circle-dashed', 'arrow-back-up', 'user-plus']) {
const Icon = iconForName(name)!;
const { container, unmount } = render(<Icon data-testid="icon" />);
// A curated icon draws its paths straight away; the lazy blank has none.
expect(container.querySelector('svg path'), name).not.toBeNull();
unmount();
}
});
it('keeps the caller\'s attributes on the blank shown while an icon loads', () => {
const Icon = iconForName('brand-mastodon')!;
const { container } = render(<Icon data-testid="lazy" data-kind="x" className="w-3" size={12} />);
const svg = container.querySelector('svg')!;
expect(svg.getAttribute('data-testid')).toBe('lazy');
expect(svg.getAttribute('data-kind')).toBe('x');
expect(svg.getAttribute('class')).toBe('w-3');
expect(svg.getAttribute('width')).toBe('12');
});
});
+21 -2
View File
@@ -12,6 +12,13 @@
import { forwardRef, useEffect, useState } from 'react';
import type { TablerIcon } from '@tabler/icons-react';
import {
IconArrowBackUp,
IconCheck,
IconCircleCheck,
IconCircleDashed,
IconUser,
IconUserCheck,
IconUserX,
IconAddressBook,
IconAperture,
IconApple,
@@ -177,6 +184,15 @@ export const TABLER_ICON_PREFIX = 'tabler:';
export const POPULAR_ICON_NAMES: readonly string[] = ["world","rss","radio","microphone-2","message-circle","message","messages","phone","video","device-computer-camera","headphones","microphone","file-text","file-spreadsheet","notebook","book","clipboard-list","checklist","square-check","layout-kanban","brand-trello","pencil","edit","code","terminal","braces","bug","database","server","cpu","server-2","device-desktop","device-laptop","device-mobile","device-tablet","wifi","cloud","cloud-download","cloud-upload","users","user-plus","user-circle","address-book","man","heart","thumb-up","star","award","trophy","crown","photo","camera","movie","music","player-play","device-tv","brand-youtube","palette","brush","map","map-pin","navigation","compass","home","building","building-bank","building-store","building-warehouse","currency-dollar","currency-euro","credit-card","wallet","receipt","pig-money","trending-up","chart-bar","chart-column","chart-line","chart-pie","shield","shield-check","lock","lock-open","key","fingerprint","eye","flask-2","atom","dna","microscope","stethoscope","heartbeat","pill","vaccine","thermometer","sun","moon","sun-wind","snowflake","bolt","flame","christmas-tree","flower","leaf","mountain","ripple","tool","hammer","scissors","ruler","magnet","package","gift","box","archive","car","bike","bus","train","plane","ship","rocket","coffee","glass","beer","pizza","apple","cake","soup","device-gamepad-2","dice-5","puzzle","sparkles","wand","robot","brain","bulb","bookmark","flag","bell","clock","stopwatch","link","external-link","qrcode","scan","layout-grid","stack","aperture","circle-dot","target","crosshair"];
const CURATED: Record<string, TablerIcon> = {
// Plugin badges, actions and list filters (team inbox), so a list of rows
// never waits for the whole set.
'arrow-back-up': IconArrowBackUp,
'check': IconCheck,
'circle-check': IconCircleCheck,
'circle-dashed': IconCircleDashed,
'user': IconUser,
'user-check': IconUserCheck,
'user-x': IconUserX,
'address-book': IconAddressBook,
'aperture': IconAperture,
'apple': IconApple,
@@ -403,8 +419,11 @@ function lazyIcon(name: string): TablerIcon {
};
}, [Resolved]);
if (!Resolved) {
const size = props.size ?? 24;
return <svg ref={ref} width={size} height={size} className={props.className} aria-hidden="true" />;
// A blank of the same size, with the caller's attributes (test ids,
// aria, data-*), until the set has loaded.
const { size = 24, stroke: _stroke, title: _title, ...rest } = props;
void _stroke; void _title;
return <svg ref={ref} aria-hidden="true" {...rest} width={size} height={size} />;
}
return <Resolved ref={ref} {...props} />;
});
@@ -5,6 +5,7 @@ import { useAuthStore } from '@/stores/auth-store';
import { useEmailStore } from '@/stores/email-store';
import { usePolicyStore } from '@/stores/policy-store';
import type { IJMAPClient } from '@/lib/jmap/client-interface';
import { resetConfirmedGroups } from '@/lib/plugin-sandbox/account-kind';
import type { Email, Mailbox } from '@/lib/jmap/types';
import { resetActions, setActionDispatcher, setPluginActions } from '@/lib/plugin-sandbox/actions';
@@ -109,21 +110,26 @@ describe('actions for team mailboxes', () => {
expect(container).toBeEmptyDOMElement();
});
it("works out a message's account from the open folder and follows the group list as it loads", () => {
it("works out a message's account from the open folder and follows the server's word on groups", async () => {
let answer!: (types: Record<string, string>) => void;
const client = {
getAccountId: () => 'ci',
getAccountInfo: (id: string) => (id === 'ch' ? { name: 'org-team@example.org', isPersonal: false, isReadOnly: false, accountCapabilities: {} } : null),
getSharedAccounts: () => [{ id: 'ci', isPrimary: true }, { id: 'ch', isPrimary: false }],
getPrincipalTypes: () => new Promise((resolve) => { answer = resolve; }),
} as unknown as IJMAPClient;
resetConfirmedGroups();
const team = { id: 'ch:a', originalId: 'a', role: 'inbox', isShared: true, accountId: 'ch' } as unknown as Mailbox;
useAuthStore.setState({ client });
useEmailStore.setState({ mailboxes: [team], selectedMailbox: 'ch:a', viewingAccountId: null });
usePolicyStore.setState({ subject: { email: 'org-alice@example.org', groups: [] } });
// The policy subject counting it a group is not enough.
usePolicyStore.setState({ subject: { email: 'org-alice@example.org', groups: ['org-team@example.org'] } });
const email = { id: 'm1', keywords: { $seen: true, $done: true } } as unknown as Email;
const { result } = renderHook(() => useMessageActionTarget(email));
expect(result.current).toEqual({ accountId: 'ch', accountKind: 'shared', keywords: ['$done', '$seen'] });
act(() => usePolicyStore.setState({ subject: { email: 'org-alice@example.org', groups: ['org-team@example.org'] } }));
await act(async () => { answer({ ch: 'group' }); });
expect(result.current.accountKind).toBe('group');
act(() => useEmailStore.setState({ selectedMailbox: 'inbox', mailboxes: [team, { id: 'inbox', role: 'inbox' } as Mailbox] }));
+9 -13
View File
@@ -15,12 +15,10 @@ import {
type ActionFilter,
type RegisteredAction,
} from '@/lib/plugin-sandbox/actions';
import { mailAccountKind, type MailAccountKind } from '@/lib/plugin-sandbox/account-kind';
import { mailAccountKind, useConfirmedGroups, type MailAccountKind } from '@/lib/plugin-sandbox/account-kind';
import type { Email } from '@/lib/jmap/types';
import { useAuthStore } from '@/stores/auth-store';
import { resolveEmailBlobContext, useEmailStore } from '@/stores/email-store';
import { usePolicyStore } from '@/stores/policy-store';
import { useAccountSubjects } from '@/lib/org/account-subjects';
const EMPTY: RegisteredAction[] = [];
@@ -43,12 +41,11 @@ type ActionEmail = Pick<Email, 'id' | 'keywords' | 'sourceAccountId' | 'sourceCl
/**
* The account and keywords of `email`, for the actions' `accounts`,
* `hasKeyword` and `notKeyword` and the click's `accountId`. Follows the
* group lists as they load, so a team mailbox's actions appear once the
* server said which accounts are groups.
* server's answer on which accounts are groups, so a team mailbox's
* actions appear once it is in.
*/
export function useMessageActionTarget(email: ActionEmail | null | undefined): MessageActionTarget {
const subject = usePolicyStore((s) => s.subject);
const subjects = useAccountSubjects((s) => s.subjects);
const groups = useConfirmedGroups((s) => s.revision);
const client = useAuthStore((s) => s.client);
const selectedMailbox = useEmailStore((s) => s.selectedMailbox);
const mailboxes = useEmailStore((s) => s.mailboxes);
@@ -64,28 +61,27 @@ export function useMessageActionTarget(email: ActionEmail | null | undefined): M
const accountId = owner.accountId && owner.accountId !== own ? owner.accountId : undefined;
return {
...(accountId ? { accountId } : {}),
accountKind: owner.client ? mailAccountKind(owner.client, owner.accountId, owner.clientAccountId) : null,
accountKind: owner.client ? mailAccountKind(owner.client, owner.accountId) : null,
keywords: keywordKey ? keywordKey.split(' ') : [],
};
// The group lists, the open folder and its mailboxes decide the account
// (resolveEmailBlobContext reads them from the stores).
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [hasEmail, keywordKey, sourceAccountId, sourceClientAccountId, client, subject, subjects, selectedMailbox, mailboxes]);
}, [hasEmail, keywordKey, sourceAccountId, sourceClientAccountId, client, groups, selectedMailbox, mailboxes]);
}
/** The account kind of the open folder, for surfaces about the selection (message-bulk-actions). */
export function useFolderAccountKind(): MailAccountKind | null {
const subject = usePolicyStore((s) => s.subject);
const subjects = useAccountSubjects((s) => s.subjects);
const groups = useConfirmedGroups((s) => s.revision);
const client = useAuthStore((s) => s.client);
const selectedMailbox = useEmailStore((s) => s.selectedMailbox);
const mailboxes = useEmailStore((s) => s.mailboxes);
return useMemo(() => {
const owner = resolveEmailBlobContext({}, client);
return owner.client ? mailAccountKind(owner.client, owner.accountId, owner.clientAccountId) : null;
return owner.client ? mailAccountKind(owner.client, owner.accountId) : null;
// See useMessageActionTarget.
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [client, subject, subjects, selectedMailbox, mailboxes]);
}, [client, groups, selectedMailbox, mailboxes]);
}
/** The context a message action's click carries: the surface's, plus the message's account. */