feat(admin): analytics dashboard polish (position + scroll restore + cache)#6680
feat(admin): analytics dashboard polish (position + scroll restore + cache)#6680
Conversation
Three small quality-of-life improvements on the analytics dashboard: 1. Promote Cumulative Users to a headline chart right after the revenue KPI cards. It used to be paired with DAU halfway down the page; now it's the first thing below the MRR/ARR/Subscriptions tiles. DAU moves up to a full-width card in its old slot. 2. Remember scroll position on refresh. The dashboard layout now saves the scrollable <main>'s scrollTop to sessionStorage, keyed by pathname, and restores it on mount. Because the page grows as SWR fills in, the restore poller runs up to ~2s to keep nudging the scroll back to the saved offset until it sticks. 3. Cache SWR responses in localStorage. The SWRProvider installs a localStorage-backed Map provider that rehydrates on first render and mirrors writes in a microtask. On refresh the dashboard now paints instantly with stale cached data while each endpoint revalidates in the background — no more 25s+ wait for cold data. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Greptile SummaryThis PR makes three UX improvements to
Confidence Score: 4/5Safe to merge after fixing the missing cleanup in the scroll-restore polling loop; P2 findings are non-blocking. One P1 bug exists: the useEffect that polls scrollTop never returns a cleanup function, so navigating between dashboard routes triggers a 2-second window where the old polling fights the new page's scroll state. The SWR cache and chart reposition changes are well-implemented and carry only minor P2 concerns. web/admin/app/(protected)/dashboard/layout.tsx — missing useEffect cleanup for the scroll-restore polling loop. Important Files Changed
Sequence DiagramsequenceDiagram
participant Browser
participant Layout as DashboardLayout
participant SS as sessionStorage
participant LS as localStorage
participant SWR as SWR Cache
Note over Browser,SWR: Page load (fresh or refresh)
Browser->>LS: getItem("omi-admin-swr-cache-v1")
LS-->>SWR: Rehydrate stale entries into Map
Browser->>Layout: Mount main ref
Layout->>SS: getItem("omi-admin-scroll:/dashboard/analytics")
SS-->>Layout: saved scrollTop
Layout->>Layout: tick() — set scrollTop every 50ms until stable
Note over Browser,SWR: SWR background revalidation
Browser->>SWR: fetch each endpoint
SWR-->>Browser: Fresh data replaces stale
SWR->>LS: schedulePersist() via queueMicrotask
Note over Browser,SWR: Route change (bug path)
Browser->>Layout: pathname changes — storageKey changes
Layout->>Layout: old tick() still firing — no cleanup!
Layout->>Layout: new useEffect starts fresh tick()
Note over Layout: Both polls compete for scrollTop ~2s
Reviews (1): Last reviewed commit: "feat(admin): promote Cumulative Users + ..." | Re-trigger Greptile |
| useEffect(() => { | ||
| if (!enabled) return; | ||
| const el = mainRef.current; | ||
| if (!el) return; | ||
|
|
||
| let saved: number | null = null; | ||
| try { | ||
| const raw = window.sessionStorage.getItem(storageKey); | ||
| if (raw != null) saved = parseInt(raw, 10); | ||
| } catch { | ||
| /* noop */ | ||
| } | ||
| if (saved == null || Number.isNaN(saved)) return; | ||
|
|
||
| let tries = 0; | ||
| const maxTries = 40; // ~2s at 50ms cadence | ||
| const tick = () => { | ||
| const node = mainRef.current; | ||
| if (!node) return; | ||
| node.scrollTop = saved!; | ||
| if (Math.abs(node.scrollTop - saved!) > 2 && tries++ < maxTries) { | ||
| setTimeout(tick, 50); | ||
| } | ||
| }; | ||
| tick(); | ||
| }, [enabled, mainRef, storageKey]); |
There was a problem hiding this comment.
Polling loop leaks across route changes — no cleanup
The scroll-restore useEffect starts a setTimeout chain but returns no cleanup function. When storageKey changes (user navigates from /analytics to another dashboard route), React discards the old effect without cancelling the pending timers. For ~2 s afterward, the old tick continues setting mainRef.current.scrollTop to the previous page's saved value, fighting any scroll state the new route is trying to restore.
| useEffect(() => { | |
| if (!enabled) return; | |
| const el = mainRef.current; | |
| if (!el) return; | |
| let saved: number | null = null; | |
| try { | |
| const raw = window.sessionStorage.getItem(storageKey); | |
| if (raw != null) saved = parseInt(raw, 10); | |
| } catch { | |
| /* noop */ | |
| } | |
| if (saved == null || Number.isNaN(saved)) return; | |
| let tries = 0; | |
| const maxTries = 40; // ~2s at 50ms cadence | |
| const tick = () => { | |
| const node = mainRef.current; | |
| if (!node) return; | |
| node.scrollTop = saved!; | |
| if (Math.abs(node.scrollTop - saved!) > 2 && tries++ < maxTries) { | |
| setTimeout(tick, 50); | |
| } | |
| }; | |
| tick(); | |
| }, [enabled, mainRef, storageKey]); | |
| let tries = 0; | |
| const maxTries = 40; // ~2s at 50ms cadence | |
| let timeoutId: ReturnType<typeof setTimeout> | null = null; | |
| let cancelled = false; | |
| const tick = () => { | |
| if (cancelled) return; | |
| const node = mainRef.current; | |
| if (!node) return; | |
| node.scrollTop = saved!; | |
| if (Math.abs(node.scrollTop - saved!) > 2 && tries++ < maxTries) { | |
| timeoutId = setTimeout(tick, 50); | |
| } | |
| }; | |
| tick(); | |
| return () => { | |
| cancelled = true; | |
| if (timeoutId != null) clearTimeout(timeoutId); | |
| }; |
| const STORAGE_KEY = 'omi-admin-swr-cache-v1'; | ||
| const MAX_ENTRY_BYTES = 1_000_000; | ||
| const MAX_TOTAL_BYTES = 5_000_000; | ||
|
|
||
| type CacheEntry = { data: unknown; error: unknown; isValidating: boolean; isLoading: boolean }; | ||
|
|
||
| function createLocalStorageProvider(): Map<string, CacheEntry> { | ||
| if (typeof window === 'undefined') { | ||
| return new Map(); | ||
| } | ||
|
|
||
| let hydrated: [string, CacheEntry][] = []; | ||
| try { | ||
| const raw = window.localStorage.getItem(STORAGE_KEY); | ||
| if (raw) hydrated = JSON.parse(raw); | ||
| } catch (err) { | ||
| console.warn('SWR cache rehydrate failed:', err); | ||
| } | ||
|
|
||
| const map = new Map<string, CacheEntry>(hydrated); | ||
|
|
||
| let persistScheduled = false; | ||
| const schedulePersist = () => { | ||
| if (persistScheduled) return; | ||
| persistScheduled = true; | ||
| queueMicrotask(() => { | ||
| persistScheduled = false; | ||
| try { | ||
| // Only persist entries that actually carry data, skip errored / | ||
| // in-flight entries and anything too large to fit in the quota. | ||
| const entries: [string, CacheEntry][] = []; | ||
| let totalBytes = 0; | ||
| map.forEach((value, key) => { | ||
| if (totalBytes >= MAX_TOTAL_BYTES) return; | ||
| if (value == null || value.error != null || value.data == null) return; | ||
| const serialized = JSON.stringify([key, { data: value.data }]); | ||
| if (serialized.length > MAX_ENTRY_BYTES) return; | ||
| if (totalBytes + serialized.length > MAX_TOTAL_BYTES) return; | ||
| entries.push([key, { data: value.data, error: null, isValidating: false, isLoading: false }]); | ||
| totalBytes += serialized.length; | ||
| }); | ||
| window.localStorage.setItem(STORAGE_KEY, JSON.stringify(entries)); |
There was a problem hiding this comment.
Stale analytics data shown on revalidation error
localStorage cache rehydrates stale data on every page load. If a background revalidation then returns an error (network hiccup, token expiry, etc.), SWR keeps the last-known data alongside the error. The admin dashboard convention in CLAUDE.md is explicit: "UI on error: clear stale data, show N/A — never display old data with error flag." The components that consume these hooks would need to explicitly check if (error) showNA() rather than falling through to the cached value. Confirm that all affected data hooks enforce this rule — or have the provider clear a key's cached value on error via the onError config callback so the pattern is enforced globally rather than relying on each chart to handle it.
| export function SWRProvider({ children }: { children: ReactNode }) { | ||
| const provider = useMemo(() => { | ||
| const cache = createLocalStorageProvider(); | ||
| return () => cache; | ||
| }, []); |
There was a problem hiding this comment.
Cache not cleared on logout — stale admin data persists in localStorage
createLocalStorageProvider is instantiated once via useMemo and the localStorage key omi-admin-swr-cache-v1 is never removed when the admin logs out. The next person who opens the browser (even if they log in as a different admin) will immediately see the previous session's analytics data during rehydration. Since this is sensitive business data, consider clearing the key in the auth provider's logout handler or on 401/403 responses.
Summary
Three UX improvements on
/dashboard/analytics.1. Cumulative Users promoted to headline position
Moves the Cumulative Users card from deep in the analytics section (paired with DAU) to right after the Revenue KPI cards, before MRR Over Time. DAU gets its own full-width card in the old slot.
2. Scroll position persists on refresh
The dashboard layout's scrollable
<main>now savesscrollToptosessionStorage(keyed by pathname) on scroll and restores it on mount. Because the page grows as SWR fills in, a ~2s poller keeps nudging the scroll back until it sticks.3. Instant refresh via SWR localStorage cache
SWRProvidernow installs a localStorage-backed Map cache provider that rehydrates on first render and mirrors writes in a microtask. On refresh the dashboard paints instantly with stale cached data while each endpoint revalidates in the background — no more cold-start waits while the 15+ charts fetch from scratch.Quota-safe: 1MB per entry, 5MB total, skips error/empty entries, silently drops on quota-exceeded.
Test plan
/dashboard/analytics, confirm Cumulative Users card appears above MRR Over Time, DAU is below🤖 Generated with Claude Code