From 56b4509b03894e7434c1099cc9df047d64aeb5fa Mon Sep 17 00:00:00 2001 From: Maurice Date: Tue, 18 Aug 2026 21:03:45 +0200 Subject: [PATCH] feat(journey): put photos on the map where they were taken MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The visible half of the capture-metadata work (#1614). Photos that know their coordinates are drawn as thumbnails on the Journey map, in the shape the report asked for: a picture rather than an abstract pin, and a count when several collapse into one. Clustering is a screen-space grid rather than a library. The Journey maps have never had clustering, and the one the planner uses hangs off react-leaflet while this map drives Leaflet directly; bucketing by rounded pixel position is a few lines, is deterministic, and does the job — photos of one place collapse into one thumbnail with a count, and pulling the map apart separates them again. The layer sits below the entry pins, because the itinerary is the point of the map and the photos are context. Its own effect, so photos arriving do not tear down and rebuild the map the way the entry effect does, and redrawn on zoom and pan because the buckets are in screen space. The thumbnail URL is built in the page rather than the hook: it needs the share token, which is the credential the byte proxy checks. Also re-points the trailing-migration guard at the new last slot, which is what its own comment asks of anyone appending one. --- .../components/Journey/JourneyMap.test.tsx | 36 +++++++ client/src/components/Journey/JourneyMap.tsx | 98 ++++++++++++++++++- client/src/pages/JourneyPublicPage.tsx | 15 ++- .../pages/JourneyPublicPage.wiring.test.tsx | 1 + .../pages/journeyPublic/journeyPublicModel.ts | 5 + .../pages/journeyPublic/useJourneyPublic.ts | 11 ++- .../integration/leg-mode-incoming.test.ts | 32 ++---- 7 files changed, 173 insertions(+), 25 deletions(-) diff --git a/client/src/components/Journey/JourneyMap.test.tsx b/client/src/components/Journey/JourneyMap.test.tsx index 2678fc038..d73e8fd52 100644 --- a/client/src/components/Journey/JourneyMap.test.tsx +++ b/client/src/components/Journey/JourneyMap.test.tsx @@ -29,6 +29,10 @@ vi.mock('leaflet', () => { getZoom: vi.fn(() => 10), zoomIn: vi.fn(), zoomOut: vi.fn(), + // The photo layer subscribes to zoom/pan and clusters in screen space (#1614). + on: vi.fn(), + off: vi.fn(), + latLngToContainerPoint: vi.fn(() => ({ x: 0, y: 0 })), }; return { default: { @@ -38,6 +42,7 @@ vi.mock('leaflet', () => { polyline: vi.fn(() => { const line: any = { addTo: vi.fn(() => line), bindTooltip: vi.fn(() => line) }; return line }), divIcon: vi.fn(() => ({})), latLngBounds: vi.fn(() => ({})), + layerGroup: vi.fn(() => ({ addLayer: vi.fn(), addTo: vi.fn(), remove: vi.fn() })), }, map: vi.fn(() => mockMap), tileLayer: vi.fn(() => ({ addTo: vi.fn() })), @@ -576,4 +581,35 @@ describe('JourneyMap', () => { expect(labels.some(l => typeof l === 'string' && l.includes(' { + render( + , + ); + + // The mock projects every photo to the same point, so all three land in one + // bucket: one marker, and its badge says 3. + const html = vi.mocked(L.divIcon).mock.calls.map(c => String(c[0]?.html)).filter(h => h.includes('/uploads/')); + expect(html).toHaveLength(1); + expect(html[0]).toContain('/uploads/a.jpg'); + expect(html[0]).toContain('>3<'); + }); + + it('FE-COMP-JOURNEYMAP-047: a lone photo gets no count badge', () => { + render( + , + ); + + const html = vi.mocked(L.divIcon).mock.calls.map(c => String(c[0]?.html)).filter(h => h.includes('/uploads/')); + expect(html).toHaveLength(1); + expect(html[0]).not.toMatch(/>\d+() + for (const photo of photos) { + const pt = map.latLngToContainerPoint([photo.lat, photo.lng]) + const key = `${Math.round(pt.x / PHOTO_CLUSTER_PX)}:${Math.round(pt.y / PHOTO_CLUSTER_PX)}` + const list = buckets.get(key) + if (list) list.push(photo) + else buckets.set(key, [photo]) + } + return [...buckets.values()].map(members => ({ + // Anchor on the first member rather than the centroid: the thumbnail shown is + // that photo's, so the pin should point where that picture was taken. + lat: members[0].lat, + lng: members[0].lng, + members, + })) +} + +function photoMarkerHtml(thumbUrl: string, count: number): string { + const badge = count > 1 + ? `${count}` + : '' + return `
${badge}` +} + export interface JourneyMapHandle { highlightMarker: (id: string | null) => void focusMarker: (id: string) => void invalidateSize: () => void } +/** A photo that knows where it was taken (#1614). */ +export interface MapPhoto { + id: string + lat: number + lng: number + thumbUrl: string +} + interface MapEntry { id: string lat: number @@ -35,6 +82,9 @@ interface Props { ref?: Ref checkins: any[] entries: MapEntry[] + /** Photos placed by their own capture coordinates, clustered by proximity. */ + photos?: MapPhoto[] + onPhotoClick?: (photoIds: string[]) => void trail?: { lat: number; lng: number }[] /** Routed GPX geometries from the journey's trips (#1260). */ tracks?: JourneyTrack[] @@ -92,7 +142,7 @@ const EMPTY_TRACKS: JourneyTrack[] = [] const TRACK_FALLBACK_COLOR = '#4f46e5' function JourneyMap( - { entries, trail, tracks, height = 220, dark, activeMarkerId, onMarkerClick, fullScreen, paddingBottom, ref }: Props, + { entries, photos, onPhotoClick, trail, tracks, height = 220, dark, activeMarkerId, onMarkerClick, fullScreen, paddingBottom, ref }: Props, ) { const stableTrail = trail || EMPTY_TRAIL const stableTracks = tracks || EMPTY_TRACKS @@ -104,6 +154,9 @@ function JourneyMap( const highlightedRef = useRef(null) const onMarkerClickRef = useRef(onMarkerClick) onMarkerClickRef.current = onMarkerClick + const photoLayerRef = useRef(null) + const onPhotoClickRef = useRef(onPhotoClick) + onPhotoClickRef.current = onPhotoClick const darkRef = useRef(dark) darkRef.current = dark @@ -284,6 +337,49 @@ function JourneyMap( } }, [entries, stableTrail, stableTracks, dark, mapTileUrl, fullScreen, paddingBottom]) + // Photo layer (#1614). Its own effect on purpose: photos arriving must not tear + // down and rebuild the map the way the entry effect does. Redrawn on zoom and + // pan because the clustering is done in screen space. + useEffect(() => { + const map = mapRef.current + if (!map) return + + const draw = () => { + photoLayerRef.current?.remove() + photoLayerRef.current = null + if (!photos?.length) return + + const group = L.layerGroup() + for (const cluster of clusterPhotos(map, photos)) { + const marker = L.marker([cluster.lat, cluster.lng], { + icon: L.divIcon({ + className: '', + iconSize: [48, 48], + iconAnchor: [24, 24], + html: photoMarkerHtml(cluster.members[0].thumbUrl, cluster.members.length), + }), + // Below the entry pins: the itinerary is the point of the map, the photos + // are context. + zIndexOffset: -500, + }) + marker.on('click', () => onPhotoClickRef.current?.(cluster.members.map(m => m.id))) + group.addLayer(marker) + } + group.addTo(map) + photoLayerRef.current = group + } + + draw() + map.on('zoomend', draw) + map.on('moveend', draw) + return () => { + map.off('zoomend', draw) + map.off('moveend', draw) + photoLayerRef.current?.remove() + photoLayerRef.current = null + } + }, [photos, entries, stableTrail, stableTracks, dark, mapTileUrl, fullScreen, paddingBottom]) + // react to activeMarkerId prop changes — runs after map is built useEffect(() => { if (!activeMarkerId || !mapRef.current) return diff --git a/client/src/pages/JourneyPublicPage.tsx b/client/src/pages/JourneyPublicPage.tsx index a5dadfe1a..e75a976ec 100644 --- a/client/src/pages/JourneyPublicPage.tsx +++ b/client/src/pages/JourneyPublicPage.tsx @@ -117,6 +117,7 @@ export default function JourneyPublicPage() { sidebarMapItems, allPhotos, stopNumberById, + mapPhotos, desktopTwoColumn, } = useJourneyPublic(); @@ -148,6 +149,17 @@ export default function JourneyPublicPage() { perms.share_map && { id: 'map' as const, icon: MapPin, label: t('journey.share.map') }, ].filter(Boolean) as { id: 'timeline' | 'gallery' | 'map'; icon: any; label: string }[]; + // The hook cannot build these: the thumbnail URL needs the share token, which is + // the credential for the byte proxy. + const photoLayer = token + ? mapPhotos.map((p: { id: string; lat: number; lng: number; photoId: number }) => ({ + id: p.id, + lat: p.lat, + lng: p.lng, + thumbUrl: photoUrl({ photo_id: p.photoId }, token, 'thumbnail'), + })) + : []; + // Shared timeline renderer used in both layout modes const renderTimeline = () => (
@@ -770,6 +782,7 @@ export default function JourneyPublicPage() { ref={mapRef} checkins={[]} entries={sidebarMapItems as any} + photos={photoLayer} height={9999} fullScreen activeMarkerId={activeEntryId ?? undefined} @@ -830,7 +843,7 @@ export default function JourneyPublicPage() { {/* Map (standalone tab — only in single-column mode) */} {view === 'map' && perms.share_map && (
- +
)}
diff --git a/client/src/pages/JourneyPublicPage.wiring.test.tsx b/client/src/pages/JourneyPublicPage.wiring.test.tsx index e88780ad8..e7fd69019 100644 --- a/client/src/pages/JourneyPublicPage.wiring.test.tsx +++ b/client/src/pages/JourneyPublicPage.wiring.test.tsx @@ -83,6 +83,7 @@ function buildHook(over: Record = {}): Record sortedDates: [...grouped.keys()].sort(), sidebarMapItems: [], stopNumberById: new Map(), + mapPhotos: [] as { id: string; lat: number; lng: number; photoId: number }[], displayDates: [...grouped.keys()].sort(), newestFirst: false, setNewestFirst: vi.fn(), diff --git a/client/src/pages/journeyPublic/journeyPublicModel.ts b/client/src/pages/journeyPublic/journeyPublicModel.ts index 5ee6a3a76..7cecf9698 100644 --- a/client/src/pages/journeyPublic/journeyPublicModel.ts +++ b/client/src/pages/journeyPublic/journeyPublicModel.ts @@ -39,6 +39,11 @@ export interface PublicGalleryPhoto { owner_id?: number | null file_path?: string | null caption?: string | null + /** Where and when the picture was taken (#1614). Absent for most photos, and the + * coordinates are withheld entirely unless the owner shared the map. */ + taken_at?: string | null + lat?: number | null + lng?: number | null } export function groupByDate(entries: PublicEntry[]): Map { diff --git a/client/src/pages/journeyPublic/useJourneyPublic.ts b/client/src/pages/journeyPublic/useJourneyPublic.ts index b3b67f43d..d4ae76fe8 100644 --- a/client/src/pages/journeyPublic/useJourneyPublic.ts +++ b/client/src/pages/journeyPublic/useJourneyPublic.ts @@ -97,6 +97,15 @@ export function useJourneyPublic() { return m }, [sidebarMapItems]) + // Photos that know where they were taken. Only meaningful when the map is shared — + // the server nulls the coordinates otherwise, so this comes out empty by itself. + const mapPhotos = useMemo( + () => gallery + .filter(p => typeof p.lat === 'number' && typeof p.lng === 'number') + .map(p => ({ id: String(p.id), lat: p.lat!, lng: p.lng!, photoId: p.photo_id })), + [gallery], + ) + // A journey shared while the trip is still running reads like a blog, so the owner // can publish it newest-first (#1614). The reader may flip it either way; only the // display order changes, never the numbering. @@ -126,7 +135,7 @@ export function useJourneyPublic() { view, setView, lightbox, setLightbox, showLangPicker, setShowLangPicker, mapRef, activeEntryId, setActiveEntryId, viewingEntry, setViewingEntry, handleMarkerClick, perms, journey, stats, - timelineEntries, groupedEntries, sortedDates, displayDates, sidebarMapItems, allPhotos, stopNumberById, + timelineEntries, groupedEntries, sortedDates, displayDates, sidebarMapItems, allPhotos, stopNumberById, mapPhotos, newestFirst: effectiveNewestFirst, setNewestFirst, desktopTwoColumn, } diff --git a/server/tests/integration/leg-mode-incoming.test.ts b/server/tests/integration/leg-mode-incoming.test.ts index a1e7a328d..4ad1b4ff5 100644 --- a/server/tests/integration/leg-mode-incoming.test.ts +++ b/server/tests/integration/leg-mode-incoming.test.ts @@ -43,9 +43,8 @@ describe('incoming_leg_transport_mode migration', () => { // // >>> Appending a migration? Re-point the "undo" below at whatever yours // >>> does. That is the whole maintenance cost of this guard. The current - // >>> trailing slot copies the lowest-id admin's Google and Unsplash keys - // >>> into app_settings, so undoing it means clearing those rows and - // >>> leaving an admin holding a key for it to find again. + // >>> trailing slot adds journey_share_tokens.newest_first, so undoing it + // >>> means dropping that column and letting the replay put it back. const upgraded = new Database(':memory:'); upgraded.exec('PRAGMA foreign_keys = ON'); createTables(upgraded); @@ -53,30 +52,19 @@ describe('incoming_leg_transport_mode migration', () => { const { version } = upgraded.prepare('SELECT version FROM schema_version').get() as { version: number }; - upgraded.prepare("DELETE FROM app_settings WHERE key IN ('maps_api_key', 'unsplash_api_key')").run(); - upgraded - .prepare( - `INSERT INTO users (id, username, email, password_hash, role, maps_api_key) - VALUES (2, 'b', 'b@test.local', 'x', 'admin', NULL)` - ) - .run(); - // The only key on the install, which is the one condition under which the - // migration promotes it at all; the rest of that decision is pinned in - // tests/unit/db/instance-api-key-backfill.test.ts. - upgraded - .prepare( - `INSERT INTO users (id, username, email, password_hash, role, maps_api_key) - VALUES (1, 'a', 'a@test.local', 'x', 'admin', 'key-of-the-first-admin')` - ) - .run(); + const hasNewestFirst = () => + (upgraded.prepare("SELECT name FROM pragma_table_info('journey_share_tokens')").all() as { name: string }[]) + .some(c => c.name === 'newest_first'); + + expect(hasNewestFirst()).toBe(true); + upgraded.exec('ALTER TABLE journey_share_tokens DROP COLUMN newest_first'); + expect(hasNewestFirst()).toBe(false); upgraded.prepare('UPDATE schema_version SET version = ?').run(version - 1); runMigrations(upgraded); - expect(upgraded.prepare("SELECT value FROM app_settings WHERE key = 'maps_api_key'").get()).toEqual({ - value: 'key-of-the-first-admin', - }); + expect(hasNewestFirst()).toBe(true); expect(upgraded.prepare('SELECT version FROM schema_version').get()).toEqual({ version }); upgraded.close(); });