From 07e6838d908b205e6e01002cd05fda98adf562fd Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Thu, 15 Oct 2026 23:44:06 +0200 Subject: [PATCH] fix: stop the mail list skeleton flickering after a reload or account switch --- .../__tests__/email-list-skeleton.test.tsx | 46 +++++++++++++++++++ components/email/email-list.tsx | 45 ++++++++++-------- .../__tests__/auth-store-switch-race.test.ts | 27 +++++++++++ .../email-store-initial-loading.test.ts | 32 +++++++++++++ stores/auth-store.ts | 4 +- stores/email-store.ts | 5 +- 6 files changed, 137 insertions(+), 22 deletions(-) create mode 100644 components/email/__tests__/email-list-skeleton.test.tsx create mode 100644 stores/__tests__/email-store-initial-loading.test.ts diff --git a/components/email/__tests__/email-list-skeleton.test.tsx b/components/email/__tests__/email-list-skeleton.test.tsx new file mode 100644 index 000000000..0f6221deb --- /dev/null +++ b/components/email/__tests__/email-list-skeleton.test.tsx @@ -0,0 +1,46 @@ +import { act, 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'; + +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: null, identities: [], username: null, activeAccountId: 'acct-1' })), + }; +}); +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 initialEmailState = useEmailStore.getState(); + +beforeEach(() => { + useEmailStore.setState(initialEmailState, true); + useEmailStore.setState({ selectedMailbox: 'inbox' }); +}); + +describe('email list loading skeleton', () => { + // An account switch updates the store many times before the first page + // arrives. A remounted skeleton replays its fade-in, so it flickered. + it('stays the same element while the list re-renders', () => { + const view = render(); + const skeleton = screen.getByTestId('email-list-skeleton'); + + act(() => { useEmailStore.setState({ mailboxes: [], isLoadingMore: false, hasMoreEmails: true }); }); + view.rerender(); + + expect(screen.getByTestId('email-list-skeleton')).toBe(skeleton); + }); +}); diff --git a/components/email/email-list.tsx b/components/email/email-list.tsx index e31e97500..3a486bfbd 100644 --- a/components/email/email-list.tsx +++ b/components/email/email-list.tsx @@ -82,6 +82,31 @@ interface EmailListProps { /** Below this width the batch toolbar drops its labels (see batchNarrow). */ const BATCH_TOOLBAR_WIDE = 480; +// Declared out here so re-renders of the list keep the same element. Inside +// EmailList every render made a new component, which remounted the skeleton +// and replayed its fade-in each time the store changed while a list loaded. +function LoadingSkeleton() { + return ( +
+ {[...Array(8)].map((_, i) => ( +
+
+
+
+
+
+
+
+
+
+
+
+
+ ))} +
+ ); +} + export function EmailList({ emails, selectedEmailId, @@ -416,26 +441,6 @@ export function EmailList({ virtualizer.scrollToOffset(0); }, [viewKey, virtualizer]); - const LoadingSkeleton = () => ( -
- {[...Array(8)].map((_, i) => ( -
-
-
-
-
-
-
-
-
-
-
-
-
- ))} -
- ); - const hasSelection = selectedEmailKeys.size > 0; // What plugin buttons in the selection toolbar act on: the JMAP ids of the // selected messages, thread members included. diff --git a/stores/__tests__/auth-store-switch-race.test.ts b/stores/__tests__/auth-store-switch-race.test.ts index 1ad01d8a6..3b186e477 100644 --- a/stores/__tests__/auth-store-switch-race.test.ts +++ b/stores/__tests__/auth-store-switch-race.test.ts @@ -43,6 +43,7 @@ import { useAuthStore } from '@/stores/auth-store'; import { useAccountStore } from '@/stores/account-store'; import { useIdentityStore } from '@/stores/identity-store'; import { useEmailStore } from '@/stores/email-store'; +import { useSettingsStore } from '@/stores/settings-store'; import { evictAccount } from '@/lib/account-state-manager'; const acct = (username: string, host: string, slot: number) => ({ @@ -89,3 +90,29 @@ it("overlapping switches keep every account's data under its own key", async () expect(st.primaryIdentity?.email).toBe('a@x.test'); expect(useEmailStore.getState().emails.map((e) => e.subject)).toEqual(['mail of A']); }); + +it("keeps the outgoing account's mail on screen while its settings are saved", async () => { + useAccountStore.setState({ accounts: [A, B] as never, activeAccountId: A.id, defaultAccountId: A.id }); + useAuthStore.setState({ isAuthenticated: true, activeAccountId: A.id, client: null }); + const sw = (id: string) => useAuthStore.getState().switchAccount(id); + // Connect both, so the next switch takes the seamless path. + await sw(B.id); await sw(A.id); + useEmailStore.setState({ emails: [{ id: 'mA', subject: 'mail of A' }] as never }); + + // The flush is a request; whatever the store holds meanwhile is on screen. + const flushSync = useSettingsStore.getState().flushSync; + let onScreen: string[] = []; + useSettingsStore.setState({ + flushSync: async () => { + await sleep(0); + onScreen = useEmailStore.getState().emails.map((e) => e.subject ?? ''); + }, + }); + try { + await sw(B.id); + } finally { + useSettingsStore.setState({ flushSync }); + } + expect(onScreen).toEqual(['mail of A']); + expect(useAuthStore.getState().activeAccountId).toBe(B.id); +}); diff --git a/stores/__tests__/email-store-initial-loading.test.ts b/stores/__tests__/email-store-initial-loading.test.ts new file mode 100644 index 000000000..fc6f89704 --- /dev/null +++ b/stores/__tests__/email-store-initial-loading.test.ts @@ -0,0 +1,32 @@ +import { beforeEach, describe, expect, it } from 'vitest'; +import { useEmailStore } from '@/stores/email-store'; + +// On a first load the folders arrive before the first page of mail. If the +// folder fetch turned isLoading off, the list showed "No emails" until the +// page fetch turned it on again, then a fresh skeleton. + +const initialState = useEmailStore.getState(); + +const clientWith = (mailboxes: unknown[]) => ({ + getAllMailboxesWithState: async () => ({ mailboxes, states: {} }), +}); + +beforeEach(() => { + useEmailStore.setState(initialState, true); +}); + +describe('fetchMailboxes on a first load', () => { + it('leaves the list loading for the page fetch that follows', async () => { + const inbox = { id: 'inbox', name: 'Inbox', role: 'inbox', totalEmails: 3, unreadEmails: 0, sortOrder: 0 }; + await useEmailStore.getState().fetchMailboxes(clientWith([inbox]) as never); + + expect(useEmailStore.getState().selectedMailbox).toBe('inbox'); + expect(useEmailStore.getState().isLoading).toBe(true); + }); + + it('stops loading when the server has no folders', async () => { + await useEmailStore.getState().fetchMailboxes(clientWith([]) as never); + + expect(useEmailStore.getState().isLoading).toBe(false); + }); +}); diff --git a/stores/auth-store.ts b/stores/auth-store.ts index 38d47723f..e3a78b048 100644 --- a/stores/auth-store.ts +++ b/stores/auth-store.ts @@ -2499,11 +2499,13 @@ export const useAuthStore = create()( // clearing it), so snapshot + clear it now - synchronously, right before // the restore and client swap below - so the UI never blanks between the // two accounts. The network path already snapshotted and cleared above. + // The settings flush is a request, so it goes first: awaited after the + // clear, the list showed "No emails" until it came back. if (wasConnected) { - snapshotAndClearForSwitch(get().activeAccountId); await useSettingsStore.getState().flushSync(); useSettingsStore.getState().disableSync(); if (superseded()) return; + snapshotAndClearForSwitch(get().activeAccountId); } // Restore cached state or fetch fresh diff --git a/stores/email-store.ts b/stores/email-store.ts index 1f2f03c6d..aecbea409 100644 --- a/stores/email-store.ts +++ b/stores/email-store.ts @@ -2449,7 +2449,10 @@ export const useEmailStore = create((set, get) => ({ || isUnifiedMailboxId(currentSelectedMailbox) || isCrossViewId(currentSelectedMailbox) || (currentSelectedMailbox && mailboxes.some(m => m.id === currentSelectedMailbox)); - const loadingPatch = isInitialLoad + // A first load with folders leaves isLoading on: the first page is + // fetched next and turns it off. Dropping it here showed "No emails" + // between the two, and a new skeleton after it. + const loadingPatch = isInitialLoad && mailboxes.length === 0 ? { isLoading: false, mailboxSyncStates: fetched.states } : { mailboxSyncStates: fetched.states }; if (!selectionValid) {