Make Crime map points draggable - #45
Conversation
Crime markers were fixed after placement, forcing users to repick locations. Enable MapLibre dragging, update only local coordinates and overlays while moving, cancel stale work at drag start, and debounce the final refresh for 350 ms. Constraint: Existing map-click, address search, A/B comparison, and mode ownership stay intact. Rejected: Refresh on every drag event | It would create request storms and stale results. Confidence: high Scope-risk: narrow Reversibility: clean Directive: Drag callbacks must not start analysis before the settle timer. Tested: VITE_FEATURE_DIARY=1 VITE_TRACT_CRIME_SNAPSHOT=1 npm run validate; browser smoke; npm audit --audit-level=high
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ba342049df
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| updateDraggedPoint(target, position); | ||
| publishCurrentSelection(undefined, { origin: 'map' }); | ||
| onPointChange(target); | ||
| void requestRefresh(); |
There was a problem hiding this comment.
When dragging begins while a coordinator-owned refresh is in flight—including during initial Crime loading—refreshOwner.cancel() makes that request return superseded, which src/mode_coordinator.js does not settle. The replacement call here invokes the route's private requestRefresh() directly, bypassing coordinator status publication, so a successful drag refresh can leave the surface skeleton, aria-busy, and data status stuck in the loading state indefinitely; route this refresh through the coordinator or otherwise propagate its result to the status owner.
Useful? React with 👍 / 👎.
| onDragStart: () => { | ||
| refreshOwner.cancel(); | ||
| publishCurrentSelection(undefined, { origin: 'map' }); |
There was a problem hiding this comment.
If the user starts another adjustment during the 450 ms fitBoundsWithPanel animation launched by the previous settled drag, this cancellation only invalidates the refresh and does not stop the map animation or release the points controller's programmatic-move ownership. The map can therefore continue translating beneath a held pointer, moving the marker away from the intended drop position before dragend; stop the active camera transition on drag start or avoid initiating a fit for settled marker drags.
Useful? React with 👍 / 👎.
| markerA = createMapMarker({ | ||
| color: '#c86b00', | ||
| className: 'analysis-marker analysis-marker--a', | ||
| draggable: true, |
There was a problem hiding this comment.
For keyboard-only users, enabling MapLibre's pointer dragging does not provide any way to focus and move the marker: the added wiring listens only for drag events, while localizeMapMarker supplies only a title and aria-label. The label now advertises location adjustment that cannot be operated without a pointer, so add focusable keyboard movement controls (for example, arrow-key adjustments) or an equivalent accessible adjustment interface.
Useful? React with 👍 / 👎.
| onSettled: (position) => { | ||
| updateDraggedPoint(target, position); |
There was a problem hiding this comment.
If Point B is cleared shortly after being dropped, the panel sets centerBLonLat to null but waits 300 ms before refreshing, while this 350 ms settle callback remains armed. For example, clearing B more than 50 ms after dragend lets this callback run before the panel refresh; updateDraggedPoint then recreates B, so the delayed refresh observes the resurrected comparison and the user's clear action is undone. Cancel or version pending settles when a point is replaced or cleared, or verify that the target still exists before committing.
Useful? React with 👍 / 👎.
1 participant
Summary
Verification
Boundaries