mirror of
https://github.com/bulwarkmail/webmail.git
synced 2026-11-03 00:13:09 +00:00
perf: keep the list on screen while a search runs and stop re-rendering on every keystroke
This commit is contained in:
@@ -121,6 +121,7 @@ import { PLUGIN_REPLY_EVENT, type PluginReplyRequest } from "@/lib/plugin-sandbo
|
||||
import { mailtoComposerDraft } from "./mailto-draft";
|
||||
import { getQuoteBodies } from "@/lib/email-composer-utils";
|
||||
import { emailHooks } from "@/lib/plugin-hooks";
|
||||
import { afterRequestDispatch } from "@/lib/after-request-dispatch";
|
||||
import { createMailboxWithHooks } from "@/lib/plugin-mail-hooks";
|
||||
import { emailToReadView } from "@/lib/plugin-projection";
|
||||
import { emitThreadExpand } from "@/lib/plugin-thread-expand";
|
||||
@@ -3354,9 +3355,13 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
|
||||
|
||||
const handleSearch = async (query: string) => {
|
||||
if (!client) return;
|
||||
setSearchQuery(query);
|
||||
useSearchHistoryStore.getState().addRecentSearch(query);
|
||||
// Nothing the mail view renders from is written before the request has
|
||||
// left (see afterRequestDispatch): searchEmails records the query itself,
|
||||
// and the history entry only feeds the suggestion dropdown.
|
||||
afterRequestDispatch(() => useSearchHistoryStore.getState().addRecentSearch(query));
|
||||
if (!isFilterEmpty(searchFilters)) {
|
||||
// advancedSearch reads the query from the store.
|
||||
setSearchQuery(query);
|
||||
await advancedSearch(client);
|
||||
} else {
|
||||
await searchEmails(client, query);
|
||||
|
||||
@@ -202,3 +202,65 @@ describe('SearchBox and password managers', () => {
|
||||
expect(input()).toHaveAttribute('type', 'search');
|
||||
});
|
||||
});
|
||||
|
||||
describe('SearchBox draft', () => {
|
||||
beforeEach(() => {
|
||||
useSearchHistoryStore.setState({ recentSearches: [] });
|
||||
});
|
||||
|
||||
it('keeps the typed text to itself until it is submitted', () => {
|
||||
const onChange = vi.fn();
|
||||
const onSubmit = vi.fn();
|
||||
render(<Harness onChange={onChange} onSubmit={onSubmit} />);
|
||||
fireEvent.change(input(), { target: { value: 'i' } });
|
||||
fireEvent.change(input(), { target: { value: 'in' } });
|
||||
fireEvent.change(input(), { target: { value: 'invoice' } });
|
||||
|
||||
expect(onChange).not.toHaveBeenCalled();
|
||||
expect(input().value).toBe('invoice');
|
||||
|
||||
fireEvent.submit(input().closest('form')!);
|
||||
expect(onSubmit).toHaveBeenCalledWith('invoice');
|
||||
});
|
||||
|
||||
it('hands the typed text over when focus leaves the box', () => {
|
||||
const onChange = vi.fn();
|
||||
render(<Harness onChange={onChange} />);
|
||||
fireEvent.change(input(), { target: { value: 'invoice' } });
|
||||
fireEvent.blur(input());
|
||||
expect(onChange).toHaveBeenCalledWith('invoice');
|
||||
});
|
||||
|
||||
it('leaves the search on Enter in an emptied box, like the clear button', () => {
|
||||
const onClear = vi.fn();
|
||||
const onSubmit = vi.fn();
|
||||
render(<Harness initial="invoice" onClear={onClear} onSubmit={onSubmit} />);
|
||||
fireEvent.change(input(), { target: { value: '' } });
|
||||
fireEvent.submit(input().closest('form')!);
|
||||
|
||||
expect(onClear).toHaveBeenCalledTimes(1);
|
||||
expect(onSubmit).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('does nothing on Enter in an empty box when no search is in force', () => {
|
||||
const onClear = vi.fn();
|
||||
render(<Harness onClear={onClear} />);
|
||||
fireEvent.submit(input().closest('form')!);
|
||||
expect(onClear).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('shows a query set from outside', () => {
|
||||
function Outside() {
|
||||
const [value, setValue] = React.useState('');
|
||||
return (
|
||||
<>
|
||||
<button onClick={() => setValue('from:alice')}>set</button>
|
||||
<SearchBox value={value} onChange={setValue} onSubmit={() => {}} onClear={() => {}} onSelectContact={() => {}} />
|
||||
</>
|
||||
);
|
||||
}
|
||||
render(<Outside />);
|
||||
fireEvent.click(screen.getByText('set'));
|
||||
expect(input().value).toBe('from:alice');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -42,6 +42,18 @@ export function SearchBox({ value, onChange, onSubmit, onClear, onSelectContact,
|
||||
const containerRef = useRef<HTMLDivElement>(null);
|
||||
const [open, setOpen] = useState(false);
|
||||
const [selectedIndex, setSelectedIndex] = useState(-1);
|
||||
// The text being typed belongs to the box until it is submitted, cleared or
|
||||
// the box loses focus. `value` is the search in force; handing it every
|
||||
// keystroke re-rendered the whole mail view per character typed.
|
||||
const [draft, setDraft] = useState(value);
|
||||
const [adopted, setAdopted] = useState(value);
|
||||
if (value !== adopted) {
|
||||
setAdopted(value);
|
||||
setDraft(value);
|
||||
}
|
||||
const commitDraft = () => {
|
||||
if (draft !== value) onChange(draft);
|
||||
};
|
||||
|
||||
const getAutocomplete = useContactStore((s) => s.getAutocomplete);
|
||||
// Subscribed only so the memo below recomputes as the lookup's inputs load.
|
||||
@@ -52,13 +64,13 @@ export function SearchBox({ value, onChange, onSubmit, onClear, onSelectContact,
|
||||
const removeRecentSearch = useSearchHistoryStore((s) => s.removeRecentSearch);
|
||||
|
||||
const suggestions = useMemo<SearchSuggestion[]>(() => {
|
||||
const query = value.trim();
|
||||
const query = draft.trim();
|
||||
const matches = query ? getAutocomplete(query).filter((r) => !r.group && r.email) : [];
|
||||
return buildSearchSuggestions(value, recentSearches, matches);
|
||||
return buildSearchSuggestions(draft, recentSearches, matches);
|
||||
// The three store slices are inputs of getAutocomplete (read via get()),
|
||||
// not of this closure — list them so the memo recomputes when they load.
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, [value, recentSearches, getAutocomplete, contacts, recentRecipients, directoryPrincipals]);
|
||||
}, [draft, recentSearches, getAutocomplete, contacts, recentRecipients, directoryPrincipals]);
|
||||
|
||||
const showDropdown = open && !disabled && suggestions.length > 0;
|
||||
const activeIndex = showDropdown && selectedIndex < suggestions.length ? selectedIndex : -1;
|
||||
@@ -72,6 +84,7 @@ export function SearchBox({ value, onChange, onSubmit, onClear, onSelectContact,
|
||||
(suggestion: SearchSuggestion) => {
|
||||
close();
|
||||
if (suggestion.kind === "recent") {
|
||||
setDraft(suggestion.query);
|
||||
onChange(suggestion.query);
|
||||
onSubmit(suggestion.query);
|
||||
} else {
|
||||
@@ -112,9 +125,13 @@ export function SearchBox({ value, onChange, onSubmit, onClear, onSelectContact,
|
||||
<form
|
||||
onSubmit={(e) => {
|
||||
e.preventDefault();
|
||||
if (value.trim()) {
|
||||
if (draft.trim()) {
|
||||
close();
|
||||
onSubmit(value);
|
||||
onSubmit(draft);
|
||||
} else if (value) {
|
||||
// Enter on an emptied box leaves the search, as the clear button does.
|
||||
close();
|
||||
onClear();
|
||||
}
|
||||
}}
|
||||
className="relative"
|
||||
@@ -127,9 +144,9 @@ export function SearchBox({ value, onChange, onSubmit, onClear, onSelectContact,
|
||||
type="search"
|
||||
autoFocus={autoFocus}
|
||||
placeholder={placeholder ?? t("search_placeholder_hint")}
|
||||
value={value}
|
||||
value={draft}
|
||||
onChange={(e) => {
|
||||
onChange(e.target.value);
|
||||
setDraft(e.target.value);
|
||||
setOpen(true);
|
||||
setSelectedIndex(-1);
|
||||
}}
|
||||
@@ -139,10 +156,13 @@ export function SearchBox({ value, onChange, onSubmit, onClear, onSelectContact,
|
||||
// Focus moving into the dropdown (e.g. its action buttons) keeps it open.
|
||||
const next = e.relatedTarget as Node | null;
|
||||
if (next && containerRef.current?.contains(next)) return;
|
||||
// Anything that reads the query from outside (the filter panel's
|
||||
// search) sees what was typed once focus moves to it.
|
||||
commitDraft();
|
||||
close();
|
||||
}}
|
||||
onKeyDown={handleKeyDown}
|
||||
className={cn("ps-9 h-9 [&::-webkit-search-cancel-button]:appearance-none [&::-webkit-search-decoration]:appearance-none", value && "pe-8")}
|
||||
className={cn("ps-9 h-9 [&::-webkit-search-cancel-button]:appearance-none [&::-webkit-search-decoration]:appearance-none", draft && "pe-8")}
|
||||
role="combobox"
|
||||
aria-expanded={showDropdown}
|
||||
aria-controls={listId}
|
||||
@@ -154,10 +174,11 @@ export function SearchBox({ value, onChange, onSubmit, onClear, onSelectContact,
|
||||
disabled={disabled}
|
||||
title={title}
|
||||
/>
|
||||
{value && (
|
||||
{draft && (
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => {
|
||||
setDraft("");
|
||||
close();
|
||||
onClear();
|
||||
}}
|
||||
@@ -171,7 +192,7 @@ export function SearchBox({ value, onChange, onSubmit, onClear, onSelectContact,
|
||||
{showDropdown && (
|
||||
<SearchSuggestions
|
||||
id={listId}
|
||||
query={value}
|
||||
query={draft}
|
||||
suggestions={suggestions}
|
||||
selectedIndex={activeIndex}
|
||||
onSelect={select}
|
||||
|
||||
@@ -0,0 +1,18 @@
|
||||
/**
|
||||
* Runs `fn` in a task of its own, once the current one has ended.
|
||||
*
|
||||
* Chromium hands a fetch() to the network only when the task that called it
|
||||
* yields, so a request issued from a click or key handler otherwise leaves
|
||||
* the browser after the handler's whole synchronous render pass: tens of
|
||||
* milliseconds for a tree this size. Issuing the request first and writing
|
||||
* the state that only paints a loading indicator from here lets the round
|
||||
* trip overlap that rendering instead of following it. Outside a browser it
|
||||
* runs `fn` at once.
|
||||
*/
|
||||
export function afterRequestDispatch(fn: () => void): void {
|
||||
if (typeof window === 'undefined') {
|
||||
fn();
|
||||
return;
|
||||
}
|
||||
window.setTimeout(fn, 0);
|
||||
}
|
||||
@@ -0,0 +1,73 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import { useEmailStore } from '../email-store';
|
||||
import { useAuthStore } from '../auth-store';
|
||||
import { useSettingsStore } from '@/stores/settings-store';
|
||||
import { DEFAULT_SEARCH_FILTERS } from '@/lib/jmap/search-utils';
|
||||
import type { Email, Mailbox } from '@/lib/jmap/types';
|
||||
import type { IJMAPClient, SearchEmailsResult } from '@/lib/jmap/client-interface';
|
||||
|
||||
// Starting a search keeps the rows on screen (dimmed by the loading state)
|
||||
// instead of blanking the list, and writes that loading state only after the
|
||||
// request has left, so the click's render pass does not hold the request back.
|
||||
|
||||
const row = (id: string): Email => ({
|
||||
id, threadId: `t-${id}`, mailboxIds: { inbox: true }, keywords: {}, subject: id,
|
||||
receivedAt: '2026-10-01T00:00:00Z', from: [], to: [], preview: '', size: 1, hasAttachment: false,
|
||||
} as Email);
|
||||
|
||||
const nextTask = () => new Promise((resolve) => setTimeout(resolve, 0));
|
||||
|
||||
describe('starting a search', () => {
|
||||
let answer: (result: SearchEmailsResult) => void;
|
||||
let client: IJMAPClient;
|
||||
const folderRows = [row('old-1'), row('old-2')];
|
||||
|
||||
beforeEach(() => {
|
||||
client = {
|
||||
searchEmails: vi.fn(() => new Promise<SearchEmailsResult>((resolve) => { answer = resolve; })),
|
||||
advancedSearchEmails: vi.fn(),
|
||||
getSomeEmails: vi.fn().mockResolvedValue([]),
|
||||
getAccountId: () => 'me',
|
||||
} as unknown as IJMAPClient;
|
||||
useAuthStore.setState({
|
||||
activeAccountId: 'account-a',
|
||||
getClientForAccount: (id: string) => (id === 'account-a' ? client : undefined) as never,
|
||||
} as never);
|
||||
useSettingsStore.setState({ emailsPerPage: 50 } as never);
|
||||
useEmailStore.setState({
|
||||
isUnifiedView: false, unifiedRole: null, crossView: null, viewingAccountId: null, selectedKeyword: null,
|
||||
selectedMailbox: 'inbox', searchMailboxId: 'inbox',
|
||||
searchQuery: '', searchFilters: { ...DEFAULT_SEARCH_FILTERS }, searchAbortController: null,
|
||||
emails: folderRows, isLoading: false,
|
||||
mailboxes: [{ id: 'inbox', name: 'Inbox', role: 'inbox', totalEmails: 2, unreadEmails: 0 } as Mailbox],
|
||||
accountMailboxes: {},
|
||||
});
|
||||
});
|
||||
|
||||
it('sends the request before anything the list renders from changes', async () => {
|
||||
const done = useEmailStore.getState().searchEmails(client, 'invoice');
|
||||
|
||||
expect(client.searchEmails).toHaveBeenCalledTimes(1);
|
||||
expect(useEmailStore.getState()).toMatchObject({ emails: folderRows, isLoading: false, searchQuery: '' });
|
||||
|
||||
await nextTask();
|
||||
// Loading now, over the old rows rather than an empty list.
|
||||
expect(useEmailStore.getState()).toMatchObject({ emails: folderRows, isLoading: true, searchQuery: 'invoice' });
|
||||
|
||||
answer({ emails: [row('hit')], hasMore: false, total: 1 });
|
||||
await done;
|
||||
expect(useEmailStore.getState().emails.map((e) => e.id)).toEqual(['hit']);
|
||||
expect(useEmailStore.getState()).toMatchObject({ isLoading: false, searchQuery: 'invoice' });
|
||||
});
|
||||
|
||||
it('records the query when the answer lands before the deferred loading write', async () => {
|
||||
const done = useEmailStore.getState().searchEmails(client, 'invoice');
|
||||
answer({ emails: [row('hit')], hasMore: false, total: 1 });
|
||||
await done;
|
||||
|
||||
expect(useEmailStore.getState()).toMatchObject({ searchQuery: 'invoice', isLoading: false });
|
||||
await nextTask();
|
||||
// The late loading write sees a finished search and stays out.
|
||||
expect(useEmailStore.getState().isLoading).toBe(false);
|
||||
});
|
||||
});
|
||||
+17
-16
@@ -3,6 +3,7 @@ import { Email, Mailbox, StateChange, ScheduledEmail, SendEmailResult, isUnified
|
||||
import type { UnifiedMailboxRole, CrossView } from "@/lib/jmap/types";
|
||||
import type { IJMAPClient, SearchEmailsResult } from "@/lib/jmap/client-interface";
|
||||
import { withSearchSnippets, type DeferredSnippetBatch } from "@/lib/search-snippet";
|
||||
import { afterRequestDispatch } from "@/lib/after-request-dispatch";
|
||||
import { useSettingsStore, getMessageListOrderFor } from "@/stores/settings-store";
|
||||
import { useCalendarStore } from "@/stores/calendar-store";
|
||||
import { orderKeywords, placeHeldRows, type HeldKeywords, type SortLevel } from "@/lib/message-list-order";
|
||||
@@ -3647,15 +3648,15 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
|
||||
}
|
||||
|
||||
const controller = new AbortController();
|
||||
set({
|
||||
isLoading: true,
|
||||
error: null,
|
||||
searchQuery: typedQuery,
|
||||
emails: [],
|
||||
hasMoreEmails: false,
|
||||
totalEmails: 0,
|
||||
searchAbortController: controller,
|
||||
}); // Clear emails for loading state
|
||||
// The rows on screen stay, dimmed, until the results land, as on a folder
|
||||
// switch, instead of being torn down for a skeleton and rebuilt a round
|
||||
// trip later. What the mail view renders from is written only once the
|
||||
// request is on its way (see afterRequestDispatch); the controller is
|
||||
// recorded at once so a superseding search can still cancel this one.
|
||||
set({ searchAbortController: controller });
|
||||
afterRequestDispatch(() => {
|
||||
if (get().searchAbortController === controller) set({ isLoading: true, error: null, searchQuery: typedQuery });
|
||||
});
|
||||
try {
|
||||
// The store keeps what the user typed; plugins' changes apply to this run.
|
||||
// Without listeners the request goes out in this same tick, as before.
|
||||
@@ -3746,6 +3747,8 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
|
||||
externalSearchResults: externals,
|
||||
hasMoreEmails: result.hasMore,
|
||||
totalEmails: result.total,
|
||||
// Also here: a fast answer can land before the deferred loading write.
|
||||
searchQuery: typedQuery,
|
||||
isLoading: false,
|
||||
searchAbortController: null,
|
||||
...(unifiedErrors ? { unifiedErrors } : {})
|
||||
@@ -3755,6 +3758,7 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
|
||||
if (controller.signal.aborted) return;
|
||||
set({
|
||||
error: error instanceof Error ? error.message : "Failed to search emails",
|
||||
searchQuery: typedQuery,
|
||||
isLoading: false,
|
||||
emails: [],
|
||||
externalSearchResults: [],
|
||||
@@ -3774,13 +3778,10 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
|
||||
}
|
||||
|
||||
const controller = new AbortController();
|
||||
set({
|
||||
isLoading: true,
|
||||
error: null,
|
||||
emails: [],
|
||||
hasMoreEmails: false,
|
||||
totalEmails: 0,
|
||||
searchAbortController: controller,
|
||||
// See searchEmails: the rows stay, the loading state follows the request.
|
||||
set({ searchAbortController: controller });
|
||||
afterRequestDispatch(() => {
|
||||
if (get().searchAbortController === controller) set({ isLoading: true, error: null });
|
||||
});
|
||||
|
||||
try {
|
||||
|
||||
Reference in New Issue
Block a user