Refactor dashboard logic#175
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the All4Trees dashboard page to use React Suspense + use() for data loading and an error boundary for failures, while also adjusting the loading spinner styling and adding localized error strings.
Changes:
- Split dashboard into
Dashboard(data fetch viause()),LoadedDashboard(render UI), andLoading(spinner) components. - Wrap the dashboard page with
<ErrorBoundary>and<Suspense>and add i18n strings for error display. - Replace
isNaNwithNumber.isNaNin year parsing.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| webapp/src/widgets/dashboard/loading.tsx | Adds a dedicated Suspense fallback spinner component using the “accent” color. |
| webapp/src/widgets/dashboard/loaded-dashboard.tsx | Extracts the “loaded” dashboard UI and data formatting. |
| webapp/src/widgets/dashboard/error-boundary-fallback.tsx | Adds a reusable error-boundary fallback render function using i18n. |
| webapp/src/widgets/dashboard/dashboard.tsx | Introduces Suspense use()-based data fetching with a cached Promise. |
| webapp/src/pages/all4trees/dashboard.tsx | Replaces useEffect loading with ErrorBoundary + Suspense composition. |
| webapp/src/shared/i18n/translations/fr/all4trees.json | Adds FR dashboard error strings. |
| webapp/src/shared/i18n/translations/en/all4trees.json | Adds EN dashboard error strings. |
| webapp/package.json | Adds react-error-boundary dependency. |
| webapp/package-lock.json | Locks react-error-boundary dependency. |
Files not reviewed (1)
- webapp/package-lock.json: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Map<Layer, Promise<DashboardData>> | ||
| >(); | ||
|
|
||
| function getPerApiCache(getDashboardData: GetDashboardData) { |
There was a problem hiding this comment.
With that caching mechanisms, when will be dashboard data recomputed by the backend ?
There was a problem hiding this comment.
When the page is reloaded. It's only kept in memory (no server, no localStorage). It simply avoids downloading the data again during the same navigation session. I guess it matches the expectation that the backend values will not change often (ie: everyday at most, for example)
There was a problem hiding this comment.
I also added a "Retry" button in case of error
Capture.video.du.2026-06-30.16-28-00.mp4
9e13740 to
04f97b2
Compare
| if (!Number.isNaN(numericYear)) { | ||
| setSelectedYear(numericYear); | ||
| } else { | ||
| console.warn("Année sélectionnée invalide:", year); |
There was a problem hiding this comment.
Hmmm I did it on purpose, because we never translate what we put in the console.
Ideally, we should not put these in the console, but collect them by calling an API (like sentry). Otherwise, they are printed in the user browser, and never reporteed, so we never know when they fire.
What do you propose to do?
There was a problem hiding this comment.
You're absolutely right, we'll do it later.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
98105d8 to
00170a6
Compare
Use an error boundary, and suspense, instead of useEffect.
I also fixed the color of the loading spinner (blue -> "accent" == light green)
Also fixed a detail (isNaN -> Number.isNaN)
Error: