Skip to content

Refactor dashboard logic#175

Open
severo wants to merge 18 commits into
mainfrom
refactor-dashboard-logic
Open

Refactor dashboard logic#175
severo wants to merge 18 commits into
mainfrom
refactor-dashboard-logic

Conversation

@severo

@severo severo commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator

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:

Capture d’écran du 2026-06-25 10-30-48

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 via use()), LoadedDashboard (render UI), and Loading (spinner) components.
  • Wrap the dashboard page with <ErrorBoundary> and <Suspense> and add i18n strings for error display.
  • Replace isNaN with Number.isNaN in 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.

Comment thread webapp/src/widgets/dashboard/dashboard.tsx Outdated
Comment thread webapp/src/widgets/dashboard/loaded-dashboard.tsx Outdated
Map<Layer, Promise<DashboardData>>
>();

function getPerApiCache(getDashboardData: GetDashboardData) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With that caching mechanisms, when will be dashboard data recomputed by the backend ?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also added a "Retry" button in case of error

Capture.video.du.2026-06-30.16-28-00.mp4

@severo severo force-pushed the refactor-dashboard-logic branch 2 times, most recently from 9e13740 to 04f97b2 Compare June 30, 2026 15:17
@severo severo requested review from arnaudfnr and Copilot June 30, 2026 16:20

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 9 changed files in this pull request and generated 5 comments.

Files not reviewed (1)
  • webapp/package-lock.json: Generated file

Comment thread webapp/src/widgets/dashboard/loaded-dashboard.tsx
Comment thread webapp/src/widgets/dashboard/loaded-dashboard.tsx
Comment thread webapp/src/widgets/dashboard/loaded-dashboard.tsx
Comment thread webapp/src/widgets/dashboard/dashboard.tsx Outdated
Comment thread webapp/src/widgets/dashboard/dashboard.tsx Outdated
Comment thread webapp/src/shared/i18n/translations/fr/all4trees.json Outdated
Comment thread webapp/src/widgets/dashboard/error-boundary-fallback.tsx Outdated
if (!Number.isNaN(numericYear)) {
setSelectedYear(numericYear);
} else {
console.warn("Année sélectionnée invalide:", year);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be translated

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're absolutely right, we'll do it later.

@severo severo force-pushed the refactor-dashboard-logic branch from 98105d8 to 00170a6 Compare July 2, 2026 04:21
@severo severo requested a review from arnaudfnr July 2, 2026 04:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants