fix(geo-editor): dead first click when switching between draw tools (#889) - #1020
Conversation
Bump maplibre-gl-geo-editor to 0.10.1, which sequences a newly selected draw tool after the previous tool's asynchronous Geoman teardown. Before, enableDraw ran while the prior mode's disableAllModes() was still in flight, so the teardown swallowed the first canvas click and the new tool only started drawing on the second click. Fixes #889
✅ Deploy Preview for geolibre-app ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis change updates the maplibre-gl-geo-editor dependency version from ^0.10.0 to ^0.10.1 in two package.json files: apps/geolibre-desktop/package.json and packages/plugins/package.json. No other code or configuration changes are included. ChangesDependency version bump
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Pull request overview
Updates GeoLibre’s GeoEditor integration to pick up the upstream fix for the “dead first click” when switching directly between draw tools (e.g., Marker → Circle), by bumping maplibre-gl-geo-editor from 0.10.0 to 0.10.1.
Changes:
- Bump
maplibre-gl-geo-editordependency to^0.10.1where it is consumed. - Update
package-lock.jsonto lockmaplibre-gl-geo-editor@0.10.1(resolved URL + integrity). - Ensure both the desktop app and plugins package align on the same bumped version.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/plugins/package.json | Bumps maplibre-gl-geo-editor to ^0.10.1 for the plugins workspace. |
| apps/geolibre-desktop/package.json | Bumps maplibre-gl-geo-editor to ^0.10.1 for the desktop app. |
| package-lock.json | Locks the dependency graph to maplibre-gl-geo-editor@0.10.1 and updates metadata accordingly. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Code reviewReviewed via Bugs: None found. There is no application logic in this diff to introduce a regression; I confirmed Security: None found. No new dependencies, no changed network/input-handling code, no secrets. (Confidence: high) Performance: None found. Not applicable to a patch-version dependency bump. (Confidence: high) Quality:
CLAUDE.md: No violations. The branch ( No inline comments were posted — nothing in this diff rose to a level warranting a line-anchored finding. |
⚡ Cloudflare Pages preview
|
Summary
Fixes #889. When switching directly from one GeoEditor draw tool to another
(e.g. Marker -> Circle), the first click on the map canvas was unresponsive: the
newly selected tool only started drawing on the second click ("dead click").
Root cause
The bug lives in the
maplibre-gl-geo-editorpackage's Geoman wrapper.enableDrawMode()calledgeoman.enableDraw()synchronously while theprevious tool's
geoman.disableAllModes()teardown was still in flight (thatcall is asynchronous). The in-flight teardown then landed on top of the freshly
enabled draw mode and swallowed its first canvas click.
Fix
Bumps
maplibre-gl-geo-editor0.10.0 -> 0.10.1, which sequences the newlyselected tool's activation after the previous tool's teardown promise settles,
guarded by a monotonic request token so fast reselects (e.g.
circle -> line -> circle) can't let a stale request win. Freehand is sequenced the same way.Verification
The reported symptom is Safari/macOS-specific and timing-dependent; it does not
reproduce on Chromium/Linux (confirmed by the reporter and locally), so this was
verified by the upstream package's unit tests (which cover the deferred enable,
the reselect race, and freehand sequencing) plus root-cause analysis, rather
than a live Safari repro. The fix is browser-agnostic: it removes the sync/async
race regardless of which browser's event timing exposes it.
npm run buildpasses.
Summary by CodeRabbit