feat(frontend): loading skeletons across data fetchers (#344 T3) - #346
Conversation
Replace empty-panel-during-fetch with shimmer skeleton placeholders in the 5 data-fetcher modules. Before: dashboard/plans/history/recs sat blank while the (sometimes multi-second) Promise.all resolved, then content popped in with no visual continuity. riexchange flashed plain "Loading…" text. After: every fetch renders a skeleton in the target container's shape (KPI tiles, table rows, block) at fetch start; the success render replaces children for a clean handoff; the error path calls teardownSkeleton before showing the error so a shimmer never sits next to a stale error message. - new: frontend/src/lib/skeleton.ts (createElement helpers + show/teardown lifecycle; uses data-skeleton-active marker so teardown is idempotent and safe to call from both success and error paths) - new: skeleton.test.ts (13 tests for primitives + lifecycle) - updated: dashboard.ts, plans.ts, history.ts, recommendations.ts, riexchange.ts — synchronous skeleton render before fetch, teardown on error - updated: base.css — .skeleton shimmer keyframes + .skeleton-tile + prefers-reduced-motion drops animation, keeps static muted bg - updated: dashboard.ts error path migrated from innerHTML string- templating to createElement/textContent (XSS-safe by construction) - updated: dashboard.test.ts — 2 new regression tests for the skeleton lifecycle (error path tears down; success path replaces with real KPI tiles) Verification: 1630/1630 tests pass / 1 skipped. Coverage 80.93% stmts / 82.70% lines (improved from 80.76 / 82.54 baseline). Build 522 KiB (within ±10% of 512 baseline).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughThis PR adds a skeleton UI system: DOM helper primitives and lifecycle, shimmer CSS, Jest tests, and integration of skeleton placeholders across dashboard, history, plans, recommendations, and RI exchange pages to show and teardown placeholders during async loads. ChangesLoading Skeleton UI System
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related issues
Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
frontend/src/plans.ts (1)
32-39: ⚡ Quick winMove planned-purchases skeleton setup into
loadPlannedPurchases().Right now the skeleton is only shown when
loadPlans()callsloadPlannedPurchases(). Direct calls toloadPlannedPurchases()(e.g., after actions) won’t show loading state.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/plans.ts` around lines 32 - 39, The planned-purchases skeleton setup currently lives only in the top-level initialization (next to plansList) so it runs only when loadPlans() triggers loadPlannedPurchases(); move the logic that finds the plannedContainer and calls showSkeletonRows(plannedContainer, 5, 11) into the start of loadPlannedPurchases() (and remove the duplicate from the top-level init), ensuring loadPlannedPurchases() shows the skeleton whenever it is invoked directly; keep the plans list skeleton (showSkeletonTiles) where it is or move similarly if needed, and ensure any early exits in loadPlannedPurchases() clear or hide the skeleton on error/after data render.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/history.ts`:
- Around line 161-167: The skeleton columns passed to showSkeletonRows are
incorrect: the code in the history initializer calls showSkeletonRows(listEl, 8,
10) but renderHistoryList actually renders 11 columns, producing a layout
mismatch; update the call to showSkeletonRows(listEl, 8, 11) (locate the call
near the listEl = document.getElementById('history-list') block) so the skeleton
column count matches renderHistoryList's table column count.
In `@frontend/src/lib/skeleton.ts`:
- Around line 89-96: The showSkeletonTiles function (and the similar
showSkeletonRows / showSkeletonInline helpers) currently call
teardownSkeleton(container) which only removes children if a skeleton marker is
present, so on first render existing content can be left and skeletons appended;
modify these show* functions to always remove existing children before appending
skeletons — either call a new/updated teardownSkeleton that force-clears
children when invoked (no marker check) or explicitly clear container.innerHTML
/ removeChild loop at the start of showSkeletonTiles (and the other
showSkeleton* functions) so skeletons replace any existing content and then set
container.dataset['skeletonActive'] = '1'.
In `@frontend/src/recommendations.ts`:
- Around line 240-247: The skeleton is using a hardcoded cols=10 which can
mismatch the actual table; instead compute the column count dynamically from the
rendered recommendations table (e.g., count the header cells / visible columns
including the checkbox) and pass that value to showSkeletonRows. Locate the code
around listEl and showSkeletonRows in recommendations.ts, replace the fixed 10
with a runtime-derived cols (e.g., count of header <th> or first-row cells for
'#recommendations-list') so the skeleton rows match the table shape.
In `@frontend/src/riexchange.ts`:
- Around line 84-87: The skeleton column count passed to showSkeletonRows
doesn't match the actual rendered tables (they have 8 columns), so update each
call to showSkeletonRows used for the RI tables to pass 8 columns instead of 7;
specifically, find the showSkeletonRows invocations tied to renderRIsTable (and
the other RI table rendering) and change their second argument from 7 to 8 so
the skeleton shape aligns with the real table layout.
---
Nitpick comments:
In `@frontend/src/plans.ts`:
- Around line 32-39: The planned-purchases skeleton setup currently lives only
in the top-level initialization (next to plansList) so it runs only when
loadPlans() triggers loadPlannedPurchases(); move the logic that finds the
plannedContainer and calls showSkeletonRows(plannedContainer, 5, 11) into the
start of loadPlannedPurchases() (and remove the duplicate from the top-level
init), ensuring loadPlannedPurchases() shows the skeleton whenever it is invoked
directly; keep the plans list skeleton (showSkeletonTiles) where it is or move
similarly if needed, and ensure any early exits in loadPlannedPurchases() clear
or hide the skeleton on error/after data render.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 60e16177-3281-434f-9a65-418156485160
📒 Files selected for processing (9)
frontend/src/__tests__/dashboard.test.tsfrontend/src/__tests__/skeleton.test.tsfrontend/src/dashboard.tsfrontend/src/history.tsfrontend/src/lib/skeleton.tsfrontend/src/plans.tsfrontend/src/recommendations.tsfrontend/src/riexchange.tsfrontend/src/styles/base.css
|
CI pre-commit run 25797170214 passed on commit 5e3f78b. All checks green. |
CodeRabbit pass-1 review surfaced 4 actionable issues + 1 nitpick on the loading-skeleton landing. Fix all 5: 1. lib/skeleton.ts: show* helpers were calling teardownSkeleton, which only wipes children when the marker is already set. On the FIRST render (no marker yet), a container that already held real content would end up with skeletons appended below the real content rather than replacing it. Factor activateSkeleton() that unconditionally clears children + sets the marker; route all show* helpers through it. Add 3 regression tests covering this exact scenario. 2. history.ts:167: skeleton column count was 10 but the rendered history table has 11 columns (Status / Date / Provider / Service / Type / Region / Count / Term / Upfront Cost / Monthly Savings / Plan). Bump to 11. 3. recommendations.ts:247: hardcoded cols=10 didn't match the actual rendered table (checkbox column + dynamic visible-data columns). Compute `1 + visibleColumns().length` so toggling Columns ▾ keeps the skeleton aligned with the eventual table shape. 4. riexchange.ts: both showSkeletonRows calls used incorrect column counts (7 and 5) while both rendered tables have 8 columns. Align both to 8. 5. plans.ts (nitpick): planned-purchases skeleton moved out of loadPlans into loadPlannedPurchases so direct callers of that fetch (refresh after a single action) also get the loading affordance. Verification: 1633/1634 tests pass / 1 skipped (3 new skeleton regression tests added). Coverage 80.93% / 82.70% (parity). Build 522 KiB (unchanged).
|
Pushed fix
Verification: 1633/1634 tests pass / 1 skipped (3 new regression tests). Coverage 80.93% / 82.70% (parity). Build 522 KiB unchanged. @coderabbitai review |
|
Triggering a fresh review pass on commit ✅ Actions performedReview triggered.
|
|
CR pass-2 clean. CodeRabbit acked all five fixes on commit |
Summary
Issue #344 (UI revamp phase 2) — task T3 of 4: shimmer skeleton placeholders for every data-fetcher module.
Before this PR: dashboard / plans / history / recommendations panels sat blank during fetch; riexchange flashed plain "Loading…" text. After: every fetch renders a skeleton matching the target's eventual shape (KPI tiles for the dashboard, table rows for plans/history/recs/riexchange, block for upcoming-purchases). The success render replaces children for a clean handoff; the error path tears the skeleton down before rendering the error so a shimmer never sits next to a stale failure message.
What ships
frontend/src/lib/skeleton.ts—createElement-only helpers (skeletonBox,skeletonText,skeletonTile,skeletonRow) + show/teardown lifecycle (showSkeletonTiles,showSkeletonRows,showSkeletonBlock,teardownSkeleton,isSkeletonActive).data-skeleton-activemarker on the container makes teardown idempotent and lets the error path detect a pending skeleton without disturbing already-rendered content.frontend/src/styles/base.css—.skeletonlinear-gradient +@keyframes cudly-shimmer(1.6s ease-in-out infinite).prefers-reduced-motiondrops the animation but keeps a static muted background so the "loading" affordance still reads visually.dashboard.ts,plans.ts,history.ts,recommendations.ts,riexchange.ts— synchronous skeleton render at fetch start, teardown on error. The dashboard's error path also migrated frominnerHTMLstring-template tocreateElement/textContent(XSS-safe by construction).Lifecycle contract (per plan.md §T3)
catchblock callsteardownSkeleton(container)before rendering the<p class="error">— verified by new regression tests indashboard.test.ts.Verification
skeleton.test.ts+ 2 new dashboard skeleton-lifecycle regression tests.Not in this PR
Closes neither #344 nor any other issue on its own — T4 closes the phase-2 epic.
Summary by CodeRabbit
New Features
Bug Fixes
Tests