fix(web): resolve map bounds crash, canvas zoom sizing, and add auto-reconnect on refresh - #1384
fix(web): resolve map bounds crash, canvas zoom sizing, and add auto-reconnect on refresh#1384jkboroff wants to merge 1 commit into
Conversation
…reconnect on refresh
|
Someone is attempting to deploy a commit to the Meshtastic Team on Vercel. A member of the Team first needs to authorize it. |
|
root seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
📝 WalkthroughWalkthroughThe app now auto-connects to a saved HTTP connection and resets connection state during rehydration. Map rendering supports more interactions, validates coordinates before camera changes, handles map errors, and updates control placement. ChangesConnection startup and state
Map behavior
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant App
participant AutoConnect
participant deviceStore
participant connect
App->>AutoConnect: render
AutoConnect->>deviceStore: read saved connections and selected device
AutoConnect->>connect: start selected HTTP connection
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/core/hooks/useMapFitting.ts`:
- Around line 51-52: Filter the coordinates produced in the multi-node path of
useMapFitting before passing them to boundsFromLngLat, excluding the [0, 0]
sentinel generated by toLngLat for missing positions. Preserve valid coordinates
and keep the existing single-node rejection behavior consistent.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 35fef18b-7f7f-483e-b23e-a99c7b175d29
📒 Files selected for processing (6)
apps/web/src/App.tsxapps/web/src/components/Map.tsxapps/web/src/core/hooks/useMapFitting.tsapps/web/src/core/stores/deviceStore/index.tsapps/web/src/core/utils/geo.tsapps/web/src/pages/Map/index.tsx
| const coords = nodes.map((n) => toLngLat(n.position)); | ||
| const bounds = boundsFromLngLat(coords); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Exclude the invalid [0, 0] sentinel before calculating bounds.
At Line 51, toLngLat(undefined) produces [0, 0]. boundsFromLngLat accepts this pair because both values are finite. A missing node position can then move a multi-node fit toward Null Island. This differs from the single-node path, which rejects [0, 0].
Proposed fix
- const coords = nodes.map((n) => toLngLat(n.position));
+ const coords = nodes
+ .map((n) => toLngLat(n.position))
+ .filter(
+ ([lng, lat]) =>
+ Number.isFinite(lng) &&
+ Number.isFinite(lat) &&
+ !(lng === 0 && lat === 0),
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const coords = nodes.map((n) => toLngLat(n.position)); | |
| const bounds = boundsFromLngLat(coords); | |
| const coords = nodes | |
| .map((n) => toLngLat(n.position)) | |
| .filter( | |
| ([lng, lat]) => | |
| Number.isFinite(lng) && | |
| Number.isFinite(lat) && | |
| !(lng === 0 && lat === 0), | |
| ); | |
| const bounds = boundsFromLngLat(coords); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/core/hooks/useMapFitting.ts` around lines 51 - 52, Filter the
coordinates produced in the multi-node path of useMapFitting before passing them
to boundsFromLngLat, excluding the [0, 0] sentinel generated by toLngLat for
missing positions. Preserve valid coordinates and keep the existing single-node
rejection behavior consistent.
2 participants
Description
This PR resolves three runtime stability and UX issues in the web client: map bounding box calculation crashes, map canvas zoom interaction blocking, and missing auto-reconnect on browser page refreshes.
Related Issues
N/A
Changes Made
• Map Bounds Calculation & Fitting (geo.ts & useMapFitting.ts): Replaced Turf lineString calls with direct min/max bounding box calculations and added coordinate sanity checks to prevent MapLibre camera transition
crashes (can't access property "center").
• Map Canvas Sizing & Controls Overlay (Map.tsx & pages/Map/index.tsx): Explicitly set width: "100%", height: "100%" on style and adjusted floating tool overlay positioning (top-36) so overlay elements don't
block + / - navigation zoom buttons.
• Auto-Reconnect on Page Reload (App.tsx & deviceStore/index.ts): Reset stale saved connection statuses to "disconnected" on IndexedDB store rehydration and added an AutoConnect handler that waits for IndexedDB
rehydration to finish before automatically reconnecting to the default/saved HTTP connection.
Testing Done
• Verified local HTTP / HTTPS connections over custom port & Nginx proxy.
• Verified auto-reconnect transitions directly to dashboard on cold page reload (Ctrl+F5).
• Tested map pan, touch zoom, scroll zoom, and NavigationControl zoom buttons.
Screenshots (if applicable)
N/A
Checklist
[✓] Code follows project style guidelines
[✓] Documentation has been updated or added
[ ] Tests have been added or updated
[✓] All i18n translation labels have been added (read CONTRIBUTING_I18N_DEVELOPER_GUIDE.md for more details)
Summary by CodeRabbit