feat: /deputes and /votes skeletons using ContentSkeleton (MON-215) - #302
Conversation
Route-level loading.tsx skeletons for /deputes and /votes, matching each page's real row grid (avatar/name/department, date/title/theme/result), replacing the generic root loading.tsx fallback for these two routes. /deputes has no client-side loading state to gate - the list is filtered client-side over data already fetched server-side - so its skeleton only applies to the route-level initial-navigation case. /votes does have a client-side refetch (useSWR on filter/pagination changes): its bare "Chargement..." text is replaced with the same VoteRowSkeleton, gated by MON-214's useLoadingPhase so fast responses never flash a loader and 200ms-1.5s shows a subtle inline AsyncStatus before the full skeleton.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🧪 dbt data-health check — ✅ passedValidates prod data health. A failure means current prod data is stale/broken, not necessarily that this PR is wrong. |
Walid-peach
left a comment
There was a problem hiding this comment.
Attention Score
60/100 - NEEDS YOUR ATTENTION
- Base 100
- -30: 1 Must Fix finding
- -10: Testing / Validation Gaps section is non-empty
Reason to take a closer look: the route-level/votesskeleton under-represents the real initial page height by up to 5x, which produces exactly the visible layout jump this issue's acceptance criteria explicitly rules out.
Summary
This PR wires MON-214's ContentSkeleton/AsyncStatus primitives into /deputes and /votes: new route-level loading.tsx skeletons for initial navigation, plus a timing-policy-gated skeleton replacing /votes' bare "Chargement…" text for client-side filter/pagination refetches. The scope correction on DeputiesClient.tsx (no client-side loading state exists there, so nothing to gate beyond the route-level skeleton) is well-reasoned and clearly documented in both the code and the PR description. One row-count mismatch needs fixing before this meets its own "no material CLS" acceptance criterion.
Must Fix
frontend/src/app/votes/loading.tsx:9setsROW_COUNT = 10, butvotes/page.tsxfetchesapi.votes.list({ limit: 50 })and renders all of it with no client-side windowing (VotesClient.tsxmaps over the fullvotesarray). On a cache-miss/first navigation to/voteswith no filters applied — the default, most common case, not an edge case — the skeleton shows 10 rows, then the real page mounts with up to 50, growing the list section roughly 5x taller. That's a large, visible layout jump on exactly the surface this PR's own acceptance criterion ("Loading and final content transitions introduce no material cumulative layout shift") targets.frontend/src/app/deputes/loading.tsxdoesn't have this problem — itsROW_COUNT = 10already matchesDeputiesClient.tsx's realPAGE_SIZE = 10. Suggest bumpingvotes/loading.tsx'sROW_COUNTto 50 to match the real initial render (the in-pageVotesClient.tsxrefetch skeleton's smallerSKELETON_ROW_COUNT = 8is a different, lower-risk situation — the hero/header are already on screen and stable during a refetch, so a partial row-count mismatch there is far less jarring than an entire page growing after first paint).
Should Fix
None.
Nice to Have
- Route-level
loading.tsx(both files) has no way to honor the "no loader up to ~200ms" tier of the timing policy — Next.js renders the Suspense fallback as soon as the boundary is hit, with no client-side timer to delay it the wayuseLoadingPhasedoes forVotesClient.tsx's in-page case. This isn't a regression (the previous generic rootloading.tsxhad the same zero-delay characteristic), and there's no obvious low-effort fix within the Next.js App Router's Suspense-basedloading.tsxconvention, but worth a one-line note in the PR description or a follow-up issue so it doesn't read as an oversight later. frontend/src/app/votes/VotesClient.tsx's newAsyncStatusinline-phase branch wraps a single status in its own<div style={{ padding: '20px 0' }}>— consistent with the removed bare-text branch's own wrapping div, so no behavior change, just flagging thatAsyncStatus'sclassNameprop is being used to addflex justify-centerrather than the parent div, which works but reads slightly indirect on first glance.
Testing / Validation Gaps
The Must Fix above wasn't caught by the automated tests, since none of the three new VotesClient loading-phase tests or the VotesLoading/DeputiesLoading route-skeleton tests assert row count against the real page's data size — they only check aria-busy, the role="status"/sr-only label, and that skeleton blocks are aria-hidden. Worth adding an assertion (e.g. VotesLoading renders the same row count as VotesPage's PAGE_SIZE) so a future edit to either constant doesn't silently drift them apart again.
Documentation / Reviewer Notes
The PR description's manual-verification section (live production API, Playwright script confirming aria-busy/skeleton presence on navigation) is a genuinely strong signal beyond the unit tests — worth preserving as the pattern for future MON-207 sub-issues (MON-216, MON-217) rather than relying on unit tests alone for a visual/timing-sensitive feature like this.
Verdict
Needs changes before merge
/votes' loading.tsx skeleton showed 10 rows while VotesPage renders up to PAGE_SIZE=50 with no client-side windowing - a cache-miss navigation would grow the list ~5x once real data landed, a large visible layout jump on the exact surface this issue's "no material CLS" criterion targets. Both loading.tsx files now import PAGE_SIZE from their client component (VotesClient/DeputiesClient, both now exporting it) instead of a separate placeholder constant, so the two can't drift apart again. Added a test on each asserting the skeleton row count equals PAGE_SIZE.
|
Applied the Must Fix from the review: CI green again after the fix (lint, tests, build). |
What
Wires MON-214's
ContentSkeleton/AsyncStatusprimitives into/deputesand/votes: route-levelloading.tsxskeletons matching each page's real row grid, plus a timing-policy-gated skeleton for/votes' client-side filter/pagination refetches.Why
Neither page had a layout-matched loading treatment.
/votesshowed a bare "Chargement…" string on every filter/pagination change (no timing discipline, so even instant responses could flash it), and both pages fell back to the generic rootloading.tsx("Chargement…" text) during initial navigation, with no relationship to the final card/row geometry.Re-reading the actual code surfaced a scope correction worth calling out:
DeputiesClient.tsxhas no client-side loading state at all. The deputies list is filtered client-side (useMemooverinitial.items, already fetched server-side) - there's nouseSWR/fetch call the client ever awaits after the page mounts, unlike/votes. The issue's scope bullet ("Replace the bare 'Chargement…' text ...DeputiesClient.tsx") assumed a loading state there that doesn't exist in the current code. The one loading moment/deputesactually has is the route-level Suspense fallback while the server awaitsfetchAllDeputies()- so that's where its skeleton lives.Changes
frontend/src/app/deputes/DeputyRowSkeleton.tsx/frontend/src/app/votes/VoteRowSkeleton.tsx- row skeletons mirroring each page's real grid (avatar/name/department + group badge + arrow for deputies; date/title+subtitle/theme badge/result badge+meter+arrow for votes), built from MON-214'sSkeletonBlock.frontend/src/app/deputes/loading.tsx/frontend/src/app/votes/loading.tsx- new route-level skeletons (hero shell +ContentSkeleton-wrapped row list) that override the generic rootloading.tsxfallback for these two routes during initial navigation.frontend/src/app/votes/VotesClient.tsx- theisLoadingbranch (previously a bare "Chargement…" string) now runs through MON-214'suseLoadingPhase:'none'shows nothing,'inline'(200ms-1.5s) shows a compactAsyncStatus,'content'(past 1.5s) shows the fullVoteRowSkeletonlist.VoteRowSkeletonis shared withvotes/loading.tsxso both stay in sync with the real layout.Testing
npx jest VotesClient VotesLoading DeputiesLoading- 14 tests: route-skeleton aria-busy/status role/sr-only label/decorative rows for both newloading.tsxfiles, plus three newVotesClienttests exercising the actual timing gate with fake timers (no loader under 200ms, inline status between 200ms-1.5s, full skeleton past 1.5s). ExistingVotesClient/DeputiesClienttests still pass unmodified in behavior.npm test- full suite, 222/222 passing (one run hit an unrelated Jest worker SIGSEGV infra flake; a retry with--runInBandpassed clean).npm run lint/npm run build- clean.frontend/.env.localpointed atNEXT_PUBLIC_API_URL=https://monelu-production.up.railway.app, the documented default) via a scripted Playwright check: confirmedrole="status"+aria-busy="true"+ non-zero skeleton blocks are present immediately after navigating to both/votesand/deputes, and that both settle to zero skeleton blocks with real rows rendered. Screenshot of the/votesskeleton mid-navigation confirms the row grid lines up with the settled page (image attached to the Linear comment / shared with the requester).Risks / Notes
DeputiesClient.tsx's render logic is intentionally untouched - see the scope-correction note above (it now exports itsPAGE_SIZEconstant sodeputes/loading.tsxcan match it, but nothing about its own behavior changed). If a client-side loading state is ever added there (e.g. a future server-backed search), it should reuseDeputyRowSkeleton+useLoadingPhasethe same wayVotesClient.tsxdoes here.votes/loading.tsx's skeleton row count (originally a hardcoded 10) didn't matchVotesClient's realPAGE_SIZE(50, rendered with no client-side windowing) - fixed by having bothloading.tsxfiles importPAGE_SIZEfrom their respective client component instead of a separate placeholder constant, plus a test on each asserting the row counts stay equal.Breaking Changes
None.