From a1180b2b19a194009c32b5b9adbcd10f43ceafa0 Mon Sep 17 00:00:00 2001 From: Maurice Date: Wed, 21 Oct 2026 19:08:25 +0200 Subject: [PATCH] fix(journey): open the map on the plan when nothing else is located, say when the recordings could not be read and open photo hits on the shared map --- client/src/components/Journey/JourneyMap.tsx | 43 +++++++++++++++-- .../src/components/Journey/JourneyMapGL.tsx | 48 ++++++++++++++++--- .../Journey/journeyMapLayers.test.ts | 19 +++++++- .../components/Journey/journeyMapLayers.ts | 13 +++-- .../components/Journey/useJourneyMapLayers.ts | 5 ++ .../Journey/useJourneyRecordings.test.ts | 16 ++++++- .../Journey/useJourneyRecordings.ts | 19 ++++++-- client/src/pages/JourneyPublicPage.tsx | 28 ++++++++++- .../pages/journeyDetail/useJourneyDetail.ts | 5 ++ 9 files changed, 175 insertions(+), 21 deletions(-) diff --git a/client/src/components/Journey/JourneyMap.tsx b/client/src/components/Journey/JourneyMap.tsx index 7bcd4667a..db8957191 100644 --- a/client/src/components/Journey/JourneyMap.tsx +++ b/client/src/components/Journey/JourneyMap.tsx @@ -183,6 +183,14 @@ const TRACK_FALLBACK_COLOR = '#4f46e5' /** The dash of a planned track; wide enough not to pass for the faint line between entries. */ const PLANNED_DASH = '8 7' +function markCoords(marks: JourneyMapMark[] | undefined): L.LatLngTuple[] { + return (marks ?? []).map(mark => [mark.lat, mark.lng] as L.LatLngTuple) +} + +function fitOptions(paddingBottom: number | undefined): L.FitBoundsOptions { + return { paddingTopLeft: [50, 50], paddingBottomRight: [50, paddingBottom || 50], maxZoom: 16 } +} + function JourneyMap( { entries, photos, onPhotoClick, trail, tracks, height = 220, dark, activeMarkerId, onMarkerClick, fullScreen, paddingBottom, cartoApiKey, hideMarkerTooltip, layerVisibility = EVERY_LAYER, marks, onMarksPick, plannedEntryIds = NO_PLANNED, ref }: Props, ) { @@ -222,6 +230,12 @@ function JourneyMap( onPhotoClickRef.current = onPhotoClick const markLayerRef = useRef(null) const onMarksPickRef = useRef(onMarksPick) + const marksRef = useRef(marks) + marksRef.current = marks + // A journey with nothing located of its own opens on the world until its + // plan or stays arrive; those get the first view unless the reader moved it. + const awaitingMarksRef = useRef(false) + const readerMovedRef = useRef(false) // What each switchable layer drew, and which of them are on the map right now. const drawnRef = useRef(NOTHING_DRAWN()) const shownRef = useRef(layerVisibility) @@ -303,6 +317,11 @@ function JourneyMap( L.control.attribution({ position: 'bottomright', prefix: false }).addTo(map) mapRef.current = map cancelledRef.current = false + awaitingMarksRef.current = false + readerMovedRef.current = false + const readerMoved = () => { readerMovedRef.current = true } + map.on('dragstart', readerMoved) + containerRef.current.addEventListener('wheel', readerMoved, { passive: true }) // The basemap is a vector style unless the user brought their own raster // template, so which layer draws it is decided per template rather than once. @@ -445,12 +464,12 @@ function JourneyMap( map.invalidateSize() // Tracks only get a say when the journey has nothing located of its own — // a world view would be worse than framing the one thing on the map. - const fitCoords = allCoords.length > 0 ? allCoords : trackCoords + const fitCoords = allCoords.length > 0 ? allCoords : trackCoords.length > 0 ? trackCoords : markCoords(marksRef.current) if (fitCoords.length > 0) { - const pb = paddingBottom || 50 - map.fitBounds(L.latLngBounds(fitCoords), { paddingTopLeft: [50, 50], paddingBottomRight: [50, pb], maxZoom: 16 }) + map.fitBounds(L.latLngBounds(fitCoords), fitOptions(paddingBottom)) } else { map.setView([30, 0], 2) + awaitingMarksRef.current = true } } catch {} }) @@ -459,8 +478,10 @@ function JourneyMap( if (mapRef.current) map.invalidateSize() }, 200) + const container = containerRef.current return () => { cancelledRef.current = true + container.removeEventListener('wheel', readerMoved) map.remove() mapRef.current = null tileLayerRef.current = null @@ -560,6 +581,12 @@ function JourneyMap( markLayerRef.current = drawLeafletMarks(map, marks, members => onMarksPickRef.current?.(members)) } + const coords = markCoords(marks) + if (awaitingMarksRef.current && coords.length > 0) { + awaitingMarksRef.current = false + if (!readerMovedRef.current) map.fitBounds(L.latLngBounds(coords), fitOptions(paddingBottom)) + } + draw() map.on('zoomend', draw) map.on('moveend', draw) @@ -584,8 +611,14 @@ function JourneyMap( return () => clearTimeout(timer) }, [activeMarkerId]) - const zoomIn = () => mapRef.current?.zoomIn() - const zoomOut = () => mapRef.current?.zoomOut() + const zoomIn = () => { + readerMovedRef.current = true + mapRef.current?.zoomIn() + } + const zoomOut = () => { + readerMovedRef.current = true + mapRef.current?.zoomOut() + } return (
diff --git a/client/src/components/Journey/JourneyMapGL.tsx b/client/src/components/Journey/JourneyMapGL.tsx index 7b1864775..5231e0372 100644 --- a/client/src/components/Journey/JourneyMapGL.tsx +++ b/client/src/components/Journey/JourneyMapGL.tsx @@ -148,6 +148,21 @@ const EMPTY_TRACKS: JourneyTrack[] = [] /** Fallback when a track carries no colour of its own, matching the planner's default. */ const TRACK_FALLBACK_COLOR = '#4f46e5' +function markBounds(gl: Props['gl'], marks: JourneyMapMark[]) { + const bounds = new gl.LngLatBounds() + marks.forEach(mark => bounds.extend([mark.lng, mark.lat])) + return bounds +} + +function fitOptions(paddingBottom: number | undefined, tilted: boolean) { + return { + padding: { top: 50, bottom: paddingBottom || 50, left: 50, right: 50 }, + maxZoom: 16, + pitch: tilted ? 45 : 0, + duration: 0, + } +} + function JourneyMapGL( { entries, trail, tracks, height = 220, dark, activeMarkerId, onMarkerClick, fullScreen, paddingBottom, glProvider = 'mapbox-gl', gl, hideMarkerTooltip, onMarkerPhotoClick, photos, onPhotoClick, layerVisibility = EVERY_LAYER, marks, onMarksPick, plannedEntryIds = NO_PLANNED, ref }: Props, ) { @@ -184,6 +199,12 @@ function JourneyMapGL( const photoMarkersRef = useRef<{ remove: () => void }[]>([]) const markMarkersRef = useRef<{ remove: () => void }[]>([]) const onMarksPickRef = useRef(onMarksPick) + const marksRef = useRef(marks) + marksRef.current = marks + // As on the Leaflet map: nothing located of its own, so the plan or the + // stays get the first view once they arrive, unless the reader moved it. + const awaitingMarksRef = useRef(false) + const readerMovedRef = useRef(false) const visibilityRef = useRef(layerVisibility) const shownRef = useRef(layerVisibility) const plannedRef = useRef(plannedEntryIds) @@ -343,6 +364,11 @@ function JourneyMapGL( const map = new gl.Map(mapOptions as any) mapRef.current = map + awaitingMarksRef.current = false + readerMovedRef.current = false + map.on('movestart', (ev: { originalEvent?: unknown }) => { + if (ev.originalEvent) readerMovedRef.current = true + }) shownRef.current = visibilityRef.current iconPlannedRef.current = plannedRef.current // A layer whose switch is off is added hidden, so switching it on later needs no rebuild. @@ -471,15 +497,15 @@ function JourneyMapGL( // fit bounds to all points if (hasPoints) { - const pb = paddingBottom || 50 try { - map.fitBounds(bounds, { - padding: { top: 50, bottom: pb, left: 50, right: 50 }, - maxZoom: 16, - pitch: enableMapbox3d && fullScreen ? 45 : 0, - duration: 0, - }) + map.fitBounds(bounds, fitOptions(paddingBottom, enableMapbox3d && fullScreen)) } catch { /* empty bounds */ } + } else if (marksRef.current?.length) { + try { + map.fitBounds(markBounds(gl, marksRef.current), fitOptions(paddingBottom, enableMapbox3d && fullScreen)) + } catch { /* empty bounds */ } + } else { + awaitingMarksRef.current = true } }) @@ -571,6 +597,14 @@ function JourneyMapGL( const draw = () => { clear() if (!marks?.length) return + if (awaitingMarksRef.current) { + awaitingMarksRef.current = false + if (!readerMovedRef.current) { + try { + map.fitBounds(markBounds(gl, marks), fitOptions(paddingBottom, enableMapbox3d && fullScreen)) + } catch { /* empty bounds */ } + } + } for (const hit of clusterPoints(marks, (lat, lng) => map.project([lng, lat]), MARK_CLUSTER_PX)) { const look = markHitSvg(hit.members) const el = document.createElement('div') diff --git a/client/src/components/Journey/journeyMapLayers.test.ts b/client/src/components/Journey/journeyMapLayers.test.ts index 356cd8478..4ccf08dd0 100644 --- a/client/src/components/Journey/journeyMapLayers.test.ts +++ b/client/src/components/Journey/journeyMapLayers.test.ts @@ -1,4 +1,4 @@ -// FE-JOURNEY-MAPLAYERS-001 to FE-JOURNEY-MAPLAYERS-008 +// FE-JOURNEY-MAPLAYERS-001 to FE-JOURNEY-MAPLAYERS-009 // // JRN-04: the journey map's layer switches as kept in this browser, the marks // the plan and the stays become, and the switches a reader is offered. @@ -169,4 +169,21 @@ describe('journey map layers', () => { expect(controls.find((c) => c.layer === 'plan')).toEqual({ layer: 'plan', count: 3, state: 'unavailable' }); expect(controls.find((c) => c.layer === 'entries')).toEqual({ layer: 'entries', count: 5, state: 'unavailable' }); }); + + it('FE-JOURNEY-MAPLAYERS-009: a journey whose recordings could not be read says so instead of nothing recorded', () => { + const recorded = (recordedRead: 'loading' | 'ready' | 'unavailable', recordedDays: number) => + buildControls({ + ownJourney: true, + sources: {}, + loading: false, + recordedRead, + counts: { places: 0, routes: 0, recordedDays, entries: 0, stays: 0, photos: 0 }, + }).find((c) => c.layer === 'recorded')?.state; + + expect(recorded('unavailable', 0)).toBe('unavailable'); + expect(recorded('unavailable', 2)).toBe('unavailable'); + expect(recorded('loading', 0)).toBe('loading'); + expect(recorded('loading', 1)).toBe('ready'); + expect(recorded('ready', 0)).toBe('empty'); + }); }); diff --git a/client/src/components/Journey/journeyMapLayers.ts b/client/src/components/Journey/journeyMapLayers.ts index 5ee50b3e2..35a22a0ef 100644 --- a/client/src/components/Journey/journeyMapLayers.ts +++ b/client/src/components/Journey/journeyMapLayers.ts @@ -209,10 +209,13 @@ export function buildControls({ sources, loading, counts, + recordedRead = 'ready', }: { ownJourney: boolean; sources: ReturnType; loading: boolean; + /** How the screen's own read of what was recorded went, for a journey's own map. */ + recordedRead?: 'loading' | 'ready' | 'unavailable'; counts: { places: number; routes: number; recordedDays: number; entries: number; stays: number; photos: number }; }): JourneyMapLayerControl[] { const list: JourneyMapLayerControl[] = []; @@ -225,9 +228,7 @@ export function buildControls({ } if (ownJourney || sources.recordings?.status !== 'withheld') { const state = ownJourney - ? counts.recordedDays > 0 - ? 'ready' - : 'empty' + ? ownRecordedState(recordedRead, counts.recordedDays) : stateOf(sources.recordings, loading, counts.recordedDays); list.push({ layer: 'recorded', count: counts.recordedDays, state }); } @@ -238,6 +239,12 @@ export function buildControls({ return list; } +function ownRecordedState(read: 'loading' | 'ready' | 'unavailable', days: number): JourneyMapLayerState { + if (read === 'unavailable') return 'unavailable'; + if (days > 0) return 'ready'; + return read === 'loading' ? 'loading' : 'empty'; +} + /** Whether a track belongs to the plan (a trip's GPX) or to what was recorded. */ export function isPlannedTrack(track: JourneyTrack): boolean { return !track.kind || track.kind === 'trip_gpx'; diff --git a/client/src/components/Journey/useJourneyMapLayers.ts b/client/src/components/Journey/useJourneyMapLayers.ts index eeb8834c1..020a102b2 100644 --- a/client/src/components/Journey/useJourneyMapLayers.ts +++ b/client/src/components/Journey/useJourneyMapLayers.ts @@ -44,6 +44,8 @@ interface Options { photoCount: number; /** What was recorded that the screen already holds: the kept days and the live trail of a journey's people. */ recordedTracks?: JourneyTrack[]; + /** How reading those went: a failed read is not a journey with nothing recorded. */ + recordedRead?: 'loading' | 'ready' | 'unavailable'; /** The colour of the pins of a day, so a stay and a recorded day read with their day. */ colorOfDay: (date: string) => string | null; /** The day an entry is on, for the colour of its stays. */ @@ -114,6 +116,7 @@ export function useJourneyMapLayers({ entryIdsOnMap, photoCount, recordedTracks = NO_TRACKS, + recordedRead = 'ready', colorOfDay, dateOfEntry, }: Options): JourneyMapLayersView { @@ -181,6 +184,7 @@ export function useJourneyMapLayers({ ownJourney: journeyId !== null, sources, loading, + recordedRead, counts: { places: places.length, routes: routes.length, @@ -194,6 +198,7 @@ export function useJourneyMapLayers({ journeyId, sources, loading, + recordedRead, places.length, routes.length, recordedDays, diff --git a/client/src/components/Journey/useJourneyRecordings.test.ts b/client/src/components/Journey/useJourneyRecordings.test.ts index 51f6043c8..a1c3af93c 100644 --- a/client/src/components/Journey/useJourneyRecordings.test.ts +++ b/client/src/components/Journey/useJourneyRecordings.test.ts @@ -1,4 +1,4 @@ -// FE-JRN-RECORDINGS-HOOK-001 to FE-JRN-RECORDINGS-HOOK-005 +// FE-JRN-RECORDINGS-HOOK-001 to FE-JRN-RECORDINGS-HOOK-006 // // JRN-01: the recorded days a journey keeps, as the desktop page and the phone // screen read them: the list, the lines they draw on the map, which day's @@ -113,4 +113,18 @@ describe('useJourneyRecordings', () => { act(() => result.current.close()); expect(result.current.dialog).toBeNull(); }); + + it('FE-JRN-RECORDINGS-HOOK-006: a first read that fails says so, rather than passing for a journey that kept no day', async () => { + repo.list.mockRejectedValueOnce(new Error('offline')); + const { result } = renderHook(() => useJourneyRecordings(JOURNEY, colorOf)); + expect(result.current.status).toBe('loading'); + await waitFor(() => expect(result.current.status).toBe('failed')); + expect(result.current.days).toEqual([]); + + repo.list.mockResolvedValueOnce([buildRecordedDay()]); + await act(async () => { + await pings.handlers.get('journey:recording:updated')?.(); + }); + expect(result.current.status).toBe('ready'); + }); }); diff --git a/client/src/components/Journey/useJourneyRecordings.ts b/client/src/components/Journey/useJourneyRecordings.ts index 6e973bfc1..350192c0f 100644 --- a/client/src/components/Journey/useJourneyRecordings.ts +++ b/client/src/components/Journey/useJourneyRecordings.ts @@ -8,6 +8,8 @@ import type { JourneyRecordingViewProps } from './useJourneyRecordingDay'; interface JourneyRecordingsState { days: JourneyRecordedDay[]; + /** Whether the days were read: a failed first read is not a journey that kept none. */ + status: 'idle' | 'loading' | 'ready' | 'failed'; byDate: ReadonlyMap; /** The kept days as lines for the journey map, one per piece, never joined across a gap. */ tracks: JourneyTrack[]; @@ -44,7 +46,16 @@ export function useJourneyRecordings( // never shows the other one's days, and nothing has to be reset in an effect. const [loaded, setLoaded] = useState<{ journeyId: number | null; days: JourneyRecordedDay[] }>(NOTHING_LOADED); const [opened, setOpened] = useState<{ journeyId: number | null; date: string | null }>(NOTHING_OPEN); + const [failedFor, setFailedFor] = useState(null); const days = loaded.journeyId === journeyId ? loaded.days : NO_DAYS; + const status = + journeyId === null + ? 'idle' + : loaded.journeyId === journeyId + ? 'ready' + : failedFor === journeyId + ? 'failed' + : 'loading'; const openDate = opened.journeyId === journeyId ? opened.date : null; const read = useCallback(async () => { @@ -61,12 +72,14 @@ export function useJourneyRecordings( useEffect(() => { let cancelled = false; void read().then((next) => { - if (next && !cancelled) setLoaded(next); + if (cancelled) return; + if (next) setLoaded(next); + else setFailedFor(journeyId); }); return () => { cancelled = true; }; - }, [read]); + }, [read, journeyId]); const reload = useCallback(async () => { const next = await read(); @@ -130,7 +143,7 @@ export function useJourneyRecordings( [journeyId, journeyTitle, openDate, byDate, kept, removed, close] ); - return { days, byDate, tracks, openDate, dialog, open, close, kept, removed }; + return { days, status, byDate, tracks, openDate, dialog, open, close, kept, removed }; } /** Stable empty identity, so a journey without kept days never re-renders its map for nothing. */ diff --git a/client/src/pages/JourneyPublicPage.tsx b/client/src/pages/JourneyPublicPage.tsx index c73ab6474..b0b25d90c 100644 --- a/client/src/pages/JourneyPublicPage.tsx +++ b/client/src/pages/JourneyPublicPage.tsx @@ -166,6 +166,28 @@ export default function JourneyPublicPage() { })) : []; + // A photo hit on the map opens the photos it holds, as the gallery would. + const openMapPhotos = (ids: string[]) => { + const wanted = new Set(ids); + const picked = allPhotos.filter((p) => wanted.has(String(p.id))); + if (!token || picked.length === 0) return; + setLightbox({ + photos: picked.map((p) => ({ + id: String(p.id), + src: photoUrl(p, token, 'original'), + caption: p.caption, + mediaType: p.media_type, + })), + index: 0, + }); + }; + + // The map tab only exists while the timeline is not shared, so an entry is shown on the map itself. + const focusEntryOnMap = (entryId: string) => { + setActiveEntryId(entryId); + mapRef.current?.focusMarker(entryId); + }; + // Shared timeline renderer used in both layout modes const renderTimeline = () => (
@@ -633,6 +655,7 @@ export default function JourneyPublicPage() { checkins={[]} entries={sidebarMapItems as any} photos={photoLayer} + onPhotoClick={openMapPhotos} height={9999} fullScreen activeMarkerId={activeEntryId ?? undefined} @@ -683,6 +706,7 @@ export default function JourneyPublicPage() { layers={mapLayers} entryTitleOf={entryTitleOf} photos={photoLayer} + onPhotoClick={openMapPhotos} controlsTop={availableViews.length > 1 ? 'calc(env(safe-area-inset-top, 0px) + 56px)' : undefined} showMood={journey.show_mood !== 0} showWeather={journey.show_weather !== 0} @@ -699,13 +723,15 @@ export default function JourneyPublicPage() { {view === 'map' && perms.share_map && (
diff --git a/client/src/pages/journeyDetail/useJourneyDetail.ts b/client/src/pages/journeyDetail/useJourneyDetail.ts index c2d2d1e49..e8e65c3b6 100644 --- a/client/src/pages/journeyDetail/useJourneyDetail.ts +++ b/client/src/pages/journeyDetail/useJourneyDetail.ts @@ -552,12 +552,17 @@ export function useJourneyDetail({ journeyId, embedded = false }: JourneyDetailO (current?.trips ?? []).map(trip => trip.trip_id).join(','), showTripTracks ? 'tracks' : '', ].join('|'), [current?.entries, current?.trips, showTripTracks]) + // The kept days and the live trail are read by this screen, not by the layers' read. + const recordedRead = recordings.status === 'failed' || dawarichTrail.status === 'unavailable' + ? 'unavailable' + : recordings.status === 'loading' || dawarichTrail.status === 'loading' ? 'loading' : 'ready' const mapLayers = useJourneyMapLayers({ source: journeyLoaded && current ? { journeyId: current.id } : null, refreshKey: mapRefreshKey, entryIdsOnMap, photoCount: mapPhotos.length, recordedTracks: mapTracks, + recordedRead, colorOfDay, dateOfEntry, })