From 0307716b60a9354fe4ad8054dafcfcca8f726476 Mon Sep 17 00:00:00 2001 From: waterWang Date: Fri, 7 Aug 2026 22:11:56 +0800 Subject: [PATCH] fix: keep chart visible during refetch to avoid flicker (Closes #1411) --- app/dashboard/insight/page.tsx | 19 ++- tests/react/insight-page-flicker.test.tsx | 175 ++++++++++++++++++++++ 2 files changed, 191 insertions(+), 3 deletions(-) create mode 100644 tests/react/insight-page-flicker.test.tsx diff --git a/app/dashboard/insight/page.tsx b/app/dashboard/insight/page.tsx index 68e8089d..40f6c396 100644 --- a/app/dashboard/insight/page.tsx +++ b/app/dashboard/insight/page.tsx @@ -47,6 +47,10 @@ export default function InsightPage() { const [loading, setLoading] = useState(true); const [error, setError] = useState(null); const [reloadKey, setReloadKey] = useState(0); + // Track whether we have ever loaded data successfully. + // When refetching (e.g. period change) we keep the stale chart visible + // to avoid the flicker → skeleton → chart cycle. + const [hasData, setHasData] = useState(false); const handleRetry = useCallback(() => { setReloadKey((current) => current + 1); @@ -54,7 +58,11 @@ export default function InsightPage() { useEffect(() => { const controller = new AbortController(); - setLoading(true); + // Only show loading skeleton on the very first load. + // When refetching (period change, retry) keep the stale chart visible. + if (!hasData) { + setLoading(true); + } setError(null); runWidgetFetchWithRetry({ @@ -66,11 +74,16 @@ export default function InsightPage() { .then((res) => { if (controller.signal.aborted) return; setData(res); + setHasData(true); setLoading(false); }) .catch((err) => { if (err.name === 'AbortError') return; - setError(err); + // Keep the previous data visible on a failed refetch; only + // surface the error state when there is nothing to show. + if (!hasData) { + setError(err); + } setLoading(false); }); @@ -83,7 +96,7 @@ export default function InsightPage() { let content; - if (loading) { + if (loading && !hasData) { content = (
diff --git a/tests/react/insight-page-flicker.test.tsx b/tests/react/insight-page-flicker.test.tsx new file mode 100644 index 00000000..989b5975 --- /dev/null +++ b/tests/react/insight-page-flicker.test.tsx @@ -0,0 +1,175 @@ +/** + * Regression test: Chart must NOT flicker (skeleton → chart → skeleton → chart) + * when the period selector triggers a refetch. + * + * Before the fix, InsightPage would set loading=true on every period change, + * replacing the chart with a skeleton and causing a visible flash. + * After the fix, the old chart data stays visible while refetching. + */ +import { afterEach, beforeAll, describe, expect, it, vi } from 'vitest' +import { cleanup, screen, act, waitFor } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import InsightPage from '@/app/dashboard/insight/page' +import { renderWithProviders } from '@/tests/react/renderWithProviders' + +// ── Mocks ───────────────────────────────────────────────────────────────────── + +const MOCK_DATA = { + period: 'current_month', + spendingTotal: 3200, + savingsTotal: 1400, + billsTotal: 420, + insuranceTotal: 80, + breakdown: { spending: 3200, savings: 1400, bills: 420, insurance: 80 }, + trend: { '2026-01-01': 520, '2026-01-08': 780 }, +} + +// Shared mutable state accessible from BOTH the vi.mock factory and the tests. +const mockState = vi.hoisted(() => ({ + resolveApi: null as ((data: typeof MOCK_DATA) => void) | null, + rejectApi: null as ((err: Error) => void) | null, + apiCallCount: 0, + shouldReject: false, +})) + +vi.mock('@/lib/client/widgetFetchRetry', () => ({ + runWidgetFetchWithRetry: vi.fn(() => { + mockState.apiCallCount++ + if (mockState.shouldReject) { + return Promise.reject(new Error('Network error')) + } + // Return a deferred promise so tests can resolve/reject at their pace + return new Promise((resolve, reject) => { + mockState.resolveApi = resolve as (data: typeof MOCK_DATA) => void + mockState.rejectApi = reject + }) + }), +})) + +beforeAll(() => { + vi.stubGlobal('ResizeObserver', class { + observe() {} + unobserve() {} + disconnect() {} + }) + Object.defineProperty(window, 'matchMedia', { + writable: true, + value: vi.fn().mockImplementation((query: string) => ({ + matches: false, + media: query, + onchange: null, + addListener: vi.fn(), + removeListener: vi.fn(), + addEventListener: vi.fn(), + removeEventListener: vi.fn(), + dispatchEvent: vi.fn(), + })), + }) +}) + +afterEach(() => { + cleanup() + vi.clearAllMocks() + mockState.apiCallCount = 0 + mockState.resolveApi = null + mockState.rejectApi = null + mockState.shouldReject = false +}) + +describe('InsightPage chart flicker regression', () => { + it('shows skeleton on initial load, then chart after data arrives', async () => { + renderWithProviders() + + // Resolve the API call + await act(async () => { + mockState.resolveApi!(MOCK_DATA) + }) + + // After data arrives, stats appear + await waitFor(() => { + expect(screen.getByText('$3,200')).toBeInTheDocument() + }) + }) + + it('keeps the chart visible when period changes (no skeleton flash)', async () => { + const user = userEvent.setup() + renderWithProviders() + + // Resolve initial load + await act(async () => { + mockState.resolveApi!(MOCK_DATA) + }) + + await waitFor(() => { + expect(screen.getByText('$3,200')).toBeInTheDocument() + }) + + // Change period to "last_3_months" — this triggers a refetch + // Before the fix, the chart would disappear and show a skeleton. + const select = screen.getByRole('combobox', { name: /select period/i }) + await act(async () => { + await user.selectOptions(select, 'last_3_months') + }) + + // The old data must STILL be visible while the API call is in-flight + // (no skeleton flash). The stats should remain visible. + expect(screen.getByText('$3,200')).toBeInTheDocument() + expect(screen.getByText('$1,400')).toBeInTheDocument() + + // Resolve the second API call + const secondData = { ...MOCK_DATA, period: 'last_3_months', spendingTotal: 9000 } + await act(async () => { + mockState.resolveApi!(secondData) + }) + + // Now the new data appears + await waitFor(() => { + expect(screen.getByText('$9,000.00')).toBeInTheDocument() + }) + }) + + it('shows error state on initial load failure', async () => { + mockState.shouldReject = true + renderWithProviders() + + await waitFor(() => { + expect(screen.getByText(/failed to load insights/i)).toBeInTheDocument() + }, { timeout: 5000 }) + }) + + it('keeps previous data visible when refetch fails after initial success', async () => { + const user = userEvent.setup() + renderWithProviders() + + // Resolve initial load + await act(async () => { + mockState.resolveApi!(MOCK_DATA) + }) + + await waitFor(() => { + expect(screen.getByText('$3,200')).toBeInTheDocument() + }) + + // Change period — triggers refetch + const select = screen.getByRole('combobox', { name: /select period/i }) + await act(async () => { + await user.selectOptions(select, 'last_year') + }) + + // Still shows old data + expect(screen.getByText('$3,200')).toBeInTheDocument() + + // Reject the refetch + await act(async () => { + mockState.rejectApi!(new Error('Network error')) + }) + + // Old data stays visible; no error state shown because we already have data + await waitFor(() => { + expect(screen.getByText('$3,200')).toBeInTheDocument() + }) + + // Error state is NOT shown + expect(screen.queryByText(/failed to load insights/i)).not.toBeInTheDocument() + }) +}) \ No newline at end of file