Skip to content

Commit eeef954

Browse files
MartinCastroAlvarezmartin-castro-laminr-aiclaude
authored
fix(spa): skeleton only the table on a list filter change (#508) (#509)
Changing a list_filter (or page / search / ordering) replaced the whole changelist with a full-page skeleton until the new rows arrived — the title, search box, filter chips and Customize button all vanished, making each filter interaction feel like a hard reload. Django's changelist only re-renders the results table. useList is backed by useSwrCache, whose cache key encodes the filters. A filter change → new key → the hook reset data to the new key's cache (null for an unseen combo), and ListPage shows its full-page skeleton whenever `loading && !data`, so the page blanked. That key-reset is right for a detail view (object→object must not flash the previous record, #416) but wrong for a list, where the chrome and columns are filter-independent. Add a `keepPreviousData` option to useSwrCache: on a key change it keeps the previous value on screen and shows a foreground load instead of blanking (adopting the new key's cache immediately when it exists). useList opts in; useDetail keeps the reset. ListPage is remounted per model so the retained-data behaviour is scoped to same-model filter/page changes and a model switch still resets cleanly. Result: the header stays put and only the Table shows skeleton rows (its built-in loading state) on a filter change. Co-authored-by: Martin Castro Laminrs <mcastro@laminr.ai> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 360d524 commit eeef954

4 files changed

Lines changed: 100 additions & 8 deletions

File tree

frontend/apps/web/src/App.tsx

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Route, Routes } from 'react-router-dom';
1+
import { Route, Routes, useParams } from 'react-router-dom';
22

33
import { ApiError, useRegistry } from '@dar/data';
44

@@ -11,6 +11,15 @@ import { LoginPage } from './pages/LoginPage';
1111
import { CreatePage } from './pages/CreatePage';
1212
import { ToastProvider } from './toast';
1313

14+
// Remount ListPage when the model changes so per-model state (selection,
15+
// retained "keep previous data" rows) resets cleanly on a model switch —
16+
// while filter / page / search changes within a model keep the same
17+
// instance so only the table skeletons, not the whole page (#368).
18+
function KeyedListPage() {
19+
const { appLabel, modelName } = useParams<{ appLabel: string; modelName: string }>();
20+
return <ListPage key={`${appLabel}/${modelName}`} />;
21+
}
22+
1423
export function App() {
1524
const registry = useRegistry();
1625

@@ -36,7 +45,7 @@ export function App() {
3645
<ErrorBoundary>
3746
<Routes>
3847
<Route path="/" element={<HomePage />} />
39-
<Route path=":appLabel/:modelName" element={<ListPage />} />
48+
<Route path=":appLabel/:modelName" element={<KeyedListPage />} />
4049
{/* Literal `add` is ranked above the `:pk` route by React
4150
Router, so /app/model/add opens the create form, not a
4251
detail with pk="add". */}

frontend/packages/data/src/list-context.tsx

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,11 @@ export function useList(params: UseListParams): ListState {
8888
// Keep the list live without a manual reload: poll in the
8989
// background and re-validate on focus (the hook's default).
9090
refetchInterval: LIST_REFETCH_INTERVAL_MS,
91+
// On a filter / page / search change the cache key changes; keep the
92+
// prior response on screen so the page chrome (columns, header) stays
93+
// put and only the table shows a loading skeleton, instead of the
94+
// whole page blanking to a full-page skeleton (#368).
95+
keepPreviousData: true,
9196
});
9297
}
9398

frontend/packages/data/src/swr-cache.test.ts

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,4 +66,60 @@ describe('useSwrCache', () => {
6666
expect(raw).not.toBeNull();
6767
expect(JSON.parse(raw as string)).toEqual({ v: 'persisted' });
6868
});
69+
70+
it('blanks to a loading state on a key change by default (#416)', async () => {
71+
// Detail-style: a new key with no cache must not keep the previous
72+
// record's data on screen — it would flash the wrong record.
73+
const fetcher = vi.fn().mockResolvedValue({ v: 'a' });
74+
const { result, rerender } = renderHook(
75+
({ k }: { k: string }) => useSwrCache({ cacheKey: k, fetcher, ...opts }),
76+
{ initialProps: { k: 'kc-a' } },
77+
);
78+
await waitFor(() => expect(result.current.data).toEqual({ v: 'a' }));
79+
80+
fetcher.mockResolvedValueOnce({ v: 'b' });
81+
rerender({ k: 'kc-b' });
82+
// New uncached key → blanked + loading until the fetch lands.
83+
expect(result.current.data).toBeNull();
84+
expect(result.current.loading).toBe(true);
85+
await waitFor(() => expect(result.current.data).toEqual({ v: 'b' }));
86+
});
87+
88+
it('keeps the previous data on a key change when keepPreviousData is set (#368)', async () => {
89+
// List-style: a filter change (new key, no cache) keeps the prior
90+
// rows on screen and shows a foreground load, so the page chrome
91+
// stays put and only the table skeletons.
92+
const fetcher = vi.fn().mockResolvedValue({ v: 'page1' });
93+
const { result, rerender } = renderHook(
94+
({ k }: { k: string }) =>
95+
useSwrCache({ cacheKey: k, fetcher, keepPreviousData: true, ...opts }),
96+
{ initialProps: { k: 'kp-1' } },
97+
);
98+
await waitFor(() => expect(result.current.data).toEqual({ v: 'page1' }));
99+
100+
fetcher.mockResolvedValueOnce({ v: 'page2' });
101+
rerender({ k: 'kp-2' });
102+
// Previous data retained (not blanked) while loading the new key.
103+
expect(result.current.data).toEqual({ v: 'page1' });
104+
expect(result.current.loading).toBe(true);
105+
await waitFor(() => expect(result.current.data).toEqual({ v: 'page2' }));
106+
});
107+
108+
it('adopts the new key cached value immediately even with keepPreviousData', async () => {
109+
window.localStorage.setItem('kp-b', JSON.stringify({ v: 'cached-b' }));
110+
const fetcher = vi.fn().mockResolvedValue({ v: 'fresh-a' });
111+
const { result, rerender } = renderHook(
112+
({ k }: { k: string }) =>
113+
useSwrCache({ cacheKey: k, fetcher, keepPreviousData: true, ...opts }),
114+
{ initialProps: { k: 'kp-a' } },
115+
);
116+
await waitFor(() => expect(result.current.data).toEqual({ v: 'fresh-a' }));
117+
118+
fetcher.mockResolvedValueOnce({ v: 'fresh-b' });
119+
rerender({ k: 'kp-b' });
120+
// The new key has a cache → show it at once (no stale-a, no blank),
121+
// then revalidate to the canonical value.
122+
expect(result.current.data).toEqual({ v: 'cached-b' });
123+
await waitFor(() => expect(result.current.data).toEqual({ v: 'fresh-b' }));
124+
});
69125
});

