mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
refactor(map): share the marker thumbnail loading between the Leaflet and GL maps
MapView and MapViewGL each held the same photo loading effect: read the cached thumbs, fetch the missing ones, listen for their arrival and batch them into one animation frame. It now lives in useMarkerThumbs, which also owns the thumb state and the place photos switch, and both maps call it where the effect used to be. The effect keeps its keys (the picture sources and the switch). It reads the places through useEffectEvent, which drops the old exhaustive-deps suppression in MapViewGL and the warning in MapView.
This commit is contained in:
Maurice
@@ -603,10 +603,8 @@ function MapContextMenuHandler({ onContextMenu }: { onContextMenu: ((e: L.Leafle
|
||||
|
||||
// Travel times are shown in the day sidebar (per-segment connectors), not on the map.
|
||||
|
||||
// Module-level photo cache shared with PlaceAvatar
|
||||
import { getCached, isLoading, fetchPhoto, onThumbReady, getAllThumbs } from '../../services/photoService'
|
||||
import { isCustomPlaceImage, markerPhotoHtml, photoCacheKey, photoSourcesKey, placePhotoFull, placePhotoUrl } from './placePhoto'
|
||||
import { useAuthStore } from '../../store/authStore'
|
||||
import { useMarkerThumbs } from './useMarkerThumbs'
|
||||
import { markerPhotoHtml, placePhotoFull, placePhotoUrl } from './placePhoto'
|
||||
import { useGeolocation } from '../../hooks/useGeolocation'
|
||||
import LocationButton from './LocationButton'
|
||||
import { useIsPhone } from '../../mobile/useIsPhone'
|
||||
@@ -971,72 +969,10 @@ export const MapView = memo(function MapView({
|
||||
setTooltipPos(null)
|
||||
}, [])
|
||||
|
||||
// photoUrls: only base64 thumbs for smooth map zoom
|
||||
const [photoUrls, setPhotoUrls] = useState<Record<string, string>>(getAllThumbs)
|
||||
const placesPhotosEnabled = useAuthStore(s => s.placesPhotosEnabled)
|
||||
// Batch photo state updates through a RAF so N simultaneous photo loads
|
||||
// collapse into a single re-render instead of N separate renders.
|
||||
const pendingThumbsRef = useRef<Record<string, string>>({})
|
||||
const thumbRafRef = useRef<number | null>(null)
|
||||
|
||||
const photoSources = useMemo(() => photoSourcesKey(places), [places])
|
||||
// Flattened [lat,lng] points of the selected day's route, so the bounds fit can
|
||||
// include the full polyline once it has been computed.
|
||||
const routeCoords = useMemo<[number, number][]>(() => (route || []).flat() as [number, number][], [route])
|
||||
useEffect(() => {
|
||||
if (!places || places.length === 0 || !placesPhotosEnabled) return
|
||||
const cleanups: (() => void)[] = []
|
||||
|
||||
const setThumb = (cacheKey: string, thumb: string) => {
|
||||
pendingThumbsRef.current[cacheKey] = thumb
|
||||
if (thumbRafRef.current !== null) return
|
||||
thumbRafRef.current = requestAnimationFrame(() => {
|
||||
thumbRafRef.current = null
|
||||
const pending = pendingThumbsRef.current
|
||||
pendingThumbsRef.current = {}
|
||||
setPhotoUrls(prev => {
|
||||
const hasChange = Object.entries(pending).some(([k, v]) => prev[k] !== v)
|
||||
return hasChange ? { ...prev, ...pending } : prev
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
for (const place of places) {
|
||||
// A custom uploaded image is shown directly — never auto-fetch a provider
|
||||
// photo for it (the request would 404 for OSM-only places and the fetched
|
||||
// thumb would shadow the user's own image). (#1136)
|
||||
if (isCustomPlaceImage(place.image_url)) continue
|
||||
const cacheKey = photoCacheKey(place)
|
||||
if (!cacheKey) continue
|
||||
|
||||
const cached = getCached(cacheKey)
|
||||
if (cached?.thumbDataUrl) {
|
||||
setThumb(cacheKey, cached.thumbDataUrl)
|
||||
continue
|
||||
}
|
||||
|
||||
cleanups.push(onThumbReady(cacheKey, thumb => setThumb(cacheKey, thumb)))
|
||||
|
||||
if (!cached && !isLoading(cacheKey)) {
|
||||
const photoId =
|
||||
(place.image_url?.startsWith('/api/maps/place-photo/') ? place.image_url : null)
|
||||
|| place.google_place_id
|
||||
|| place.osm_id
|
||||
|| place.image_url
|
||||
if (photoId || (place.lat && place.lng)) {
|
||||
fetchPhoto(cacheKey, photoId || `coords:${place.lat}:${place.lng}`, place.lat, place.lng, place.name)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return () => {
|
||||
cleanups.forEach(fn => fn())
|
||||
if (thumbRafRef.current !== null) {
|
||||
cancelAnimationFrame(thumbRafRef.current)
|
||||
thumbRafRef.current = null
|
||||
}
|
||||
}
|
||||
}, [photoSources, placesPhotosEnabled])
|
||||
const photoUrls = useMarkerThumbs(places)
|
||||
|
||||
const isTouchDevice = typeof window !== 'undefined' && navigator.maxTouchPoints > 0
|
||||
// Drag a marker onto a day (#891). Pointer-driven, so it is off wherever
|
||||
|
||||
@@ -12,10 +12,9 @@ import { useTranslation } from '../../i18n/TranslationContext'
|
||||
import { MapLayerSwitcher, TourMapLayerSwitcher, MAP_LAYER_SWITCHER_INSET, type BaseLayer, type TourBaseLayer } from './MapLayerSwitcher'
|
||||
import { OPENTOPOMAP_TILE_URL, OPENTOPOMAP_TILE_ATTRIBUTION, OPENTOPOMAP_TILE_MAXZOOM } from '../../constants/mapDefaults'
|
||||
import { MapLockPill } from './MapLockPill'
|
||||
import { useAuthStore } from '../../store/authStore'
|
||||
import { getCached, isLoading, fetchPhoto, onThumbReady, getAllThumbs } from '../../services/photoService'
|
||||
import { useMarkerThumbs } from './useMarkerThumbs'
|
||||
import { placeMarkerLook, type PlaceMarkerFlags } from './markerLook'
|
||||
import { isCustomPlaceImage, markerPhotoHtml, photoCacheKey, photoSourcesKey, placePhotoFull, placePhotoUrl } from './placePhoto'
|
||||
import { markerPhotoHtml, placePhotoFull, placePhotoUrl } from './placePhoto'
|
||||
import { CATEGORY_ICON_MAP } from '../shared/categoryIcons'
|
||||
import { isStandardFamily, supportsCustom3d, wantsTerrain, addCustom3dBuildings, addTerrainAndSky } from './mapboxSetup'
|
||||
import { attachLocationMarker, type LocationMarkerHandle } from './locationMarkerMapbox'
|
||||
@@ -779,8 +778,6 @@ export function MapViewGL({
|
||||
const isMapLibre = glProvider === 'maplibre-gl'
|
||||
const glStyle = styleForActiveProvider(glProvider, rawMapboxStyle, rawMaplibreStyle)
|
||||
const enableMapbox3d = !isMapLibre && mapbox3d
|
||||
const placesPhotosEnabled = useAuthStore(s => s.placesPhotosEnabled)
|
||||
const [photoUrls, setPhotoUrls] = useState<Record<string, string>>(getAllThumbs)
|
||||
const [mapReady, setMapReady] = useState(false)
|
||||
// Hover tooltip — a cursor-following name/category/address card, matching the
|
||||
// Leaflet map's overlay exactly (no anchored popup, no photo thumbnail).
|
||||
@@ -1869,62 +1866,8 @@ export function MapViewGL({
|
||||
try { map.setConfigProperty('basemap', 'language', basemapLanguage(mapLang)) } catch { /* style/SDK may not support the basemap language property */ }
|
||||
}, [mapLang, mapReady, isMapLibre, glStyle])
|
||||
|
||||
// Photo loading — mirrors the Leaflet MapView. Updates via RAF to batch
|
||||
// simultaneous thumb arrivals into one re-render.
|
||||
const pendingThumbsRef = useRef<Record<string, string>>({})
|
||||
const thumbRafRef = useRef<number | null>(null)
|
||||
const photoSources = useMemo(() => photoSourcesKey(places), [places])
|
||||
useEffect(() => {
|
||||
if (!places || places.length === 0 || !placesPhotosEnabled) return
|
||||
const cleanups: (() => void)[] = []
|
||||
|
||||
const setThumb = (cacheKey: string, thumb: string) => {
|
||||
pendingThumbsRef.current[cacheKey] = thumb
|
||||
if (thumbRafRef.current !== null) return
|
||||
thumbRafRef.current = requestAnimationFrame(() => {
|
||||
thumbRafRef.current = null
|
||||
const pending = pendingThumbsRef.current
|
||||
pendingThumbsRef.current = {}
|
||||
setPhotoUrls(prev => {
|
||||
const hasChange = Object.entries(pending).some(([k, v]) => prev[k] !== v)
|
||||
return hasChange ? { ...prev, ...pending } : prev
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
for (const place of places) {
|
||||
// A custom uploaded image is shown directly — never auto-fetch a provider
|
||||
// photo for it (that request would 404 for OSM-only places and, worse, the
|
||||
// fetched thumb would shadow the user's own image). (#1136)
|
||||
if (isCustomPlaceImage(place.image_url)) continue
|
||||
const cacheKey = photoCacheKey(place)
|
||||
if (!cacheKey) continue
|
||||
const cached = getCached(cacheKey)
|
||||
if (cached?.thumbDataUrl) {
|
||||
setThumb(cacheKey, cached.thumbDataUrl)
|
||||
continue
|
||||
}
|
||||
cleanups.push(onThumbReady(cacheKey, thumb => setThumb(cacheKey, thumb)))
|
||||
if (!cached && !isLoading(cacheKey)) {
|
||||
const photoId =
|
||||
(place.image_url?.startsWith('/api/maps/place-photo/') ? place.image_url : null)
|
||||
|| place.google_place_id
|
||||
|| place.osm_id
|
||||
|| place.image_url
|
||||
if (photoId || (place.lat && place.lng)) {
|
||||
fetchPhoto(cacheKey, photoId || `coords:${place.lat}:${place.lng}`, place.lat, place.lng, place.name)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return () => {
|
||||
cleanups.forEach(fn => fn())
|
||||
if (thumbRafRef.current !== null) {
|
||||
cancelAnimationFrame(thumbRafRef.current)
|
||||
thumbRafRef.current = null
|
||||
}
|
||||
}
|
||||
}, [photoSources, placesPhotosEnabled]) // eslint-disable-line react-hooks/exhaustive-deps
|
||||
// Photo loading, shared with the Leaflet MapView.
|
||||
const photoUrls = useMarkerThumbs(places)
|
||||
|
||||
// Reconcile markers with places + photos. The clustered GeoJSON source decides
|
||||
// which points are currently unclustered, and we render the existing rich HTML
|
||||
|
||||
@@ -0,0 +1,134 @@
|
||||
import { act, renderHook } from '@testing-library/react';
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import type { Place } from '../../types';
|
||||
|
||||
const thumbCallbacks = new Map<string, (thumb: string) => void>();
|
||||
|
||||
vi.mock('../../services/photoService', () => ({
|
||||
getCached: vi.fn(() => undefined),
|
||||
isLoading: vi.fn(() => false),
|
||||
fetchPhoto: vi.fn(),
|
||||
onThumbReady: vi.fn((key: string, cb: (thumb: string) => void) => {
|
||||
thumbCallbacks.set(key, cb);
|
||||
return () => {
|
||||
thumbCallbacks.delete(key);
|
||||
};
|
||||
}),
|
||||
getAllThumbs: vi.fn(() => ({ seeded: 'data:seed' })),
|
||||
}));
|
||||
|
||||
import * as photoService from '../../services/photoService';
|
||||
import { useAuthStore } from '../../store/authStore';
|
||||
import { useMarkerThumbs } from './useMarkerThumbs';
|
||||
|
||||
function place(over: Partial<Place>): Place {
|
||||
return { id: 1, name: 'P', lat: 48, lng: 2, image_url: null, google_place_id: null, osm_id: null, ...over } as Place;
|
||||
}
|
||||
|
||||
let frames: FrameRequestCallback[] = [];
|
||||
|
||||
beforeEach(() => {
|
||||
frames = [];
|
||||
thumbCallbacks.clear();
|
||||
vi.clearAllMocks();
|
||||
vi.mocked(photoService.getCached).mockReturnValue(undefined);
|
||||
vi.mocked(photoService.isLoading).mockReturnValue(false);
|
||||
vi.stubGlobal('requestAnimationFrame', (cb: FrameRequestCallback) => {
|
||||
frames.push(cb);
|
||||
return frames.length;
|
||||
});
|
||||
vi.stubGlobal('cancelAnimationFrame', vi.fn());
|
||||
useAuthStore.setState({ placesPhotosEnabled: true });
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.unstubAllGlobals();
|
||||
});
|
||||
|
||||
function flushFrames() {
|
||||
const pending = frames;
|
||||
frames = [];
|
||||
act(() => pending.forEach((cb) => cb(0)));
|
||||
}
|
||||
|
||||
describe('useMarkerThumbs', () => {
|
||||
it('starts from the thumbs already cached', () => {
|
||||
const { result } = renderHook(() => useMarkerThumbs([]));
|
||||
expect(result.current).toEqual({ seeded: 'data:seed' });
|
||||
});
|
||||
|
||||
it('fetches a missing photo by its provider id, else by its coordinates', () => {
|
||||
renderHook(() =>
|
||||
useMarkerThumbs([place({ id: 1, google_place_id: 'gp-1', name: 'Tower' }), place({ id: 2, lat: 50, lng: 3 })])
|
||||
);
|
||||
expect(photoService.fetchPhoto).toHaveBeenCalledWith('gp-1', 'gp-1', 48, 2, 'Tower');
|
||||
expect(photoService.fetchPhoto).toHaveBeenCalledWith('50,3', 'coords:50:3', 50, 3, 'P');
|
||||
});
|
||||
|
||||
it('prefers the picked provider photo over the place id', () => {
|
||||
const url = '/api/maps/place-photo/abc';
|
||||
renderHook(() => useMarkerThumbs([place({ image_url: url, google_place_id: 'gp-1' })]));
|
||||
expect(photoService.fetchPhoto).toHaveBeenCalledWith(url, url, 48, 2, 'P');
|
||||
});
|
||||
|
||||
it('never fetches for a custom image, a place without identity or one already loading', () => {
|
||||
vi.mocked(photoService.isLoading).mockImplementation((key) => key === 'busy');
|
||||
renderHook(() =>
|
||||
useMarkerThumbs([
|
||||
place({ id: 1, image_url: '/uploads/places/own.jpg' }),
|
||||
place({ id: 2, lat: null as never, lng: null as never }),
|
||||
place({ id: 3, google_place_id: 'busy' }),
|
||||
])
|
||||
);
|
||||
expect(photoService.fetchPhoto).not.toHaveBeenCalled();
|
||||
expect(photoService.onThumbReady).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('does nothing while place photos are switched off', () => {
|
||||
useAuthStore.setState({ placesPhotosEnabled: false });
|
||||
renderHook(() => useMarkerThumbs([place({ google_place_id: 'gp-1' })]));
|
||||
expect(photoService.fetchPhoto).not.toHaveBeenCalled();
|
||||
expect(photoService.onThumbReady).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('batches thumbs arriving together into one frame', () => {
|
||||
vi.mocked(photoService.getCached).mockImplementation((key) =>
|
||||
key === 'gp-1' ? ({ thumbDataUrl: 'data:one' } as never) : undefined
|
||||
);
|
||||
const { result } = renderHook(() =>
|
||||
useMarkerThumbs([place({ id: 1, google_place_id: 'gp-1' }), place({ id: 2, google_place_id: 'gp-2' })])
|
||||
);
|
||||
act(() => thumbCallbacks.get('gp-2')!('data:two'));
|
||||
expect(frames).toHaveLength(1);
|
||||
flushFrames();
|
||||
expect(result.current).toEqual({ seeded: 'data:seed', 'gp-1': 'data:one', 'gp-2': 'data:two' });
|
||||
});
|
||||
|
||||
it('keeps the same object when a frame brings nothing new', () => {
|
||||
vi.mocked(photoService.getCached).mockReturnValue({ thumbDataUrl: 'data:seed' } as never);
|
||||
const { result } = renderHook(() => useMarkerThumbs([place({ google_place_id: 'seeded' })]));
|
||||
const before = result.current;
|
||||
flushFrames();
|
||||
expect(result.current).toBe(before);
|
||||
});
|
||||
|
||||
it('restarts only when a picture source changes, and cleans up the old listeners', () => {
|
||||
const first = [place({ id: 1, google_place_id: 'gp-1' })];
|
||||
const { rerender } = renderHook(({ places }) => useMarkerThumbs(places), { initialProps: { places: first } });
|
||||
expect(photoService.onThumbReady).toHaveBeenCalledTimes(1);
|
||||
|
||||
rerender({ places: [place({ id: 1, google_place_id: 'gp-1' })] });
|
||||
expect(photoService.onThumbReady).toHaveBeenCalledTimes(1);
|
||||
|
||||
rerender({ places: [place({ id: 1, google_place_id: 'gp-9' })] });
|
||||
expect(photoService.onThumbReady).toHaveBeenCalledTimes(2);
|
||||
expect([...thumbCallbacks.keys()]).toEqual(['gp-9']);
|
||||
});
|
||||
|
||||
it('cancels a pending frame when it unmounts', () => {
|
||||
const { unmount } = renderHook(() => useMarkerThumbs([place({ google_place_id: 'gp-1' })]));
|
||||
act(() => thumbCallbacks.get('gp-1')!('data:one'));
|
||||
unmount();
|
||||
expect(cancelAnimationFrame).toHaveBeenCalledWith(1);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,82 @@
|
||||
import { useEffect, useEffectEvent, useMemo, useRef, useState } from 'react';
|
||||
import { fetchPhoto, getAllThumbs, getCached, isLoading, onThumbReady } from '../../services/photoService';
|
||||
import { useAuthStore } from '../../store/authStore';
|
||||
import type { Place } from '../../types';
|
||||
import { isCustomPlaceImage, photoCacheKey, photoSourcesKey } from './placePhoto';
|
||||
|
||||
/**
|
||||
* The small thumbnails the place markers wear, keyed by photoCacheKey, for both
|
||||
* the Leaflet and the GL map. Cached thumbs are taken as they are, missing ones
|
||||
* are fetched once, and thumbs that arrive together are batched through one
|
||||
* animation frame so N photo loads cost a single re-render instead of N.
|
||||
*
|
||||
* Loading restarts only when a place's picture source changes (photoSourcesKey),
|
||||
* not on every new places array, and never while place photos are switched off.
|
||||
*/
|
||||
export function useMarkerThumbs(places: Place[]): Record<string, string> {
|
||||
// Only base64 thumbs, so the markers stay smooth while the map zooms.
|
||||
const [photoUrls, setPhotoUrls] = useState<Record<string, string>>(getAllThumbs);
|
||||
const placesPhotosEnabled = useAuthStore((s) => s.placesPhotosEnabled);
|
||||
const pendingThumbsRef = useRef<Record<string, string>>({});
|
||||
const thumbRafRef = useRef<number | null>(null);
|
||||
const photoSources = useMemo(() => photoSourcesKey(places), [places]);
|
||||
|
||||
const startLoading = useEffectEvent((): (() => void) | undefined => {
|
||||
if (!places || places.length === 0 || !placesPhotosEnabled) return;
|
||||
const cleanups: (() => void)[] = [];
|
||||
|
||||
const setThumb = (cacheKey: string, thumb: string) => {
|
||||
pendingThumbsRef.current[cacheKey] = thumb;
|
||||
if (thumbRafRef.current !== null) return;
|
||||
thumbRafRef.current = requestAnimationFrame(() => {
|
||||
thumbRafRef.current = null;
|
||||
const pending = pendingThumbsRef.current;
|
||||
pendingThumbsRef.current = {};
|
||||
setPhotoUrls((prev) => {
|
||||
const hasChange = Object.entries(pending).some(([k, v]) => prev[k] !== v);
|
||||
return hasChange ? { ...prev, ...pending } : prev;
|
||||
});
|
||||
});
|
||||
};
|
||||
|
||||
for (const place of places) {
|
||||
// A custom uploaded image is shown directly: never auto-fetch a provider
|
||||
// photo for it (the request would 404 for OSM-only places and the fetched
|
||||
// thumb would shadow the user's own image). (#1136)
|
||||
if (isCustomPlaceImage(place.image_url)) continue;
|
||||
const cacheKey = photoCacheKey(place);
|
||||
if (!cacheKey) continue;
|
||||
|
||||
const cached = getCached(cacheKey);
|
||||
if (cached?.thumbDataUrl) {
|
||||
setThumb(cacheKey, cached.thumbDataUrl);
|
||||
continue;
|
||||
}
|
||||
|
||||
cleanups.push(onThumbReady(cacheKey, (thumb) => setThumb(cacheKey, thumb)));
|
||||
|
||||
if (!cached && !isLoading(cacheKey)) {
|
||||
const photoId =
|
||||
(place.image_url?.startsWith('/api/maps/place-photo/') ? place.image_url : null) ||
|
||||
place.google_place_id ||
|
||||
place.osm_id ||
|
||||
place.image_url;
|
||||
if (photoId || (place.lat && place.lng)) {
|
||||
fetchPhoto(cacheKey, photoId || `coords:${place.lat}:${place.lng}`, place.lat, place.lng, place.name);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return () => {
|
||||
cleanups.forEach((fn) => fn());
|
||||
if (thumbRafRef.current !== null) {
|
||||
cancelAnimationFrame(thumbRafRef.current);
|
||||
thumbRafRef.current = null;
|
||||
}
|
||||
};
|
||||
});
|
||||
|
||||
useEffect(() => startLoading(), [photoSources, placesPhotosEnabled]);
|
||||
|
||||
return photoUrls;
|
||||
}
|
||||
Reference in New Issue
Block a user