fix(map): clear the hover card on marker click and camera moves (#1404)

Clicking an off-center place recenters the map under a stationary cursor, so mouseout/mouseleave never fires and the hover card sticks. Clear it on marker click and on movestart, and suppress re-shows while the camera is animating (marker rebuilds re-fire mouseenter mid-pan). feat(map): long-press + plain right-click add-place on GL maps (#1398) The GL providers only bound middle-click, so mobile had no way to add a place at a position (and Macs have no middle button). Add a 600ms touch long-press with move tolerance and the map contextmenu event - both GL libs suppress it while the right-button rotate/pitch drag is active, so the gesture keeps winning.
2026-10-22 15:06:32 +00:00 · 2026-07-04 22:26:25 +02:00
parent 634210f2b1
commit a2d80e8752
4 changed files with 287 additions and 14 deletions
@@ -29,7 +29,9 @@ vi.mock('react-leaflet', () => ({
    >
      <button
        data-testid="marker-hover-trigger"
-        onClick={() => eventHandlers?.mouseover?.({ originalEvent: { clientX: 100, clientY: 100 } })}
+        // A real mouseover never bubbles as a click to the marker, so the
+        // hover-simulation must not trigger the marker's click handler.
+        onClick={(e: any) => { e.stopPropagation(); eventHandlers?.mouseover?.({ originalEvent: { clientX: 100, clientY: 100 } }) }}
      />
      {children}
    </div>
@@ -245,6 +247,44 @@ describe('MapView', () => {
    expect(mapMock.fitBounds.mock.calls.length).toBeGreaterThan(afterFirst)
  })

+  it('FE-COMP-MAPVIEW-021: clicking a marker clears the hover tooltip (#1404)', async () => {
+    const user = userEvent.setup()
+    const places = [buildMapPlace({ id: 3, name: 'Eiffel Tower', lat: 48.8584, lng: 2.2945 })]
+    render(<MapView places={places} onMarkerClick={vi.fn()} />)
+    await user.click(screen.getByTestId('marker-hover-trigger'))
+    expect(screen.getByTestId('tooltip')).toBeTruthy()
+    // The recenter that follows the click moves the marker out from under the
+    // cursor — no mouseout will ever fire, so the click itself must clear.
+    fireEvent.click(screen.getByTestId('marker'))
+    expect(screen.queryByTestId('tooltip')).toBeNull()
+  })
+
+  it('FE-COMP-MAPVIEW-022: camera movement clears the tooltip and suppresses re-show until it ends (#1404)', async () => {
+    const user = userEvent.setup()
+    const places = [buildMapPlace({ id: 4, name: 'Louvre', lat: 48.86, lng: 2.337 })]
+    render(<MapView places={places} />)
+    await user.click(screen.getByTestId('marker-hover-trigger'))
+    expect(screen.getByTestId('tooltip')).toBeTruthy()
+
+    const findHandler = (event: string) =>
+      mapMock.on.mock.calls.find(c => c[0] === event)?.[1] as (() => void) | undefined
+    const start = findHandler('movestart zoomstart')
+    const end = findHandler('moveend zoomend')
+    expect(start).toBeTypeOf('function')
+    expect(end).toBeTypeOf('function')
+
+    fireEvent.click(screen.getByTestId('marker-hover-trigger')) // ensure hover is showing
+    start!()
+    await waitFor(() => expect(screen.queryByTestId('tooltip')).toBeNull())
+    // during the pan animation a mouseover must not re-show the card
+    fireEvent.click(screen.getByTestId('marker-hover-trigger'))
+    expect(screen.queryByTestId('tooltip')).toBeNull()
+    // once the move ends, hover works again
+    end!()
+    await user.click(screen.getByTestId('marker-hover-trigger'))
+    expect(screen.getByTestId('tooltip')).toBeTruthy()
+  })
+
  it('FE-COMP-MAPVIEW-020: a day fit expands to include the route once it arrives (#1128)', async () => {
    const L = ((await import('leaflet')).default) as unknown as { latLngBounds: ReturnType<typeof vi.fn> }
    const dayPlaces = [
@@ -139,6 +139,23 @@ function createPoiIcon(category: string) {
  return icon
 }

+// Clears the hover tooltip the moment the camera starts moving and suppresses
+// re-showing it until the move ends: after a click-recenter the marker slides
+// away under a stationary cursor, so the browser never fires mouseout — and
+// mouseover/mousemove during the pan animation would immediately re-set the
+// tooltip we just cleared (#1404).
+function CameraHoverGuard({ movingRef, onMoveStart }: { movingRef: { current: boolean }; onMoveStart: () => void }) {
+  const map = useMap()
+  useEffect(() => {
+    const start = () => { movingRef.current = true; onMoveStart() }
+    const end = () => { movingRef.current = false }
+    map.on('movestart zoomstart', start)
+    map.on('moveend zoomend', end)
+    return () => { map.off('movestart zoomstart', start); map.off('moveend zoomend', end) }
+  }, [map, movingRef, onMoveStart])
+  return null
+}
+
 // Emits the current viewport bbox on pan/zoom so the POI-explore pill can fetch
 // OSM places for the visible area.
 function ViewportController({ onViewportChange }: { onViewportChange?: (b: { south: number; west: number; north: number; east: number }) => void }) {
@@ -465,9 +482,10 @@ export const MapView = memo(function MapView({
  // Hover state for the single tooltip overlay (replaces per-marker <Tooltip>)
  const [hoveredPlace, setHoveredPlace] = useState<any>(null)
  const [tooltipPos, setTooltipPos] = useState<{ x: number; y: number } | null>(null)
+  const mapMovingRef = useRef(false)

  const handleMarkerHover = useCallback((place: any, x: number, y: number) => {
-    if (hoverDisabled) return
+    if (hoverDisabled || mapMovingRef.current) return
    setHoveredPlace(place)
    setTooltipPos({ x, y })
  }, [hoverDisabled])
@@ -491,9 +509,18 @@ export const MapView = memo(function MapView({
  }, [hoveredPlace])

  const handleMarkerClick = useCallback((id: number) => {
+    // Clear the hover card right away: the recenter that follows moves the
+    // marker out from under the cursor, so no mouseout will ever fire (#1404).
+    setHoveredPlace(null)
+    setTooltipPos(null)
    onMarkerClick?.(id)
  }, [onMarkerClick])

+  const clearHover = useCallback(() => {
+    setHoveredPlace(null)
+    setTooltipPos(null)
+  }, [])
+
  // photoUrls: only base64 thumbs for smooth map zoom
  const [photoUrls, setPhotoUrls] = useState<Record<string, string>>(getAllThumbs)
  const placesPhotosEnabled = useAuthStore(s => s.placesPhotosEnabled)
@@ -644,6 +671,7 @@ export const MapView = memo(function MapView({
      <SelectionController places={places} selectedPlaceId={selectedPlaceId} dayPlaces={dayPlaces} paddingOpts={paddingOpts} />
      <MapClickHandler onClick={onMapClick} />
      <MapContextMenuHandler onContextMenu={onMapContextMenu} />
+      <CameraHoverGuard movingRef={mapMovingRef} onMoveStart={clearHover} />
      <ViewportController onViewportChange={onViewportChange} />
      <LeafletLocationLayer position={userPosition} mode={trackingMode} />

@@ -6,7 +6,10 @@ import { resetAllStores } from '../../../tests/helpers/store'
 import { buildPlace } from '../../../tests/helpers/factories'
 import { useSettingsStore } from '../../store/settingsStore'

-// Stable fake map so fitBounds call counts survive re-renders.
+// Stable fake map so fitBounds call counts survive re-renders. The canvas
+// container is a single element so listeners registered by the component are
+// reachable from tests via dispatchEvent.
+const glCanvasContainer = vi.hoisted(() => document.createElement('div'))
 const glMap = vi.hoisted(() => ({
  on: vi.fn(),
  off: vi.fn(),
@@ -25,10 +28,12 @@ const glMap = vi.hoisted(() => ({
  setLayoutProperty: vi.fn(),
  getStyle: vi.fn().mockReturnValue({ layers: [] }),
  isStyleLoaded: vi.fn().mockReturnValue(true),
-  getCanvasContainer: vi.fn(() => document.createElement('div')),
+  getCanvasContainer: vi.fn(() => glCanvasContainer),
  getLayer: vi.fn().mockReturnValue(null),
  queryRenderedFeatures: vi.fn().mockReturnValue([]),
  querySourceFeatures: vi.fn().mockReturnValue([]),
+  unproject: vi.fn(() => ({ lng: 2.3522, lat: 48.8566 })),
+  getBounds: vi.fn(() => ({ getSouth: () => 0, getWest: () => 0, getNorth: () => 1, getEast: () => 1 })),
  easeTo: vi.fn(),
 }))

@@ -265,4 +270,127 @@ describe('MapViewGL', () => {
    expect(glMap.addLayer).toHaveBeenCalledWith(expect.objectContaining({ id: 'trip-place-clusters-circle' }))
    expect(glMap.addLayer).toHaveBeenCalledWith(expect.objectContaining({ id: 'trip-place-clusters-count' }))
  })
+
+  function touchEvent(type: string, touches: Array<{ clientX: number; clientY: number }>) {
+    const ev = new Event(type, { bubbles: true })
+    Object.defineProperty(ev, 'touches', { value: touches })
+    return ev
+  }
+
+  it('FE-COMP-MAPVIEWGL-006: touch long-press opens Add-Place at the held position (#1398)', async () => {
+    vi.useFakeTimers()
+    try {
+      const onContext = vi.fn()
+      render(<MapViewGL places={[]} fitKey={1} onMapContextMenu={onContext} />)
+      await act(async () => {})
+      act(() => {
+        glCanvasContainer.dispatchEvent(touchEvent('touchstart', [{ clientX: 30, clientY: 40 }]))
+        vi.advanceTimersByTime(650)
+      })
+      expect(onContext).toHaveBeenCalledTimes(1)
+      expect(onContext.mock.calls[0][0].latlng).toEqual({ lat: 48.8566, lng: 2.3522 })
+    } finally {
+      vi.useRealTimers()
+    }
+  })
+
+  it('FE-COMP-MAPVIEWGL-007: a moving finger (pan) cancels the long-press (#1398)', async () => {
+    vi.useFakeTimers()
+    try {
+      const onContext = vi.fn()
+      render(<MapViewGL places={[]} fitKey={1} onMapContextMenu={onContext} />)
+      await act(async () => {})
+      act(() => {
+        glCanvasContainer.dispatchEvent(touchEvent('touchstart', [{ clientX: 30, clientY: 40 }]))
+        glCanvasContainer.dispatchEvent(touchEvent('touchmove', [{ clientX: 60, clientY: 90 }]))
+        vi.advanceTimersByTime(650)
+      })
+      expect(onContext).not.toHaveBeenCalled()
+    } finally {
+      vi.useRealTimers()
+    }
+  })
+
+  it('FE-COMP-MAPVIEWGL-008: a second finger (pinch) cancels the long-press (#1398)', async () => {
+    vi.useFakeTimers()
+    try {
+      const onContext = vi.fn()
+      render(<MapViewGL places={[]} fitKey={1} onMapContextMenu={onContext} />)
+      await act(async () => {})
+      act(() => {
+        glCanvasContainer.dispatchEvent(touchEvent('touchstart', [{ clientX: 30, clientY: 40 }]))
+        glCanvasContainer.dispatchEvent(touchEvent('touchstart', [{ clientX: 30, clientY: 40 }, { clientX: 80, clientY: 40 }]))
+        vi.advanceTimersByTime(650)
+      })
+      expect(onContext).not.toHaveBeenCalled()
+    } finally {
+      vi.useRealTimers()
+    }
+  })
+
+  it('FE-COMP-MAPVIEWGL-009: a plain right-click (map contextmenu event) opens Add-Place, deduped (#1398)', async () => {
+    const onContext = vi.fn()
+    render(<MapViewGL places={[]} fitKey={1} onMapContextMenu={onContext} />)
+    await act(async () => {})
+    const handler = glMap.on.mock.calls.find(c => c[0] === 'contextmenu')?.[1] as (e: unknown) => void
+    expect(handler).toBeTypeOf('function')
+    act(() => {
+      handler({ lngLat: { lat: 48.8566, lng: 2.3522 }, originalEvent: new MouseEvent('contextmenu') })
+      // Android long-press fires the native contextmenu on top of our timer —
+      // a second event inside the dedupe window must not open a second form.
+      handler({ lngLat: { lat: 48.8566, lng: 2.3522 }, originalEvent: new MouseEvent('contextmenu') })
+    })
+    expect(onContext).toHaveBeenCalledTimes(1)
+    expect(onContext.mock.calls[0][0].latlng).toEqual({ lat: 48.8566, lng: 2.3522 })
+  })
+
+  it('FE-COMP-MAPVIEWGL-010: middle-click still opens Add-Place (#1398 regression guard)', async () => {
+    const onContext = vi.fn()
+    render(<MapViewGL places={[]} fitKey={1} onMapContextMenu={onContext} />)
+    await act(async () => {})
+    act(() => {
+      glCanvasContainer.dispatchEvent(new MouseEvent('mousedown', { button: 1, bubbles: true }))
+    })
+    expect(onContext).toHaveBeenCalledTimes(1)
+  })
+
+  it('FE-COMP-MAPVIEWGL-011: clicking a marker clears the hover card; movestart clears + suppresses it (#1404)', async () => {
+    // Markers are only reconciled once the style has loaded — fire 'load' like GL-005 does.
+    glMap.on.mockImplementation((event: string, handlerOrLayer: unknown) => {
+      if (event === 'load' && typeof handlerOrLayer === 'function') (handlerOrLayer as () => void)()
+      return glMap
+    })
+    const mapboxgl = (await import('mapbox-gl')).default
+    const places = [buildMapPlace({ id: 7, lat: 48.8584, lng: 2.2945, name: 'Tour Eiffel' })]
+    const { queryByTestId } = render(<MapViewGL places={places} fitKey={1} onMarkerClick={vi.fn()} />)
+    await act(async () => {})
+
+    const markerCall = (mapboxgl.Marker as unknown as ReturnType<typeof vi.fn>).mock.calls
+      .find(c => c[0]?.element)
+    expect(markerCall).toBeTruthy()
+    const el = markerCall![0].element as HTMLElement
+
+    // hover shows the card
+    act(() => { el.dispatchEvent(new MouseEvent('mouseenter', { clientX: 10, clientY: 10 })) })
+    expect(queryByTestId('tooltip')).toBeTruthy()
+
+    // click clears it (the flyTo that follows moves the marker away, no mouseleave will come)
+    act(() => { el.dispatchEvent(new MouseEvent('click', { bubbles: false })) })
+    expect(queryByTestId('tooltip')).toBeNull()
+
+    // hover again, then camera movement clears + suppresses
+    act(() => { el.dispatchEvent(new MouseEvent('mouseenter', { clientX: 10, clientY: 10 })) })
+    expect(queryByTestId('tooltip')).toBeTruthy()
+    const moveStart = glMap.on.mock.calls.find(c => c[0] === 'movestart')?.[1] as () => void
+    const moveEnds = glMap.on.mock.calls.filter(c => c[0] === 'moveend').map(c => c[1] as () => void)
+    act(() => { moveStart() })
+    expect(queryByTestId('tooltip')).toBeNull()
+    // while the camera is moving, a re-fired mouseenter must not bring it back
+    act(() => { el.dispatchEvent(new MouseEvent('mouseenter', { clientX: 10, clientY: 10 })) })
+    expect(queryByTestId('tooltip')).toBeNull()
+    // after the move ends, hover works again
+    act(() => { moveEnds.forEach(fn => fn()) })
+    act(() => { el.dispatchEvent(new MouseEvent('mouseenter', { clientX: 10, clientY: 10 })) })
+    expect(queryByTestId('tooltip')).toBeTruthy()
+  })
 })
@@ -70,7 +70,7 @@ interface Props {
  onMarkerClick?: (id: number) => void
  hoverDisabled?: boolean
  onMapClick?: (info: { latlng: { lat: number; lng: number } }) => void
-  onMapContextMenu?: ((e: { latlng: { lat: number; lng: number }; originalEvent: MouseEvent }) => void) | null
+  onMapContextMenu?: ((e: { latlng: { lat: number; lng: number }; originalEvent: MouseEvent | TouchEvent }) => void) | null
  center?: [number, number]
  zoom?: number
  fitKey?: number | null
@@ -228,6 +228,10 @@ export function MapViewGL({
  const [hoverPlace, setHoverPlace] = useState<(Place & { category_color?: string | null; category_icon?: string | null; category_name?: string | null }) | null>(null)
  const [hoverPos, setHoverPos] = useState<{ x: number; y: number } | null>(null)
  const hoverIdRef = useRef<number | null>(null)
+  // True while the camera is moving (flyTo after a click, pan, zoom). Marker
+  // elements get rebuilt during the move and re-fire mouseenter under a
+  // stationary cursor, which would re-show the card we just cleared (#1404).
+  const camMovingRef = useRef(false)

  // Selecting a place rebuilds its marker element, so the browser never fires
  // mouseleave on the removed node and the fixed-position hover card gets
@@ -448,7 +452,13 @@ export function MapViewGL({
      setMapReady(true)
    })

+    // Set by the long-press handler below: the touchend tap that follows a
+    // long-press must not count as a normal map click (#1398).
+    let suppressNextClick = false
    map.on('click', (e) => {
+      // The tap that ends a long-press would otherwise land here and clear
+      // the selection right after the Add-Place form opened (#1398).
+      if (suppressNextClick) { suppressNextClick = false; return }
      const t = e.originalEvent.target as HTMLElement
      if (t.closest('.mapboxgl-marker, .maplibregl-marker')) return // markers handle their own click
      // A click that lands on a cluster bubble is the cluster's to handle
@@ -469,19 +479,41 @@ export function MapViewGL({
    }
    map.on('moveend', emitViewport)
    map.once('idle', emitViewport)
-    // In the GL map the right mouse button is reserved for the
-    // built-in rotate/pitch gesture, so we bind the "add place" action
-    // to the middle mouse button (button === 1) instead.
+    // Clear the hover card (and the anchored POI popup) as soon as the camera
+    // starts moving, and keep hover suppressed until it stops: the marker
+    // slides away under a stationary cursor, so mouseleave never fires (#1404).
+    const onCamStart = () => {
+      camMovingRef.current = true
+      hoverIdRef.current = null
+      setHoverPlace(null)
+      setHoverPos(null)
+      popupRef.current?.remove()
+    }
+    const onCamEnd = () => { camMovingRef.current = false }
+    map.on('movestart', onCamStart)
+    map.on('moveend', onCamEnd)
+    // "Add place here" on the GL map (#1398). Three routes into one handler:
+    // middle-click (the original binding), a plain right-click via the map's
+    // own contextmenu event — both GL libs suppress that event while the
+    // right-button rotate/pitch drag is active, so it can't fight the gesture,
+    // and it also covers Mac ctrl-click / two-finger tap — and a touch
+    // long-press, which neither GL lib synthesizes into contextmenu (Leaflet
+    // does, which is why the OSM map already worked on mobile).
    const canvas = map.getCanvasContainer()
+    let lastContextFire = 0
+    const fireContext = (lngLat: { lat: number; lng: number }, originalEvent: MouseEvent | TouchEvent) => {
+      // Android fires a native contextmenu for a long-press on top of our own
+      // timer — dedupe so the form doesn't open twice.
+      if (Date.now() - lastContextFire < 700) return
+      lastContextFire = Date.now()
+      onClickRefs.current.context?.({ latlng: { lat: lngLat.lat, lng: lngLat.lng }, originalEvent })
+    }
    const onAuxDown = (ev: MouseEvent) => {
      if (ev.button !== 1) return
      ev.preventDefault()
      const rect = canvas.getBoundingClientRect()
      const lngLat = map.unproject([ev.clientX - rect.left, ev.clientY - rect.top])
-      onClickRefs.current.context?.({
-        latlng: { lat: lngLat.lat, lng: lngLat.lng },
-        originalEvent: ev,
-      })
+      fireContext({ lat: lngLat.lat, lng: lngLat.lng }, ev)
    }
    // Also suppress the browser's native auxclick menu on middle-click.
    const onAuxClick = (ev: MouseEvent) => {
@@ -489,6 +521,41 @@ export function MapViewGL({
    }
    canvas.addEventListener('mousedown', onAuxDown)
    canvas.addEventListener('auxclick', onAuxClick)
+    map.on('contextmenu', (e: { lngLat: { lat: number; lng: number }; originalEvent: MouseEvent }) => {
+      fireContext(e.lngLat, e.originalEvent)
+    })
+    // Touch long-press: 600 ms hold (Leaflet's tapHold feel) with a 10 px
+    // move tolerance so slow pans and pinches don't open the form.
+    let lpTimer: number | null = null
+    let lpStart: { x: number; y: number } | null = null
+    const cancelLongPress = () => {
+      if (lpTimer !== null) window.clearTimeout(lpTimer)
+      lpTimer = null
+      lpStart = null
+    }
+    const onTouchStart = (ev: TouchEvent) => {
+      if (ev.touches.length !== 1) { cancelLongPress(); return }
+      if ((ev.target as HTMLElement).closest('.mapboxgl-marker, .maplibregl-marker')) return
+      const t = ev.touches[0]
+      lpStart = { x: t.clientX, y: t.clientY }
+      lpTimer = window.setTimeout(() => {
+        lpTimer = null
+        if (!lpStart) return
+        const rect = canvas.getBoundingClientRect()
+        const lngLat = map.unproject([lpStart.x - rect.left, lpStart.y - rect.top])
+        lpStart = null
+        suppressNextClick = true
+        fireContext({ lat: lngLat.lat, lng: lngLat.lng }, ev)
+      }, 600)
+    }
+    const onTouchMove = (ev: TouchEvent) => {
+      const t = ev.touches[0]
+      if (lpStart && (!t || Math.hypot(t.clientX - lpStart.x, t.clientY - lpStart.y) > 10)) cancelLongPress()
+    }
+    canvas.addEventListener('touchstart', onTouchStart, { passive: true })
+    canvas.addEventListener('touchmove', onTouchMove, { passive: true })
+    canvas.addEventListener('touchend', cancelLongPress)
+    canvas.addEventListener('touchcancel', cancelLongPress)

    // Drop follow mode if the user pans the map manually — matches the
    // Apple Maps behaviour where the blue dot stays but the map no longer
@@ -537,6 +604,11 @@ export function MapViewGL({
    return () => {
      canvas.removeEventListener('mousedown', onAuxDown)
      canvas.removeEventListener('auxclick', onAuxClick)
+      canvas.removeEventListener('touchstart', onTouchStart)
+      canvas.removeEventListener('touchmove', onTouchMove)
+      canvas.removeEventListener('touchend', cancelLongPress)
+      canvas.removeEventListener('touchcancel', cancelLongPress)
+      cancelLongPress()
      markersRef.current.forEach(m => m.remove())
      markersRef.current.clear()
      if (popupRef.current) { popupRef.current.remove(); popupRef.current = null }
@@ -652,16 +724,21 @@ export function MapViewGL({
        const el = createMarkerElement(place as Place & { category_color?: string; category_icon?: string }, photoUrl, orderNumbers, selected)
        el.addEventListener('click', (ev) => {
          ev.stopPropagation()
+          // Clear the card right away — the flyTo that follows moves the marker
+          // out from under the cursor and mouseleave never fires (#1404).
+          hoverIdRef.current = null
+          setHoverPlace(null)
+          setHoverPos(null)
          onClickRefs.current.marker?.(place.id)
        })
        el.addEventListener('mouseenter', (ev) => {
-          if (hoverDisabledRef.current) return
+          if (hoverDisabledRef.current || camMovingRef.current) return
          hoverIdRef.current = place.id
          setHoverPlace(place as Place & { category_color?: string; category_icon?: string; category_name?: string })
          setHoverPos({ x: (ev as MouseEvent).clientX, y: (ev as MouseEvent).clientY })
        })
        el.addEventListener('mousemove', (ev) => {
-          if (hoverDisabledRef.current) return
+          if (hoverDisabledRef.current || camMovingRef.current) return
          setHoverPos({ x: (ev as MouseEvent).clientX, y: (ev as MouseEvent).clientY })
        })
        el.addEventListener('mouseleave', () => {