frontend/packages/data/src/swr-cache.ts

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,21 @@ export interface UseSwrCacheArgs<T> {
5353
* current data, not whatever was cached when you left.
5454
*/
5555
refetchOnFocus?: boolean;
56+
/**
57+
* What to show on screen while the *next* key's value loads after the
58+
* cache key changes.
59+
*
60+
* - `false` (default): adopt the new key's cached value immediately,
61+
* blanking to a loading state when it has none. Right for a detail
62+
* view, where keeping the previous object's fields on screen would
63+
* flash the *wrong* record (#416).
64+
* - `true`: keep the previous value on screen and show a foreground
65+
* load instead of blanking. Right for a list, where the chrome
66+
* (columns/header) is key-independent and only the table rows change
67+
* — so a filter/page change skeletons just the table, not the whole
68+
* page (#368).
69+
*/
70+
keepPreviousData?: boolean;
5671
}
5772

5873
function readCache<T>(key: string): T | null {
@@ -81,23 +96,30 @@ export function useSwrCache<T>({
8196
deps = [],
8297
refetchInterval = 0,
8398
refetchOnFocus = true,
99+
keepPreviousData = false,
84100
}: UseSwrCacheArgs<T>): SwrState<T> {
85101
const cached = useMemo<T | null>(() => readCache<T>(cacheKey), [cacheKey]);
86102
const [data, setData] = useState<T | null>(cached);
87103
const [loading, setLoading] = useState<boolean>(cached === null);
88104
const [error, setError] = useState<Error | null>(null);
89105

90106
// When the cache key changes (e.g. navigating from one object's detail
91-
// to a different object's), reset to the NEW key's cached value during
92-
// render. Otherwise `data` keeps the previous key's value — `useState`
93-
// initializers only run on mount — and the view shows stale content
94-
// until the background fetch lands (#416: detail→detail nav flashed the
95-
// prior record). Setting state during render is a supported React
107+
// to a different object's, or changing a list filter), reconcile state
108+
// during render. `useState` initializers only run on mount, so without
109+
// this `data` would keep the previous key's value until the background
110+
// fetch lands. Setting state during render is a supported React
96111
// pattern: it re-renders immediately, before paint, so no flicker.
112+
//
113+
// `keepPreviousData` chooses the behaviour: the default blanks to the
114+
// new key's cache (so a detail view never flashes the wrong record,
115+
// #416); `true` keeps the previous value on screen and just shows a
116+
// foreground load (so a list filter change skeletons only the table,
117+
// not the whole page, #368). The new key's cached value is still
118+
// adopted immediately when it exists.
97119
const keyRef = useRef(cacheKey);
98120
if (keyRef.current !== cacheKey) {
99121
keyRef.current = cacheKey;
100-
setData(cached);
122+
if (!keepPreviousData || cached !== null) setData(cached);
101123
setLoading(cached === null);
102124
setError(null);
103125
}

0 commit comments

Comments
 (0)