From 9cc427d15625241d7db8843276eb083d18d1286c Mon Sep 17 00:00:00 2001 From: Maurice Date: Wed, 2 Sep 2026 19:00:48 +0200 Subject: [PATCH] fix(journey): gallery actions on tablets, tracks switch on phones (#2194) Between 768 and 1023px the view-controls row is hidden and the provider and upload buttons had moved into it, leaving no way to add photos; the gallery actions render below the floating bar there now. The mobile journey settings sheet gets the linked-trip tracks switch the desktop dialog got, and the request gate and the fitBounds behaviour are pinned by tests. --- .../components/Journey/JourneyMap.test.tsx | 35 ++++++++- .../screens/journey/MJourneySettingsSheet.tsx | 35 ++++++++- client/src/pages/JourneyDetailPage.tsx | 54 ++++++++------ .../pages/JourneyDetailPage.wiring.test.tsx | 51 +++++++++++++ .../journeyDetail/useJourneyDetail.test.tsx | 72 ++++++++++++++++++- .../journey/MJourneySettingsSheet.test.tsx | 44 +++++++++++- 6 files changed, 262 insertions(+), 29 deletions(-) diff --git a/client/src/components/Journey/JourneyMap.test.tsx b/client/src/components/Journey/JourneyMap.test.tsx index 518e53908..5178f2909 100644 --- a/client/src/components/Journey/JourneyMap.test.tsx +++ b/client/src/components/Journey/JourneyMap.test.tsx @@ -1,4 +1,4 @@ -// FE-COMP-JOURNEYMAP-001 to FE-COMP-JOURNEYMAP-027 +// FE-COMP-JOURNEYMAP-001 to FE-COMP-JOURNEYMAP-049 vi.mock('../../api/websocket', () => ({ connect: vi.fn(), @@ -604,6 +604,39 @@ describe('JourneyMap', () => { expect(colors).not.toContain('#ffffff'); }); + /** The opening viewport is the assertion here, so the frame gets read out of the bounds call. */ + const fitCoords = () => vi.mocked(L.latLngBounds).mock.calls[0][0] as [number, number][]; + + // #2194: a recorded drive across the country used to pull the opening view out to + // the whole trip, leaving the journey's own entries as a speck. + it('FE-COMP-JOURNEYMAP-048: frames the entries, not the tracks, when the journey has both', () => { + const origRAF = globalThis.requestAnimationFrame; + globalThis.requestAnimationFrame = (cb: FrameRequestCallback) => { cb(0); return 0; }; + try { + render( + + ); + + expect(mockedMap().fitBounds).toHaveBeenCalled(); + expect(fitCoords()).toEqual([[48.8566, 2.3522], [52.52, 13.405]]); + } finally { + globalThis.requestAnimationFrame = origRAF; + } + }); + + it('FE-COMP-JOURNEYMAP-049: with nothing else on the map the tracks are framed instead of the world', () => { + const origRAF = globalThis.requestAnimationFrame; + globalThis.requestAnimationFrame = (cb: FrameRequestCallback) => { cb(0); return 0; }; + try { + render(); + + expect(fitCoords()).toEqual([[47.1, 11.2], [47.2, 11.3]]); + expect(mockedMap().setView).not.toHaveBeenCalledWith([30, 0], 2); + } finally { + globalThis.requestAnimationFrame = origRAF; + } + }); + // A string handed to bindTooltip becomes innerHTML. A track name is a place // name off a shared trip and an entry label is a journey title, so both are // written by someone who is not necessarily the person reading the map — and diff --git a/client/src/mobile/screens/journey/MJourneySettingsSheet.tsx b/client/src/mobile/screens/journey/MJourneySettingsSheet.tsx index 5d0ba1279..27c04f8fc 100644 --- a/client/src/mobile/screens/journey/MJourneySettingsSheet.tsx +++ b/client/src/mobile/screens/journey/MJourneySettingsSheet.tsx @@ -2,7 +2,7 @@ import { useEffect, useRef, useState } from 'react' import { useNavigate } from 'react-router' import { X, Image, Plus, Trash2, UserPlus, Link as LinkIcon, - List, Grid3x3, MapPin, Archive, ArchiveRestore, + List, Grid3x3, MapPin, Route, Archive, ArchiveRestore, } from 'lucide-react' import MSheet from '../../components/MSheet' import MIconBtn from '../../components/MIconBtn' @@ -62,6 +62,7 @@ export default function MJourneySettingsSheet({ const [availableTrips, setAvailableTrips] = useState([]) const [linkingTripId, setLinkingTripId] = useState(null) const [shareLink, setShareLink] = useState(null) + const [savingTracks, setSavingTracks] = useState(false) const [copied, setCopied] = useState(false) const coverRef = useRef(null) @@ -98,6 +99,22 @@ export default function MJourneySettingsSheet({ } } + // Written on the spot rather than on Save, like the archive button: it is a view + // setting and the point of it is watching the map change (#2194). onRefresh, not + // onSaved, because onSaved closes the sheet the instant the switch is flipped and + // would drop a title the owner has typed but not saved yet. + const handleTracksToggle = async (next: boolean) => { + setSavingTracks(true) + try { + await updateJourney(journey.id, { show_trip_tracks: next }) + onRefresh() + } catch { + toast.error(t('journey.settings.saveFailed')) + } finally { + setSavingTracks(false) + } + } + const openAddTrip = async () => { setAddTripOpen(v => !v) if (availableTrips.length === 0) { @@ -236,6 +253,22 @@ export default function MJourneySettingsSheet({ className={`${inputShell} text-[0.8125rem] font-medium`} /> + {/* Trip GPX tracks on the journey map (#2194) */} +
{t('journey.settings.tracks')}
+
+ +
+
{t('journey.settings.showTripTracks')}
+
{t('journey.settings.showTripTracksHint')}
+
+ +
+ {/* Synced trips */}
{t('journey.detail.syncedTrips')}
{journey.trips.map(trip => ( diff --git a/client/src/pages/JourneyDetailPage.tsx b/client/src/pages/JourneyDetailPage.tsx index 6c63a28f2..eec0a2f9a 100644 --- a/client/src/pages/JourneyDetailPage.tsx +++ b/client/src/pages/JourneyDetailPage.tsx @@ -89,6 +89,33 @@ function JourneyDetailPageDesktop() { const skeletonLabel = hideSkeletons ? t('journey.skeletons.show') : t('journey.skeletons.hide') const barButton = 'w-10 h-10 flex-shrink-0 rounded-lg bg-surface-elevated backdrop-blur-lg border border-edge shadow-lg text-content-secondary flex items-center justify-center hover:bg-surface-hover active:scale-95 transition-transform' + // Adding photos hangs off the controls row, which is hidden below 1024px, so + // the gallery takes the actions over there instead. Exactly one host renders + // them, never both. + const galleryActions = canEditEntries && view === 'gallery' ? ( +
+ {galleryProviders.map(p => ( + + ))} + +
+ ) : null + return (
@@ -346,29 +373,7 @@ function JourneyDetailPageDesktop() { {t('journey.detail.addEntry')} )} - {canEditEntries && view === 'gallery' && ( -
- {galleryProviders.map(p => ( - - ))} - -
- )} + {!isMobileChromeless && galleryActions}
{/* Timeline (desktop only — mobile uses fullscreen combined view above) */} @@ -464,6 +469,9 @@ function JourneyDetailPageDesktop() { className={view === 'gallery' ? '' : 'hidden'} style={showMobileGallery ? { paddingTop: 'calc(var(--nav-h, 56px) + 64px)' } : undefined} > + {showMobileGallery && galleryActions && ( +
{galleryActions}
+ )} { galleryUploadRef.current = fn }} onRegisterProviders={(providers, browse) => { setGalleryProviders(providers); galleryBrowseRef.current = browse }} diff --git a/client/src/pages/JourneyDetailPage.wiring.test.tsx b/client/src/pages/JourneyDetailPage.wiring.test.tsx index 4938a52d3..3d4d6bdf5 100644 --- a/client/src/pages/JourneyDetailPage.wiring.test.tsx +++ b/client/src/pages/JourneyDetailPage.wiring.test.tsx @@ -524,4 +524,55 @@ describe('JourneyDetailPage wiring', () => { expect(screen.getByRole('button', { name: 'journey.studio.openAria' })).toBeInTheDocument(); expect(screen.getByRole('button', { name: 'journey.skeletons.hide' })).toBeInTheDocument(); }); + + // ── Gallery actions between 768 and 1023px ──────────────────────────────── + // The controls row that hosts them is `hidden` there, and a tablet has no + // other door into the gallery (phones run the MJourneyDetail screen instead). + + const immich = [{ id: 'immich', name: 'Immich' }]; + + it('FE-JRN-DETWIRE-037: the narrow gallery renders upload and providers outside the hidden controls row', () => { + setup({ isMobile: true, view: 'gallery', galleryProviders: immich }); + + expect(screen.getByText('common.upload').closest('.hidden')).toBeNull(); + expect(screen.getByText('Immich').closest('.hidden')).toBeNull(); + }); + + it('FE-JRN-DETWIRE-038: those buttons open the picker and the provider browser', () => { + const upload = vi.fn(); + const browse = vi.fn(); + setup({ + isMobile: true, view: 'gallery', galleryProviders: immich, + galleryUploadRef: { current: upload }, galleryBrowseRef: { current: browse }, + }); + + fireEvent.click(screen.getByText('common.upload')); + expect(upload).toHaveBeenCalledTimes(1); + + fireEvent.click(screen.getByText('Immich')); + expect(browse).toHaveBeenCalledWith('immich'); + }); + + it('FE-JRN-DETWIRE-039: only one host renders them, on either side of the breakpoint', () => { + const { unmount } = setup({ isMobile: true, view: 'gallery', galleryProviders: immich }); + expect(screen.getAllByText('common.upload')).toHaveLength(1); + expect(screen.getAllByText('Immich')).toHaveLength(1); + unmount(); + + setup({ view: 'gallery', galleryProviders: immich }); + expect(screen.getAllByText('common.upload')).toHaveLength(1); + expect(screen.getAllByText('Immich')).toHaveLength(1); + }); + + it('FE-JRN-DETWIRE-040: a read-only viewer gets no upload or provider button there', () => { + setup({ isMobile: true, view: 'gallery', canEditEntries: false, galleryProviders: immich }); + expect(screen.queryByText('common.upload')).not.toBeInTheDocument(); + expect(screen.queryByText('Immich')).not.toBeInTheDocument(); + }); + + it('FE-JRN-DETWIRE-041: the narrow timeline tab keeps the gallery actions away', () => { + setup({ isMobile: true, view: 'timeline', galleryProviders: immich }); + expect(screen.queryByText('common.upload')).not.toBeInTheDocument(); + expect(screen.queryByText('Immich')).not.toBeInTheDocument(); + }); }); diff --git a/client/src/pages/journeyDetail/useJourneyDetail.test.tsx b/client/src/pages/journeyDetail/useJourneyDetail.test.tsx index 9f7c24d14..96c04b781 100644 --- a/client/src/pages/journeyDetail/useJourneyDetail.test.tsx +++ b/client/src/pages/journeyDetail/useJourneyDetail.test.tsx @@ -1,12 +1,13 @@ -// FE-JRN-DETHOOK-001 to FE-JRN-DETHOOK-025 +// FE-JRN-DETHOOK-001 to FE-JRN-DETHOOK-029 import { describe, it, expect, beforeEach, afterEach, vi, type Mock } from 'vitest'; import { useLocation } from 'react-router'; -import { http, HttpResponse } from 'msw'; +import { delay, http, HttpResponse } from 'msw'; import { server } from '../../../tests/helpers/msw/server'; import { render, screen, fireEvent, waitFor } from '../../../tests/helpers/render'; import { addListener, removeListener } from '../../api/websocket'; import { useJourneyStore } from '../../store/journeyStore'; import type { JourneyDetail, JourneyEntry } from '../../store/journeyStore'; +import type { JourneyTrack } from '@trek/shared'; import { useJourneyDetail } from './useJourneyDetail'; let routeParams: { id?: string } = { id: '7' }; @@ -53,6 +54,20 @@ function serveJourney(detail: JourneyDetail | Record): void { server.use(http.get('/api/journeys/7', () => HttpResponse.json(detail))); } +function buildTrack(over: Partial = {}): JourneyTrack { + return { place_id: 4, trip_id: 2, name: 'Ridge walk', color: '#ff0000', points: [[47.1, 11.2], [47.2, 11.3]], ...over }; +} + +/** Counts what the map's geometry endpoint is actually asked for. */ +function serveTracks(journeyId: number, tracks: JourneyTrack[] = []) { + const asked = vi.fn(); + server.use(http.get(`/api/journeys/${journeyId}/tracks`, () => { + asked(); + return HttpResponse.json({ tracks }); + })); + return asked; +} + // ── Harness ────────────────────────────────────────────────────────────────── type HookState = ReturnType; @@ -383,4 +398,57 @@ describe('useJourneyDetail', () => { latest.scrollFeedTo('bottom'); expect(scrollTo).toHaveBeenLastCalledWith({ top: 9000, behavior: 'smooth' }); }); + + // ── Trip GPX tracks (#2194) ──────────────────────────────────────────────── + // The switch gates the request, not the drawing: the endpoint ships unthinned + // geometry, megabytes of it for a trip with a season of recorded drives. + + it('FE-JRN-DETHOOK-026: a journey with the switch off never asks for the geometry', async () => { + const asked = serveTracks(7, [buildTrack()]); + setup(); + await waitFor(() => expect(latest.current).not.toBeNull()); + await new Promise(r => setTimeout(r, 20)); + + expect(asked).not.toHaveBeenCalled(); + expect(latest.tracks).toEqual([]); + }); + + it('FE-JRN-DETHOOK-027: with the switch on the tracks are fetched and handed on', async () => { + // The column is INTEGER, so what arrives over the wire is 1, not true. + serveJourney(buildDetail({ show_trip_tracks: 1 })); + const asked = serveTracks(7, [buildTrack()]); + setup(); + + await waitFor(() => expect(latest.tracks).toHaveLength(1)); + expect(latest.tracks[0].name).toBe('Ridge walk'); + expect(asked).toHaveBeenCalledTimes(1); + }); + + it('FE-JRN-DETHOOK-028: the geometry follows the route id, not the journey still on screen', async () => { + // loadJourney leaves the previous journey in place while the next one loads, so an + // unguarded read of the flag would fetch journey B's tracks off journey A's setting. + routeParams = { id: '8' }; + useJourneyStore.setState({ current: buildDetail({ show_trip_tracks: 1 }) }); + const asked = serveTracks(8, [buildTrack()]); + server.use(http.get('/api/journeys/8', async () => { + await delay(20); + return HttpResponse.json(buildDetail({ id: 8, title: 'Norway 2027' })); + })); + setup(); + + await waitFor(() => expect(latest.current?.id).toBe(8)); + expect(asked).not.toHaveBeenCalled(); + expect(latest.tracks).toEqual([]); + }); + + it('FE-JRN-DETHOOK-029: a failed geometry lookup costs the map its lines, not the page', async () => { + serveJourney(buildDetail({ show_trip_tracks: 1 })); + server.use(http.get('/api/journeys/7/tracks', () => new HttpResponse(null, { status: 500 }))); + setup(); + + await waitFor(() => expect(latest.current?.title).toBe('Japan 2026')); + await new Promise(r => setTimeout(r, 20)); + expect(latest.tracks).toEqual([]); + expect(addToast).not.toHaveBeenCalled(); + }); }); diff --git a/client/tests/unit/mobile/journey/MJourneySettingsSheet.test.tsx b/client/tests/unit/mobile/journey/MJourneySettingsSheet.test.tsx index e89815eb4..b76732601 100644 --- a/client/tests/unit/mobile/journey/MJourneySettingsSheet.test.tsx +++ b/client/tests/unit/mobile/journey/MJourneySettingsSheet.test.tsx @@ -1,4 +1,4 @@ -// FE-MOB-JSET-001 to FE-MOB-JSET-030 +// FE-MOB-JSET-001 to FE-MOB-JSET-034 import { describe, it, expect, beforeEach, afterEach, vi, type Mock } from 'vitest'; import { render, screen, fireEvent, waitFor, within } from '../../../helpers/render'; import MJourneySettingsSheet from '../../../../src/mobile/screens/journey/MJourneySettingsSheet'; @@ -251,7 +251,9 @@ describe('MJourneySettingsSheet', () => { expect(await screen.findByText(`${window.location.origin}/public/journey/tok-1`)).toBeInTheDocument(); expect(addToast).toHaveBeenCalledWith('Share link created', 'success', undefined); - expect(screen.getAllByRole('switch')).toHaveLength(3); + for (const section of ['Timeline', 'Gallery', 'Map']) { + expect(screen.getByRole('switch', { name: section })).toBeInTheDocument(); + } }); it('FE-MOB-JSET-019: a failing share-link creation shows an error toast', async () => { @@ -398,4 +400,42 @@ describe('MJourneySettingsSheet', () => { ); expect(screen.getByRole('button', { name: 'Change cover' })).toBeInTheDocument(); }); + + // #2194: the phone is the only surface a mobile-only owner ever sees, so the + // switch has to live here too, with the semantics of the desktop dialog. + it('FE-MOB-JSET-032: the trip-GPX switch reads off by default and turns on in one tap', async () => { + const { props } = setup(); + + const tracks = await screen.findByRole('switch', { name: /GPX/i }); + expect(tracks).toHaveAttribute('aria-checked', 'false'); + + fireEvent.click(tracks); + + await waitFor(() => expect(updateJourney).toHaveBeenCalledWith(12, { show_trip_tracks: true })); + // Refresh, never onSaved: onSaved closes the sheet as the switch is flipped. + await waitFor(() => expect(props.onRefresh).toHaveBeenCalled()); + expect(props.onSaved).not.toHaveBeenCalled(); + }); + + it('FE-MOB-JSET-033: a journey that already draws its tracks switches them back off', async () => { + // The column is INTEGER, so what arrives over the wire is 1, not true. + setup({ show_trip_tracks: 1 }); + + const tracks = await screen.findByRole('switch', { name: /GPX/i }); + expect(tracks).toHaveAttribute('aria-checked', 'true'); + + fireEvent.click(tracks); + await waitFor(() => expect(updateJourney).toHaveBeenCalledWith(12, { show_trip_tracks: false })); + }); + + it('FE-MOB-JSET-034: a failing switch write says so and leaves the sheet open', async () => { + updateJourney.mockRejectedValueOnce(new Error('nope')); + const { props } = setup(); + + fireEvent.click(await screen.findByRole('switch', { name: /GPX/i })); + + await waitFor(() => expect(addToast).toHaveBeenCalledWith('Failed to save', 'error', undefined)); + expect(props.onRefresh).not.toHaveBeenCalled(); + expect(props.onClose).not.toHaveBeenCalled(); + }); });