fix: stop list toolbar timers from firing after the toolbar is gone

This commit is contained in:
Linus Rath
2026-10-31 02:28:55 +01:00
parent 2de0ee8e30
commit 4887ed457c
5 changed files with 98 additions and 10 deletions
@@ -1,4 +1,4 @@
import { render, screen, fireEvent, waitFor } from '@testing-library/react';
import { act, render, screen, fireEvent, waitFor } from '@testing-library/react';
import { describe, it, expect, beforeEach, vi } from 'vitest';
import type { Email, Mailbox } from '@/lib/jmap/types';
import { KEYWORD_PREFIX } from '@/lib/thread-utils';
@@ -338,4 +338,25 @@ describe('MvListToolbar removal confirmations', () => {
await waitFor(() => expect(batchArchive).toHaveBeenCalledTimes(1));
expect(screen.queryByRole('alertdialog')).toBeNull();
});
it('leaves no timer running once it is gone', async () => {
// The buttons stay disabled for a moment after an action. A bare timer
// for that outlived the toolbar and reached React after the test
// environment was torn down ("window is not defined", failing CI).
vi.useFakeTimers();
try {
useSettingsStore.setState({ confirmArchive: false } as never);
const { unmount } = render(<MvListToolbar loadedCount={2} onRefresh={() => {}} />);
await act(async () => {
fireEvent.click(screen.getByLabelText('archive'));
});
expect(batchArchive).toHaveBeenCalledTimes(1);
expect(vi.getTimerCount()).toBeGreaterThan(0);
unmount();
expect(vi.getTimerCount()).toBe(0);
} finally {
vi.useRealTimers();
}
});
});
+10 -8
View File
@@ -48,6 +48,7 @@ import { PluginSlot } from "@/components/plugins/plugin-slot";
import { PluginListFilters } from "./plugin-list-filters";
import { PluginActions, useFolderAccountKind } from "@/components/plugins/plugin-actions";
import { useUnreadFirstSection } from "@/hooks/use-unread-first-section";
import { useTimeoutWhileMounted } from "@/hooks/use-timeout-while-mounted";
import { unreadSectionRowCount } from "@/lib/unread-first";
import { MvListSectionHeader } from "./mv-list-section-header";
import { type OpenableEmail, PluginSearchResults } from "./plugin-search-results";
@@ -382,6 +383,7 @@ export function EmailList({
const [sourceEmail, setSourceEmail] = useState<Email | null>(null);
const [isProcessing, setIsProcessing] = useState(false);
const later = useTimeoutWhileMounted();
const parentRef = useRef<HTMLDivElement>(null);
const batchToolbarRef = useRef<HTMLDivElement>(null);
// The batch toolbar sits above the scroll container, so as it animates
@@ -557,7 +559,7 @@ export function EmailList({
try {
await removeFromFolder(resolveSelectedEmails(useEmailStore.getState()));
} finally {
setTimeout(() => setIsProcessing(false), 500);
later(() => setIsProcessing(false), 500);
}
};
const listSlotProps = useMemo(
@@ -584,7 +586,7 @@ export function EmailList({
try {
await markSelectionRead(read);
} finally {
setTimeout(() => setIsProcessing(false), 500);
later(() => setIsProcessing(false), 500);
}
};
@@ -631,7 +633,7 @@ export function EmailList({
} catch {
toast.error(tSpam('error_not_spam'));
} finally {
setTimeout(() => setIsProcessing(false), 500);
later(() => setIsProcessing(false), 500);
}
};
@@ -647,7 +649,7 @@ export function EmailList({
} catch {
toast.error(tSpam('error'));
} finally {
setTimeout(() => setIsProcessing(false), 500);
later(() => setIsProcessing(false), 500);
}
};
@@ -667,7 +669,7 @@ export function EmailList({
undo: undoMoveAction(client, restore, undo),
});
} finally {
setTimeout(() => setIsProcessing(false), 500);
later(() => setIsProcessing(false), 500);
}
};
@@ -686,7 +688,7 @@ export function EmailList({
undo: undoMoveAction(client, restore, undo, receipts),
});
} finally {
setTimeout(() => setIsProcessing(false), 500);
later(() => setIsProcessing(false), 500);
}
};
@@ -724,7 +726,7 @@ export function EmailList({
undo: undoMoveAction(client, restore, undo),
});
} finally {
setTimeout(() => setIsProcessing(false), 500);
later(() => setIsProcessing(false), 500);
}
};
@@ -746,7 +748,7 @@ export function EmailList({
try {
await emptyMailbox(client, currentMailbox.id);
} finally {
setTimeout(() => setIsProcessing(false), 500);
later(() => setIsProcessing(false), 500);
}
};
+3 -1
View File
@@ -39,6 +39,7 @@ import { getEmailTagIds } from "@/lib/thread-utils";
import { cn, type MailboxNode } from "@/lib/utils";
import { PluginActions } from "@/components/plugins/plugin-actions";
import { useUnreadFirstSection } from "@/hooks/use-unread-first-section";
import { useTimeoutWhileMounted } from "@/hooks/use-timeout-while-mounted";
import { commonTagScope } from "@/hooks/use-account-tags";
interface MvListToolbarProps {
@@ -156,6 +157,7 @@ export function MvListToolbar({
})));
const [isProcessing, setIsProcessing] = useState(false);
const later = useTimeoutWhileMounted();
// The ⋮, Move to and Tag menus are the ContextMenu, as every menu (R6).
const overflowMenu = useContextMenu<null>();
const menuOpen = overflowMenu.contextMenu.isOpen;
@@ -217,7 +219,7 @@ export function MvListToolbar({
}
}
} finally {
setTimeout(() => setIsProcessing(false), 400);
later(() => setIsProcessing(false), 400);
}
};
@@ -0,0 +1,34 @@
import { describe, it, expect, afterEach, vi } from 'vitest';
import { renderHook } from '@testing-library/react';
import { useTimeoutWhileMounted } from '@/hooks/use-timeout-while-mounted';
describe('useTimeoutWhileMounted', () => {
afterEach(() => {
vi.useRealTimers();
});
it('runs the callback after the delay while the component is mounted', () => {
vi.useFakeTimers();
const callback = vi.fn();
const { result } = renderHook(() => useTimeoutWhileMounted());
result.current(callback, 500);
vi.advanceTimersByTime(499);
expect(callback).not.toHaveBeenCalled();
vi.advanceTimersByTime(1);
expect(callback).toHaveBeenCalledTimes(1);
});
it('drops every pending callback when the component unmounts', () => {
vi.useFakeTimers();
const callback = vi.fn();
const { result, unmount } = renderHook(() => useTimeoutWhileMounted());
result.current(callback, 500);
result.current(callback, 800);
unmount();
vi.advanceTimersByTime(1000);
expect(callback).not.toHaveBeenCalled();
expect(vi.getTimerCount()).toBe(0);
});
});
+29
View File
@@ -0,0 +1,29 @@
import { useCallback, useEffect, useRef } from "react";
/**
* setTimeout whose pending timers are cleared when the component unmounts.
*
* A toolbar that kept its buttons disabled for a moment after an action
* called setState from a bare timer. When the component was gone by then,
* the call still reached React; in a test whose environment had already
* been torn down that threw "window is not defined" and failed the run.
*/
export function useTimeoutWhileMounted(): (callback: () => void, ms: number) => void {
const timers = useRef(new Set<ReturnType<typeof setTimeout>>());
useEffect(() => {
const pending = timers.current;
return () => {
pending.forEach(clearTimeout);
pending.clear();
};
}, []);
return useCallback((callback: () => void, ms: number) => {
const id = setTimeout(() => {
timers.current.delete(id);
callback();
}, ms);
timers.current.add(id);
}, []);
}