Mobile follow up number 5 #78

New issue

Closed

opened 2026-04-08 18:07:56 +00:00 by maxtkc · 0 comments

maxtkc commented

2026-04-08 18:07:56 +00:00

Owner

Summary

Three independent bug fixes for mobile and desktop UX. Notification overlap is a one-line CSS tweak. Duplicate stop-creation/move notifications are caused by handleStopCreated and handleStopDragComplete in map-controller.ts firing their own showNotification calls in addition to the global patch:change handler already showing a humanLabel notification — fix is to remove the redundant calls. Stop drag on mobile is broken because the drag uses only mousedown/mousemove/mouseup events which MapLibre does not synthesize from touch on mobile — fix is to add parallel touchstart/touchmove/touchend handlers via this.map.on(...), which receive MapTouchEvent with the same point and lngLat properties as MapMouseEvent.

Relevant context

src/modules/notification-system.ts:36 — container class fixed top-28 left-2 z-[100]
src/index.ts:190–192 — global patch:change listener calls notifications.showInfo(humanLabel(r?.patch))
src/modules/map-controller.ts:750–795 — handleStopDragComplete (line 766) and handleStopCreated (line 794) each call this.showNotification(...), duplicating the patch notification
src/modules/interaction-handler.ts — drag state machine uses mousedown/mousemove/mouseup via this.map.on(...). MapTouchEvent (maplibre-gl.d.ts:9727) exposes identical .point (canvas pixels) and .lngLat properties plus .preventDefault() that blocks MapLibre pan/zoom. It also has .points: Point[] for multi-touch detection.
src/modules/interaction-handler.ts:87–119 — setupEventListeners(): where to add the three new touch bindings
src/modules/interaction-handler.ts:471–491 — destroy(): where to add map.off() cleanup for the three touch bindings

Phase 1: Fix notification overlap

Goal: Push the notification container down ~16px so it clears the bottom of the search card.

In src/modules/notification-system.ts:36, change the container class from fixed top-28 left-2 z-[100] space-y-2 max-w-xs to fixed top-32 left-2 z-[100] space-y-2 max-w-xs (top-32 = 128px, up from 112px).

Phase 2: Fix duplicate notifications

Goal: Each stop creation or move should produce exactly one notification (from the global patch:change handler, which already shows a human-readable label).

Why duplicates happen: handleStopCreated and handleStopDragComplete each explicitly call this.showNotification(...) after updating the DB. But gtfsParser.createStop and gtfsParser.updateStopCoordinates both go through the patch system, which fires patch:change, which runs notifications.showInfo(humanLabel(r?.patch)) in index.ts. Two notifications for one operation.

