mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
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.
This commit is contained in:
Maurice
@@ -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(
|
||||
<JourneyMap checkins={[]} entries={entriesWithCoords} tracks={[track({ points: [[10, 10], [11, 11]] })]} />
|
||||
);
|
||||
|
||||
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(<JourneyMap checkins={[]} entries={entriesWithoutCoords} tracks={[track()]} />);
|
||||
|
||||
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
|
||||
|
||||
@@ -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<AvailableTrip[]>([])
|
||||
const [linkingTripId, setLinkingTripId] = useState<number | null>(null)
|
||||
const [shareLink, setShareLink] = useState<ShareLink | null>(null)
|
||||
const [savingTracks, setSavingTracks] = useState(false)
|
||||
const [copied, setCopied] = useState(false)
|
||||
const coverRef = useRef<HTMLInputElement>(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) */}
|
||||
<div className={`${eyebrow} mb-[6px] mt-[14px]`}>{t('journey.settings.tracks')}</div>
|
||||
<div className="flex items-center gap-[11px] rounded-[14px] bg-[color:var(--m-ic)] px-3 py-[10px]">
|
||||
<Route size={16} strokeWidth={2} className="flex-none text-m-muted" />
|
||||
<div className="min-w-0 flex-1">
|
||||
<div className="text-[0.8125rem] font-bold">{t('journey.settings.showTripTracks')}</div>
|
||||
<div className="font-geist text-[0.625rem] text-m-muted">{t('journey.settings.showTripTracksHint')}</div>
|
||||
</div>
|
||||
<MToggle
|
||||
checked={!!journey.show_trip_tracks}
|
||||
onChange={handleTracksToggle}
|
||||
disabled={savingTracks}
|
||||
ariaLabel={t('journey.settings.showTripTracks')}
|
||||
/>
|
||||
</div>
|
||||
|
||||
{/* Synced trips */}
|
||||
<div className={`${eyebrow} mb-[6px] mt-[14px]`}>{t('journey.detail.syncedTrips')}</div>
|
||||
{journey.trips.map(trip => (
|
||||
|
||||
@@ -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' ? (
|
||||
<div className="flex items-center gap-2">
|
||||
{galleryProviders.map(p => (
|
||||
<button type="button"
|
||||
key={p.id}
|
||||
onClick={() => galleryBrowseRef.current?.(p.id)}
|
||||
className="inline-flex items-center gap-1.5 h-9 px-4 rounded-full text-[13px] font-semibold transition-transform hover:-translate-y-0.5"
|
||||
style={{ background: 'var(--vg-ink)', color: 'var(--vg-bg)' }}
|
||||
>
|
||||
<Image size={16} strokeWidth={2.4} />
|
||||
{p.name}
|
||||
</button>
|
||||
))}
|
||||
<button type="button"
|
||||
onClick={() => galleryUploadRef.current?.()}
|
||||
className="inline-flex items-center gap-1.5 h-9 px-4 rounded-full text-[13px] font-semibold transition-transform hover:-translate-y-0.5"
|
||||
style={{ background: 'var(--vg-ink)', color: 'var(--vg-bg)' }}
|
||||
>
|
||||
<Plus size={16} strokeWidth={2.4} />
|
||||
{t('common.upload')}
|
||||
</button>
|
||||
</div>
|
||||
) : null
|
||||
|
||||
return (
|
||||
<div className="min-h-screen bg-zinc-50 dark:bg-zinc-950">
|
||||
<Navbar />
|
||||
@@ -346,29 +373,7 @@ function JourneyDetailPageDesktop() {
|
||||
{t('journey.detail.addEntry')}
|
||||
</button>
|
||||
)}
|
||||
{canEditEntries && view === 'gallery' && (
|
||||
<div className="flex items-center gap-2">
|
||||
{galleryProviders.map(p => (
|
||||
<button type="button"
|
||||
key={p.id}
|
||||
onClick={() => galleryBrowseRef.current?.(p.id)}
|
||||
className="inline-flex items-center gap-1.5 h-9 px-4 rounded-full text-[13px] font-semibold transition-transform hover:-translate-y-0.5"
|
||||
style={{ background: 'var(--vg-ink)', color: 'var(--vg-bg)' }}
|
||||
>
|
||||
<Image size={16} strokeWidth={2.4} />
|
||||
{p.name}
|
||||
</button>
|
||||
))}
|
||||
<button type="button"
|
||||
onClick={() => galleryUploadRef.current?.()}
|
||||
className="inline-flex items-center gap-1.5 h-9 px-4 rounded-full text-[13px] font-semibold transition-transform hover:-translate-y-0.5"
|
||||
style={{ background: 'var(--vg-ink)', color: 'var(--vg-bg)' }}
|
||||
>
|
||||
<Plus size={16} strokeWidth={2.4} />
|
||||
{t('common.upload')}
|
||||
</button>
|
||||
</div>
|
||||
)}
|
||||
{!isMobileChromeless && galleryActions}
|
||||
</div>
|
||||
|
||||
{/* 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 && (
|
||||
<div className="flex justify-end mb-4">{galleryActions}</div>
|
||||
)}
|
||||
<GalleryView
|
||||
onRegisterUpload={(fn) => { galleryUploadRef.current = fn }}
|
||||
onRegisterProviders={(providers, browse) => { setGalleryProviders(providers); galleryBrowseRef.current = browse }}
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<string, unknown>): void {
|
||||
server.use(http.get('/api/journeys/7', () => HttpResponse.json(detail)));
|
||||
}
|
||||
|
||||
function buildTrack(over: Partial<JourneyTrack> = {}): 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<typeof useJourneyDetail>;
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user