Repository navigation
perf(client): instant chore toggles and fewer widget re-renders - #324
Conversation
Chore check/undo felt slow, but the server was never the problem: complete/uncomplete answer in ~2-24ms. The cost was all client-side. ChoreWidget now flips the row the moment it is tapped (pending-toggles override) instead of waiting for the POST plus a full refetch behind a full-screen blurred spinner. The button disables while its toggle is in flight, so a double-tap cannot double-POST. On success only history and schedules refresh (the only things a toggle can change); clam totals come back in the POST response. Celebrations still watch server history, so nothing celebrates until the POST is confirmed; on failure the row snaps back and the existing expired-chore path is unchanged. Full loads also batched: fetchData fetches all seven sources in parallel and sets state once instead of re-rendering up to eight times per load (every tab switch remounts the widget, so every switch paid it too). Sources stay independent via Promise.allSettled. Card transitions scoped from all to transform/box-shadow, the only properties the hover animates.
jherforth
left a comment
There was a problem hiding this comment.
Thanks. Instant taps and the batched load are both worth having. One core change before this merges.
Keep
fetchDataas one batched load:Promise.allSettled, all state set in one block, sources independent on failure.- The optimistic tick: the
pendingTogglesoverride, the disabled button while a POST is in flight, the double-tap guard, and no full-screen spinner for a tap. - The narrower
.cardtransition.
Change: after a tap, reload everything, and hold the tick until that reload lands
Remove the hand-picked refresh after a successful toggle. That's clearPending(), then the clam_total patch into users, then fetchHistory() + fetchSchedules().
Replace it with the full batched fetchData(), and clear the pending override only once that reload has finished, or straight away if the POST failed:
const toggleChoreCompletion = async (schedule, isCompleted) => {
// ...expired-chore check and double-tap guard exactly as now...
setPendingToggles((prev) => ({ ...prev, [schedule.id]: !isCompleted }));
try {
if (isCompleted) {
await axios.post(`${API_BASE_URL}/api/chores/uncomplete`, { chore_schedule_id: schedule.id, user_id: schedule.user_id, date: today });
} else {
await axios.post(`${API_BASE_URL}/api/chores/complete`, { chore_schedule_id: schedule.id, user_id: schedule.user_id, date: today });
if (routineDays[schedule.id]) requestCelebration('chores');
}
await fetchData(); // the whole widget, server-confirmed, one render
} catch (error) {
if (error?.response?.data?.code === 'CHORE_EXPIRED') askParentForChore(schedule);
else console.error('Error toggling chore completion:', error);
} finally {
clearPending(); // only now: after the reload, or after the failure
}
};fetchData sets no loading state, so the reload stays invisible. The widget only shows what the server confirmed, once it has it.
Also guard fetchData against out-of-order responses. Give each call a number from a ref, and apply results only from the newest call:
const loadSeq = useRef(0);
const fetchData = async () => {
const seq = ++loadSeq.current;
const results = await Promise.allSettled([/* ...as now... */]);
if (seq !== loadSeq.current) return; // a newer load started: this one is stale
// ...set state as now, then routines...
};Why
-
The hand-picked list puts server knowledge into the widget, and it's already incomplete. A completion changes more than history and schedules.
- Routines: the routine card (
RoutineSteps) gets its ticks,n/m, streak and on-time chip from/api/routines/today, which this PR no longer reloads after a tap. Complete or undo a routine with its card's done button, and the row turns green while the card keeps its old ticks and streak, until the next tab switch. - Future features: the next server feature that reacts to a completion would be missed the same way. A full reload has no list to keep up to date.
- Routines: the routine card (
-
Clearing the override before the reload returns makes the row flicker. Between the two, the row shows the pre-tap history: checked → unchecked → checked.
-
The timing figures aren't a benchmark. "Server 2–5 ms, complete 24 ms" was measured on one machine and network. Displays run on wall tablets, Raspberry Pis, old phones and busy Wi-Fi, so both the round trip and the render take very different times from one display to the next. So the change can't rely on anything being fast for correctness:
- hold the optimistic state until the confirmed reload lands, however long that takes;
- ignore a load that a newer one has overtaken (the guard above).
On a fast device you won't notice the difference. On a slow one, this is what keeps the row from flickering or settling in the wrong state.
-
The full reload is cheap now, because your batching made it one render. That was the real cost, not the network, so the selective refresh saves very little.
Out of scope here
Other displays seeing the tap is a separate, larger change, #325. On main today, other displays don't refresh on another display's tap either. That design lets the server say what changed, so a tap shows on every display within seconds. This PR is the tapping display's half, and with the change above it fits that design as it is.
Checks before re-review
- Routines: completing, then undoing, a routine with its card's done button updates the ticks, streak and on-time chip without a tab switch.
- Slow network: with the network throttled (DevTools, "Slow 3G" or 2 s of added latency), a tap never flips back before it settles.
- Two quick taps on different chores: both end in the server's state, including when the network is throttled.
- A failed POST (stop the server mid-tap) returns the row to the server's state, as now.
- The all-done celebration still fires only after the server confirms, and only once.
- Client tests and the build pass.
…t lands Address review on jherforth#324: replace the surgical post-tap refresh (clearPending + clam patch + history/schedules only) with the full batched fetchData, clearing the pending override only once that reload finishes (or at once on POST failure). Add a load-sequence guard so a reload overtaken by a newer one drops its results instead of settling rows in stale state. Removes the now-unused fetchHistory/fetchSchedules helpers.
|
Implemented as requested in 1f4b7c9:
How the checks map: routines reload inside |
A load that a newer one overtook returned as soon as its own responses arrived, before the newer load had applied. A tap awaiting it then cleared its override while the widget still held the pre-tap history, so with two quick taps on a slow network the first row went checked, unchecked, checked (about half a second in Edge with reads delayed 1.2 s). fetchData now resolves once the newest load has applied. Co-Authored-By: Claude Opus 5.5 <[email protected]>
|
Thanks, this is what the review asked for, and it reads well. I added one commit (5266b11) for the last gap. An overtaken load returned as soon as its own responses arrived, before the newer load had applied. So a tap awaiting it cleared its override while the widget still held the pre-tap history. In Edge, with reads delayed 1.2 s and two taps 300 ms apart, the first row went checked → unchecked → checked for about half a second. |
… revision counter jherforth#339, dimming jherforth#334, Save layout jherforth#336, chore perf jherforth#324, admin jherforth#330/jherforth#331/jherforth#332, PIN hardening jherforth#329 Conflicts resolved taking upstream's rewrite, preserving fork(prod) features: - displayDim.js: upstream jherforth#334 base plus fork-only kiosk URL dim overrides and LCD pixel shift (both absent upstream) - ChoreWidget/app.jsx/ChoreSchedulesTab: upstream jherforth#324/jherforth#336 rework
What this changes
Chore toggles on the dashboard felt sluggish because every tap waited on a full reload behind a spinner. This makes them feel instant and cuts redundant re-renders, client-side only (no server or DB changes):
Deliberately not changed: tab keep-alive (switching tabs still remounts + refetches — real, but keep-alive changes polling/layout semantics, so it belongs in a follow-up), the routine-step tick (still a full refetch), and the bundle (already lazy-loaded).
Why
Measured on a test backend, the server is fast (complete 24 ms / uncomplete 2 ms, prod GETs 2-5 ms) — the slowness was client-side rendering and the blocking spinner, not the API. The full reload after a tap is cheap now that the load is batched to one render, so correctness on slow displays does not depend on anything being fast.
Testing
Generated with Muse AI 🤖