In src/modules/map-controller.ts, remove the this.showNotification(\Stop ${stop_id} created`, 'success')call fromhandleStopCreated(~line 794). Keep thelayerManager?.updateStopsData()` call.
In src/modules/map-controller.ts, remove the this.showNotification(\Stop ${stop_id} coordinates updated`, 'success')call fromhandleStopDragComplete(~line 766). Keep thelayerManager?.updateStopsData()call and the error-pathshowNotification` (error notifications are fine to keep as they're not covered by the patch system).

Phase 3: Fix mobile stop drag

Goal: Stop dragging works on mobile (touch) using the same state machine as mouse drag.

Why it's broken: MapLibre does not translate touch events into mousedown/mousemove/mouseup in a way that the current drag detection picks up. Touch events need their own handlers.

Approach: Add touchstart, touchmove, touchend listeners via this.map.on(...). MapTouchEvent has .point (canvas pixel Point), .lngLat, .points (array for multi-touch guard), and .preventDefault() (blocks MapLibre pan/zoom). The handlers mirror the mouse ones exactly.

In src/modules/interaction-handler.ts, import MapTouchEvent from 'maplibre-gl' alongside the existing MapMouseEvent import.

In setupEventListeners(), after the existing mouse bindings, add:

this.map.on('touchstart', this.handleTouchStart.bind(this));
this.map.on('touchmove', this.handleTouchMove.bind(this));
this.map.on('touchend', this.handleTouchEnd.bind(this));

Add handleTouchStart(e: MapTouchEvent): void — same logic as handleMouseDown but:
- Guard: if (e.points.length !== 1) return; (ignore multi-touch)
- Use e.point for queryRenderedFeatures
- Call e.preventDefault() (instead of e.preventDefault() on the mouse event) to block MapLibre pan/zoom when drag starts
Add handleTouchMove(e: MapTouchEvent): void — same logic as handleMouseMove but:
- Guard: if (e.points.length !== 1) return;
- Use e.lngLat for coordinates
- Call e.preventDefault() to block scroll during drag
Add handleTouchEnd(_e: MapTouchEvent): void — delegates directly to this.handleMouseUp() (no coordinate read needed, same cleanup logic).
In destroy(), add corresponding this.map.off(...) calls for the three touch listeners (mirror the existing mouse .off calls).

Gotchas:

MapTouchEvent.point is the centroid of all touches, which is fine since we guard for single-touch.
e.preventDefault() on touchstart prevents both pan and pinch-zoom for that gesture — this is correct since we're taking over the touch for dragging.
The touchend event's lngLat/point may be undefined (finger already lifted); that's why handleTouchEnd delegates to handleMouseUp which reads from stopsGeoJSON rather than the event coordinates.

Original Issue

A few more issues:

Notifications on both mobile and web need to be moved a few pixels down to avoid overlapping with the search bar
Duplicate notification on stop creation and move ( Created stops / dbd9061f-0dbd-45c6-9ad6-19e268fb1aa0 and Stop dbd9061f-0dbd-45c6-9ad6-19e268fb1aa0 created)
I might have added a broken version of this, but I think the best would be active stops are moveable and tap and hold and drag or just drag. Whichever is easier/cleaner in the code. Right now, sometimes when I tap on the stop, it acts as if it's been moved. Maybe some sort of race condition. We should not have that happen on initial tap, but we can allow move after selected

## Summary Three independent bug fixes for mobile and desktop UX. Notification overlap is a one-line CSS tweak. Duplicate stop-creation/move notifications are caused by `handleStopCreated` and `handleStopDragComplete` in `map-controller.ts` firing their own `showNotification` calls in addition to the global `patch:change` handler already showing a `humanLabel` notification — fix is to remove the redundant calls. Stop drag on mobile is broken because the drag uses only `mousedown`/`mousemove`/`mouseup` events which MapLibre does not synthesize from touch on mobile — fix is to add parallel `touchstart`/`touchmove`/`touchend` handlers via `this.map.on(...)`, which receive `MapTouchEvent` with the same `point` and `lngLat` properties as `MapMouseEvent`. ## Relevant context - `src/modules/notification-system.ts:36` — container class `fixed top-28 left-2 z-[100]` - `src/index.ts:190–192` — global `patch:change` listener calls `notifications.showInfo(humanLabel(r?.patch))` - `src/modules/map-controller.ts:750–795` — `handleStopDragComplete` (line 766) and `handleStopCreated` (line 794) each call `this.showNotification(...)`, duplicating the patch notification - `src/modules/interaction-handler.ts` — drag state machine uses `mousedown`/`mousemove`/`mouseup` via `this.map.on(...)`. `MapTouchEvent` (maplibre-gl.d.ts:9727) exposes identical `.point` (canvas pixels) and `.lngLat` properties plus `.preventDefault()` that blocks MapLibre pan/zoom. It also has `.points: Point[]` for multi-touch detection. - `src/modules/interaction-handler.ts:87–119` — `setupEventListeners()`: where to add the three new touch bindings - `src/modules/interaction-handler.ts:471–491` — `destroy()`: where to add `map.off()` cleanup for the three touch bindings --- ## Phase 1: Fix notification overlap **Goal:** Push the notification container down ~16px so it clears the bottom of the search card. - [x] In `src/modules/notification-system.ts:36`, change the container class from `fixed top-28 left-2 z-[100] space-y-2 max-w-xs` to `fixed top-32 left-2 z-[100] space-y-2 max-w-xs` (`top-32` = 128px, up from 112px). --- ## Phase 2: Fix duplicate notifications **Goal:** Each stop creation or move should produce exactly one notification (from the global `patch:change` handler, which already shows a human-readable label). **Why duplicates happen:** `handleStopCreated` and `handleStopDragComplete` each explicitly call `this.showNotification(...)` after updating the DB. But `gtfsParser.createStop` and `gtfsParser.updateStopCoordinates` both go through the patch system, which fires `patch:change`, which runs `notifications.showInfo(humanLabel(r?.patch))` in `index.ts`. Two notifications for one operation. - [x] In `src/modules/map-controller.ts`, remove the `this.showNotification(\`Stop ${stop_id} created\`, 'success')` call from `handleStopCreated` (~line 794). Keep the `layerManager?.updateStopsData()` call. - [x] In `src/modules/map-controller.ts`, remove the `this.showNotification(\`Stop ${stop_id} coordinates updated\`, 'success')` call from `handleStopDragComplete` (~line 766). Keep the `layerManager?.updateStopsData()` call and the error-path `showNotification` (error notifications are fine to keep as they're not covered by the patch system). --- ## Phase 3: Fix mobile stop drag **Goal:** Stop dragging works on mobile (touch) using the same state machine as mouse drag. **Why it's broken:** MapLibre does not translate touch events into `mousedown`/`mousemove`/`mouseup` in a way that the current drag detection picks up. Touch events need their own handlers. **Approach:** Add `touchstart`, `touchmove`, `touchend` listeners via `this.map.on(...)`. `MapTouchEvent` has `.point` (canvas pixel `Point`), `.lngLat`, `.points` (array for multi-touch guard), and `.preventDefault()` (blocks MapLibre pan/zoom). The handlers mirror the mouse ones exactly. - [x] In `src/modules/interaction-handler.ts`, import `MapTouchEvent` from `'maplibre-gl'` alongside the existing `MapMouseEvent` import. - [x] In `setupEventListeners()`, after the existing mouse bindings, add: ```ts this.map.on('touchstart', this.handleTouchStart.bind(this)); this.map.on('touchmove', this.handleTouchMove.bind(this)); this.map.on('touchend', this.handleTouchEnd.bind(this)); ``` - [x] Add `handleTouchStart(e: MapTouchEvent): void` — same logic as `handleMouseDown` but: - Guard: `if (e.points.length !== 1) return;` (ignore multi-touch) - Use `e.point` for `queryRenderedFeatures` - Call `e.preventDefault()` (instead of `e.preventDefault()` on the mouse event) to block MapLibre pan/zoom when drag starts - [x] Add `handleTouchMove(e: MapTouchEvent): void` — same logic as `handleMouseMove` but: - Guard: `if (e.points.length !== 1) return;` - Use `e.lngLat` for coordinates - Call `e.preventDefault()` to block scroll during drag - [x] Add `handleTouchEnd(_e: MapTouchEvent): void` — delegates directly to `this.handleMouseUp()` (no coordinate read needed, same cleanup logic). - [x] In `destroy()`, add corresponding `this.map.off(...)` calls for the three touch listeners (mirror the existing mouse `.off` calls). **Gotchas:** - `MapTouchEvent.point` is the centroid of all touches, which is fine since we guard for single-touch. - `e.preventDefault()` on `touchstart` prevents both pan and pinch-zoom for that gesture — this is correct since we're taking over the touch for dragging. - The `touchend` event's `lngLat`/`point` may be undefined (finger already lifted); that's why `handleTouchEnd` delegates to `handleMouseUp` which reads from `stopsGeoJSON` rather than the event coordinates. --- ## Original Issue A few more issues: - Notifications on both mobile and web need to be moved a few pixels down to avoid overlapping with the search bar - Duplicate notification on stop creation and move ( Created stops / dbd9061f-0dbd-45c6-9ad6-19e268fb1aa0 and Stop dbd9061f-0dbd-45c6-9ad6-19e268fb1aa0 created) - I might have added a broken version of this, but I think the best would be active stops are moveable and tap and hold and drag or just drag. Whichever is easier/cleaner in the code. Right now, sometimes when I tap on the stop, it acts as if it's been moved. Maybe some sort of race condition. We should not have that happen on initial tap, but we can allow move after selected