mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
fix(map): position place pins in the map's render cadence on MapLibre
The place pins visibly swam back and forth while the map was dragged and only settled onto their real position when it stopped. The clustered bubbles never did, and that is the clue: those are drawn inside the canvas, the pins are HTML. A library Marker repositions itself on the map's 'move' event, which fires once per pointer sample, while the canvas is redrawn once per animation frame. When the style is heavy or tiles are still streaming, the pins therefore run ahead of the map they are supposed to be pinned to. They now live in their own layer inside the canvas container and are positioned from the map's own 'render' event, so pin and map move on the same clock. Mapbox keeps the library marker: its terrain path needs the altitude handling that comes with it, and the drift was never reported there. Clicks, hover and dragging a pin onto a day are untouched — they hang off the element, which is the same element as before.
This commit is contained in:
@@ -42,6 +42,9 @@ const glMap = vi.hoisted(() => ({
|
||||
queryRenderedFeatures: vi.fn().mockReturnValue([]),
|
||||
querySourceFeatures: vi.fn().mockReturnValue([]),
|
||||
unproject: vi.fn(() => ({ lng: 2.3522, lat: 48.8566 })),
|
||||
// The MapLibre place pins position themselves with this, so a mock without it
|
||||
// never exercises the path the real map takes.
|
||||
project: vi.fn((lngLat: [number, number]) => ({ x: lngLat[0] * 10, y: lngLat[1] * 10 })),
|
||||
getBounds: vi.fn(() => ({ getSouth: () => 0, getWest: () => 0, getNorth: () => 1, getEast: () => 1 })),
|
||||
easeTo: vi.fn(),
|
||||
getCanvas: vi.fn(() => document.createElement('canvas')),
|
||||
@@ -411,6 +414,42 @@ describe('MapViewGL', () => {
|
||||
expect(glMap.addLayer).toHaveBeenCalledWith(expect.objectContaining({ id: 'trip-place-clusters-count' }))
|
||||
})
|
||||
|
||||
it('FE-COMP-MAPVIEWGL-070: MapLibre place pins are positioned from the map render, not from pointer moves', async () => {
|
||||
glMap.on.mockImplementation((event: string, handlerOrLayer: unknown) => {
|
||||
if (event === 'load' && typeof handlerOrLayer === 'function') (handlerOrLayer as () => void)()
|
||||
return glMap
|
||||
})
|
||||
useSettingsStore.setState({
|
||||
settings: {
|
||||
...useSettingsStore.getState().settings,
|
||||
map_provider: 'maplibre-gl',
|
||||
mapbox_access_token: '',
|
||||
maplibre_style: 'https://tiles.openfreemap.org/styles/liberty',
|
||||
},
|
||||
} as any)
|
||||
|
||||
render(<MapViewGL places={[buildMapPlace({ id: 1, lat: 3, lng: 2 })]} fitKey={1} glProvider="maplibre-gl" />)
|
||||
await act(async () => {})
|
||||
|
||||
// The pin sits in its own layer inside the canvas container, not in one of the
|
||||
// library's marker wrappers — those follow 'move', which fires per pointer sample.
|
||||
const layer = Array.from(glCanvasContainer.children).find(
|
||||
c => c instanceof HTMLElement && c.style.pointerEvents === 'none',
|
||||
) as HTMLElement | undefined
|
||||
expect(layer).toBeTruthy()
|
||||
expect(layer!.children).toHaveLength(1)
|
||||
const pin = layer!.children[0] as HTMLElement
|
||||
// project() is mocked as lng*10 / lat*10.
|
||||
expect(pin.style.transform).toContain('translate(20px, 30px)')
|
||||
|
||||
// Moving the camera and drawing a frame moves the pin with it.
|
||||
glMap.project.mockReturnValue({ x: 90, y: 70 })
|
||||
const render1 = glMap.on.mock.calls.find(c => c[0] === 'render')?.[1] as (() => void) | undefined
|
||||
expect(render1).toBeTypeOf('function')
|
||||
act(() => { render1!() })
|
||||
expect(pin.style.transform).toContain('translate(90px, 70px)')
|
||||
})
|
||||
|
||||
function touchEvent(type: string, touches: Array<{ clientX: number; clientY: number }>) {
|
||||
const ev = new Event(type, { bubbles: true })
|
||||
Object.defineProperty(ev, 'touches', { value: touches })
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { useEffect, useRef, useMemo, useState, createElement } from 'react'
|
||||
import { useEffect, useRef, useMemo, useState, createElement, useCallback } from 'react'
|
||||
import { makeMarkerDraggable } from './markerDrag'
|
||||
import { renderIconMarkup } from '../../utils/iconMarkup'
|
||||
import type mapboxgl from 'mapbox-gl'
|
||||
@@ -290,6 +290,72 @@ function formatViaDwellGl(seconds: number): string {
|
||||
return h > 0 ? `${h} h ${m} min` : `${m} min`
|
||||
}
|
||||
|
||||
/**
|
||||
* A place pin on the map, whatever is carrying it.
|
||||
*
|
||||
* Mapbox keeps the library's Marker (its terrain path needs the altitude handling that
|
||||
* comes with it); MapLibre gets a hand-positioned element, because a Marker repositions
|
||||
* itself on every `move` event — one per pointer sample — while the canvas only redraws
|
||||
* once per animation frame. Under a heavy style the pins then run ahead of the map and
|
||||
* visibly swim during a drag, which the clustered pins never do: those are drawn inside
|
||||
* the canvas. Positioning from the map's own `render` event puts both on one clock.
|
||||
*/
|
||||
/** What either path accepts for a position — the 3-tuple carries a terrain altitude. */
|
||||
type PinPosition = [number, number] | [number, number, number] | { lng: number; lat: number }
|
||||
|
||||
interface PlacePin {
|
||||
el: HTMLElement
|
||||
remove: () => void
|
||||
getLngLat: () => { lng: number; lat: number }
|
||||
setLngLat: (value: PinPosition) => void
|
||||
/** Re-reads the map and writes this pin's screen position. No-op for a library marker. */
|
||||
reposition: () => void
|
||||
}
|
||||
|
||||
/** Wraps the library's own marker so both paths present the same handle. */
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
function libraryPin(marker: any): PlacePin {
|
||||
return {
|
||||
el: marker.getElement(),
|
||||
remove: () => marker.remove(),
|
||||
getLngLat: () => marker.getLngLat(),
|
||||
setLngLat: (value) => marker.setLngLat(value),
|
||||
reposition: () => {},
|
||||
}
|
||||
}
|
||||
|
||||
/** A pin we place ourselves, in the map's render cadence. */
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
function makePlacePin(map: any, layer: HTMLElement, el: HTMLElement, lng: number, lat: number): PlacePin {
|
||||
let position = { lng, lat }
|
||||
el.style.position = 'absolute'
|
||||
el.style.top = '0'
|
||||
el.style.left = '0'
|
||||
el.style.pointerEvents = 'auto'
|
||||
el.style.willChange = 'transform'
|
||||
layer.appendChild(el)
|
||||
const reposition = (): void => {
|
||||
// The map is gone the moment the style rebuilds or the component unmounts, and a
|
||||
// queued render can still land after that.
|
||||
if (typeof map.project !== 'function') return
|
||||
const p = map.project([position.lng, position.lat])
|
||||
el.style.transform = `translate(-50%, -50%) translate(${p.x}px, ${p.y}px)`
|
||||
}
|
||||
reposition()
|
||||
return {
|
||||
el,
|
||||
remove: () => el.remove(),
|
||||
getLngLat: () => ({ ...position }),
|
||||
setLngLat: (value) => {
|
||||
position = Array.isArray(value)
|
||||
? { lng: value[0], lat: value[1] }
|
||||
: { lng: value.lng, lat: value.lat }
|
||||
reposition()
|
||||
},
|
||||
reposition,
|
||||
}
|
||||
}
|
||||
|
||||
// Tone dot for a plugin marker — visual twin of MapPluginMarkers' divIcon.
|
||||
function createPluginMarkerElement(tone: PluginMapMarker['tone']): HTMLDivElement {
|
||||
const color = PLUGIN_TONE_COLORS[tone] ?? PLUGIN_TONE_COLORS.default
|
||||
@@ -411,7 +477,12 @@ export function MapViewGL({
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
const mapRef = useRef<any | null>(null)
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
const markersRef = useRef<Map<number, any>>(new Map())
|
||||
const markersRef = useRef<Map<number, PlacePin>>(new Map())
|
||||
// Own layer for the hand-positioned place pins (MapLibre path, see makePlacePin).
|
||||
const pinLayerRef = useRef<HTMLDivElement | null>(null)
|
||||
const repositionPins = useCallback(() => {
|
||||
markersRef.current.forEach(pin => pin.reposition())
|
||||
}, [])
|
||||
const locationMarkerRef = useRef<LocationMarkerHandle | null>(null)
|
||||
const reservationOverlayRef = useRef<ReservationMapboxOverlay | null>(null)
|
||||
// Refs so the reservation overlay always sees the latest callback /
|
||||
@@ -737,6 +808,18 @@ export function MapViewGL({
|
||||
setMapReady(true)
|
||||
})
|
||||
|
||||
// Layer for the hand-positioned place pins, inside the canvas container so it
|
||||
// shares the canvas's stacking context — the same place the library puts its own
|
||||
// markers. `render` fires exactly when the map has drawn, so the pins land on the
|
||||
// frame the user is looking at instead of on the last pointer sample.
|
||||
if (isMapLibre) {
|
||||
const layer = document.createElement('div')
|
||||
layer.style.cssText = 'position:absolute;inset:0;overflow:hidden;pointer-events:none;'
|
||||
map.getCanvasContainer().appendChild(layer)
|
||||
pinLayerRef.current = layer
|
||||
map.on('render', repositionPins)
|
||||
}
|
||||
|
||||
// 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
|
||||
@@ -904,8 +987,8 @@ export function MapViewGL({
|
||||
const curAlt = (ll as any).alt ?? 0
|
||||
if (Math.abs(curAlt - alt) > 0.25) {
|
||||
// mapbox-gl accepts a third altitude element at runtime, but its typings
|
||||
// only model the 2-tuple form, so cast to LngLatLike.
|
||||
marker.setLngLat([ll.lng, ll.lat, alt] as unknown as mapboxgl.LngLatLike)
|
||||
// only model the 2-tuple form — PinPosition spells the runtime shape out.
|
||||
marker.setLngLat([ll.lng, ll.lat, alt])
|
||||
}
|
||||
})
|
||||
}
|
||||
@@ -921,8 +1004,11 @@ export function MapViewGL({
|
||||
canvas.removeEventListener('touchend', cancelLongPress)
|
||||
canvas.removeEventListener('touchcancel', cancelLongPress)
|
||||
cancelLongPress()
|
||||
map.off('render', repositionPins)
|
||||
markersRef.current.forEach(m => m.remove())
|
||||
markersRef.current.clear()
|
||||
pinLayerRef.current?.remove()
|
||||
pinLayerRef.current = null
|
||||
if (popupRef.current) { popupRef.current.remove(); popupRef.current = null }
|
||||
onMapReadyRef.current?.(null)
|
||||
if (reservationOverlayRef.current) {
|
||||
@@ -1082,10 +1168,12 @@ export function MapViewGL({
|
||||
// pitch. Tried `pitchAlignment: 'map'` to snap markers onto terrain,
|
||||
// but it rotates the element by the pitch angle and visually offsets
|
||||
// the anchor by ~100px at 45° tilt, which caused the observed drift.
|
||||
const m = new gl.Marker({ element: el, anchor: 'center' })
|
||||
const pin = isMapLibre && pinLayerRef.current
|
||||
? makePlacePin(map, pinLayerRef.current, el, place.lng, place.lat)
|
||||
: libraryPin(new gl.Marker({ element: el, anchor: 'center' })
|
||||
.setLngLat([place.lng, place.lat])
|
||||
.addTo(map)
|
||||
markersRef.current.set(place.id, m)
|
||||
.addTo(map))
|
||||
markersRef.current.set(place.id, pin)
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user