fix: add loading indicator overlay to velocity heatmap page#1504
fix: add loading indicator overlay to velocity heatmap page#1504eran132 wants to merge 1 commit intohasadna:mainfrom
Conversation
The velocity heatmap page loads data slowly but previously showed only a pink div with "loading!" text, making users think the page was broken. Replaced with a proper semi-transparent overlay with MUI CircularProgress spinner and descriptive text, consistent with loading patterns used in the gaps and timeline pages. Also added an Alert component for error states. Closes hasadna#1483 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Improves the velocity heatmap page UX by replacing the basic loading/error UI with a proper loading overlay and an error alert, using MUI components and propagating loading/error state from the rectangles layer up to the page.
Changes:
- Added page-level loading overlay with
CircularProgressand “Loading heatmap data…” text. - Added page-level error
Alertshown over the map when the data request fails. - Propagated loading/error state from
VelocityHeatmapRectanglesto the parent via optional callbacks.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/pages/velocityHeatmap/index.tsx | Adds loading overlay + error alert UI and wires callbacks from the rectangles layer. |
| src/pages/velocityHeatmap/components/VelocityHeatmapRectangles.tsx | Introduces onLoadingChange / onErrorChange props and forwards react-query state upward. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| backgroundColor: 'rgba(255, 255, 255, 0.7)', | ||
| zIndex: 1000, | ||
| gap: 12, | ||
| }}> | ||
| <CircularProgress size={48} /> | ||
| <span style={{ fontSize: '1.1em', color: '#333' }}>Loading heatmap data...</span> | ||
| </div> |
There was a problem hiding this comment.
The loading overlay uses zIndex: 1000, but .map-info .expand-button is styled with z-index: 2000 (see src/resources/map.scss). This means the expand button will remain above the overlay and still be clickable while loading, which defeats the purpose of an interaction-blocking overlay. Consider raising the overlay z-index above 2000 (or temporarily lowering/disabled the button / pointer events while loading).
| useEffect(() => { | ||
| onErrorChange?.(error ? String(error) : null) |
There was a problem hiding this comment.
onErrorChange?.(error ? String(error) : null) will render UI text like Error: ... for thrown Errors, and may degrade to [object Object] for non-Error rejections (e.g. from the API client). Consider normalizing the message (e.g., error instanceof Error ? error.message : JSON.stringify(error)), so the Alert shows a user-friendly string.
| useEffect(() => { | |
| onErrorChange?.(error ? String(error) : null) | |
| const getErrorMessage = (errorValue: unknown): string | null => { | |
| if (!errorValue) return null | |
| if (errorValue instanceof Error) return errorValue.message | |
| if (typeof errorValue === 'string') return errorValue | |
| try { | |
| return JSON.stringify(errorValue) || String(errorValue) | |
| } catch { | |
| return String(errorValue) | |
| } | |
| } | |
| useEffect(() => { | |
| onErrorChange?.(getErrorMessage(error)) |
| React.useEffect(() => { | ||
| if (setMinMax) setMinMax(minV, maxV) | ||
| }, [minV, maxV, setMinMax]) |
There was a problem hiding this comment.
This component mixes React.useEffect (for setMinMax) with the named useEffect import (for loading/error propagation). For consistency and easier scanning, it would be better to use one style throughout (e.g. switch the React.useEffect call to useEffect).
AvivAbachi
left a comment
AvivAbachi
left a comment
There was a problem hiding this comment.
Great work, thanks for contributing.
| gap: 12, | ||
| }}> | ||
| <CircularProgress size={48} /> | ||
| <span style={{ fontSize: '1.1em', color: '#333' }}>Loading heatmap data...</span> |
There was a problem hiding this comment.
Need to add translation in src\locale for he and en
you can also use ai for ar\ru
Summary
CircularProgressspinner and descriptive "Loading heatmap data..." textAlertcomponent for error states, positioned at the top of the mapVelocityHeatmapRectanglesto the parent page via callbacks, consistent with patterns used in the gaps and timeline pagesCloses #1483
Test plan
tsc --noEmit)npm run lint)🤖 Generated with Claude Code