diff --git a/client/src/components/Tours/TourGpxExportButton.tsx b/client/src/components/Tours/TourGpxExportButton.tsx index d62c0e985..30dc4c185 100644 --- a/client/src/components/Tours/TourGpxExportButton.tsx +++ b/client/src/components/Tours/TourGpxExportButton.tsx @@ -13,7 +13,7 @@ import { useTourGpxExport, type TourGpxExportTarget } from './useTourGpxExport'; */ export function TourGpxExportButton({ target }: { target: TourGpxExportTarget }) { const { t } = useTranslation(); - const { exportGpx, shareGpx, canShare, exporting } = useTourGpxExport(); + const { exportGpx, shareGpx, canShare, exporting } = useTourGpxExport(target); return ( <> @@ -54,7 +54,7 @@ export function TourGpxExportButton({ target }: { target: TourGpxExportTarget }) */ export function TourGpxExportBarButton({ target, unsaved }: { target: TourGpxExportTarget; unsaved: boolean }) { const { t } = useTranslation(); - const { exportGpx, shareGpx, canShare, exporting } = useTourGpxExport(); + const { exportGpx, shareGpx, canShare, exporting } = useTourGpxExport(unsaved ? null : target); return ( <> { await act(() => result.current.shareGpx(target)); expect(toast.error).toHaveBeenLastCalledWith('tours.gpxExport.shareFailed'); - share.shareFile.mockResolvedValueOnce('needs-gesture').mockResolvedValueOnce('needs-gesture'); - await act(() => result.current.shareGpx(target)); + share.shareFile.mockResolvedValueOnce('needs-gesture'); await act(() => result.current.shareGpx(target)); + expect(repo.exportGpx).toHaveBeenCalledTimes(1); + expect(toast.info).not.toHaveBeenCalled(); expect(toast.error).toHaveBeenCalledTimes(2); }); + it('a second tap shares again from the kept file, as a fresh file, and says nothing', async () => { + share.shareFile.mockResolvedValue('shared'); + const { result } = renderHook(() => useTourGpxExport()); + await act(() => result.current.shareGpx(target)); + await act(() => result.current.shareGpx(target)); + expect(repo.exportGpx).toHaveBeenCalledTimes(1); + expect(share.shareFile).toHaveBeenCalledTimes(2); + const [first, second] = share.shareFile.mock.calls.map((call) => call[0] as File); + expect(second).not.toBe(first); + expect(second).toEqual(gpxFile('Kramer-über.gpx')); + expect(toast.info).not.toHaveBeenCalled(); + expect(toast.error).not.toHaveBeenCalled(); + }); + + it('fetches the file ahead, so the tap opens the sheet before anything is awaited', async () => { + share.canShareFile.mockReturnValue(true); + share.shareFile.mockResolvedValue('shared'); + const { result } = renderHook(() => useTourGpxExport(target)); + await act(async () => undefined); + expect(repo.exportGpx).toHaveBeenCalledWith(1, 7, expect.any(AbortSignal)); + act(() => { + void result.current.shareGpx(target); + }); + // Called in the same turn as the tap, not after a network round trip. + expect(share.shareFile).toHaveBeenCalledWith(gpxFile('Kramer-über.gpx'), 'Kramer'); + expect(repo.exportGpx).toHaveBeenCalledTimes(1); + await act(async () => undefined); + expect(toast.error).not.toHaveBeenCalled(); + }); + + it('lets a tap be while the sheet is still open, and a busy browser is no failure', async () => { + let close: (value: unknown) => void = () => undefined; + share.shareFile.mockImplementationOnce(() => new Promise((resolve) => (close = resolve))); + const { result } = renderHook(() => useTourGpxExport()); + await act(async () => { + void result.current.shareGpx(target); + await new Promise((resolve) => setTimeout(resolve, 0)); + }); + expect(share.shareFile).toHaveBeenCalledTimes(1); + await act(() => result.current.shareGpx(target)); + expect(share.shareFile).toHaveBeenCalledTimes(1); + await act(async () => close('cancelled')); + + share.shareFile.mockResolvedValueOnce('busy'); + await act(() => result.current.shareGpx(target)); + expect(share.shareFile).toHaveBeenCalledTimes(2); + expect(toast.info).not.toHaveBeenCalled(); + expect(toast.error).not.toHaveBeenCalled(); + }); + + it('a failed fetch ahead is quiet, and the tap fetches itself and says why', async () => { + share.canShareFile.mockReturnValue(true); + repo.exportGpx.mockRejectedValueOnce(new Error('network')).mockRejectedValueOnce(refused(422)); + const { result } = renderHook(() => useTourGpxExport(target)); + await act(async () => undefined); + expect(toast.error).not.toHaveBeenCalled(); + await act(() => result.current.shareGpx(target)); + expect(repo.exportGpx).toHaveBeenCalledTimes(2); + expect(share.shareFile).not.toHaveBeenCalled(); + expect(toast.error).toHaveBeenCalledWith('tours.gpxExport.noRoute'); + }); + + it('fetches nothing ahead without the share, and drops the kept file when the tour changes', async () => { + const { unmount } = renderHook(() => useTourGpxExport(target)); + await act(async () => undefined); + expect(repo.exportGpx).not.toHaveBeenCalled(); + unmount(); + + share.canShareFile.mockReturnValue(true); + share.shareFile.mockResolvedValue('shared'); + const { result, rerender } = renderHook(({ prefetch }) => useTourGpxExport(prefetch), { + initialProps: { prefetch: target as typeof target | null }, + }); + await act(async () => undefined); + rerender({ prefetch: null }); + const signal = repo.exportGpx.mock.calls[0][2] as AbortSignal; + expect(signal.aborted).toBe(true); + await act(() => result.current.shareGpx(target)); + expect(repo.exportGpx).toHaveBeenCalledTimes(2); + }); + it('a kept file belongs to its tour: another tour is fetched afresh', async () => { share.shareFile.mockResolvedValueOnce('needs-gesture').mockResolvedValueOnce('shared'); const { result } = renderHook(() => useTourGpxExport()); diff --git a/client/src/components/Tours/useTourGpxExport.ts b/client/src/components/Tours/useTourGpxExport.ts index ec5548f8b..55c5c16c2 100644 --- a/client/src/components/Tours/useTourGpxExport.ts +++ b/client/src/components/Tours/useTourGpxExport.ts @@ -3,11 +3,21 @@ import { useCallback, useEffect, useRef, useState } from 'react'; import { useTranslation } from '../../i18n'; import { tourRepo } from '../../repo/tourRepo'; import { downloadBlob } from '../../utils/fileDownload'; -import { canShareFile, shareFile } from '../../utils/fileShare'; +import { canShareFile, shareFile, type FileShareOutcome } from '../../utils/fileShare'; import { useToast } from '../shared/Toast'; const GPX_TYPE = 'application/gpx+xml'; +/** A tour's GPX file kept for the next share, as the bytes and the name the server gave them. */ +interface PreparedFile { + placeId: number; + blob: Blob; + name: string; +} + +/** The file's name when the server gave none: the tour's, or plain "tour". */ +const fallbackName = (name: string) => `${name.trim() || 'tour'}.gpx`; + /** What a tour's GPX export needs to know about the tour. */ export interface TourGpxExportTarget { tripId: number | string; @@ -22,18 +32,52 @@ export interface TourGpxExportTarget { * so an unsaved edit in the planner is not in it. One export at a time per * caller; a dialog closed mid-download cancels it. Nothing about the tour * changes either way. + * + * The share sheet only opens from inside the tap that asks for it, and a tap + * that first waits on the network has often run out by the time the file is + * there. So where sharing is offered, the file of `prefetch` (the tour on + * screen, null while it has unsaved changes) is fetched ahead, quietly, and a + * tap hands it to the sheet before anything is awaited. A file a tap had to + * fetch is kept for the next one the same way. */ -export function useTourGpxExport() { +export function useTourGpxExport(prefetch: TourGpxExportTarget | null = null) { const { t } = useTranslation(); const toast = useToast(); const [exportingId, setExportingId] = useState(null); const abortRef = useRef(null); - // A file fetched for a share the browser then refused for want of a fresh tap, kept for the next one. - const readyRef = useRef<{ placeId: number; file: File } | null>(null); + // The tour's file at hand for a share, so the tap opens the sheet without waiting on the network. + const preparedRef = useRef(null); + // An open share sheet: a second tap while it is open would only make the browser refuse. + const sharingRef = useRef(false); const [canShare] = useState(() => canShareFile('tour.gpx', GPX_TYPE)); useEffect(() => () => abortRef.current?.abort(), []); + const prefetchTripId = prefetch?.tripId ?? null; + const prefetchPlaceId = prefetch?.placeId ?? null; + const prefetchName = prefetch?.name ?? ''; + useEffect(() => { + if (!canShare || prefetchTripId === null || prefetchPlaceId === null) return; + const controller = new AbortController(); + tourRepo.exportGpx(prefetchTripId, prefetchPlaceId, controller.signal).then( + (result) => { + if (controller.signal.aborted || !('blob' in result)) return; + preparedRef.current = { + placeId: prefetchPlaceId, + blob: result.blob, + name: result.filename ?? fallbackName(prefetchName), + }; + }, + // Quiet on purpose: a tap then fetches the file itself and says why it could not. + () => undefined + ); + return () => { + controller.abort(); + // Another tour, or the same one with changes the kept file does not hold. + preparedRef.current = null; + }; + }, [canShare, prefetchTripId, prefetchPlaceId, prefetchName]); + /** The tour's file from the server, or null when there is none to hand over, said in a toast. */ const fetchFile = useCallback( async ({ tripId, placeId, name }: TourGpxExportTarget): Promise => { @@ -47,7 +91,7 @@ export function useTourGpxExport() { toast.info(t('tours.gpxExport.offline')); return null; } - return new File([result.blob], result.filename ?? `${name.trim() || 'tour'}.gpx`, { type: GPX_TYPE }); + return new File([result.blob], result.filename ?? fallbackName(name), { type: GPX_TYPE }); } catch (error: unknown) { if (controller.signal.aborted) return null; const noRoute = isAxiosError(error) && error.response?.status === 422; @@ -71,14 +115,22 @@ export function useTourGpxExport() { const shareGpx = useCallback( async (target: TourGpxExportTarget) => { - const ready = readyRef.current?.placeId === target.placeId ? readyRef.current.file : null; - readyRef.current = null; - // A file at hand goes to the sheet before anything is awaited, inside the tap that asked. - const file = ready ?? (await fetchFile(target)); + if (sharingRef.current) return; + const prepared = preparedRef.current?.placeId === target.placeId ? preparedRef.current : null; + // A fresh File for every sheet, so no share is handed one an earlier sheet still holds. + // It goes to the sheet before anything is awaited, inside the tap that asked. + const file = prepared ? new File([prepared.blob], prepared.name, { type: GPX_TYPE }) : await fetchFile(target); if (!file) return; - const outcome = await shareFile(file, target.name); - if (outcome === 'needs-gesture' && !ready) { - readyRef.current = { placeId: target.placeId, file }; + if (!prepared) preparedRef.current = { placeId: target.placeId, blob: file, name: file.name }; + sharingRef.current = true; + let outcome: FileShareOutcome; + try { + outcome = await shareFile(file, target.name); + } finally { + sharingRef.current = false; + } + // A closed sheet is the user's choice and a sheet still open is no failure: neither says anything. + if (outcome === 'needs-gesture' && !prepared) { toast.info(t('tours.gpxExport.shareReady')); } else if (outcome === 'failed' || outcome === 'needs-gesture') { toast.error(t('tours.gpxExport.shareFailed')); diff --git a/client/src/utils/fileShare.test.ts b/client/src/utils/fileShare.test.ts index 481c866c4..ab57fd918 100644 --- a/client/src/utils/fileShare.test.ts +++ b/client/src/utils/fileShare.test.ts @@ -41,13 +41,15 @@ describe('shareFile (IO-01)', () => { expect(share).toHaveBeenCalledWith({ files: [file], title: 'Kramer' }); }); - it('tells a closed sheet, a refused tap and any other failure apart', async () => { + it('tells a closed sheet, a refused tap, an open sheet and any other failure apart', async () => { const share = vi.fn(); stubShare(share, () => true); share.mockRejectedValueOnce(new DOMException('closed', 'AbortError')); await expect(shareFile(gpx(), 'Kramer')).resolves.toBe('cancelled'); share.mockRejectedValueOnce(new DOMException('no activation', 'NotAllowedError')); await expect(shareFile(gpx(), 'Kramer')).resolves.toBe('needs-gesture'); + share.mockRejectedValueOnce(new DOMException('earlier share still open', 'InvalidStateError')); + await expect(shareFile(gpx(), 'Kramer')).resolves.toBe('busy'); share.mockRejectedValueOnce(new DOMException('bad data', 'DataError')); await expect(shareFile(gpx(), 'Kramer')).resolves.toBe('failed'); share.mockRejectedValueOnce(new Error('odd')); diff --git a/client/src/utils/fileShare.ts b/client/src/utils/fileShare.ts index bde9812e3..8819f2c53 100644 --- a/client/src/utils/fileShare.ts +++ b/client/src/utils/fileShare.ts @@ -20,9 +20,11 @@ export function canShareFile(name: string, type: string): boolean { /** * How a share ended: `cancelled` when the sheet was closed without a target, * `needs-gesture` when the browser refused because the tap that asked lies too - * far back (Safari, after the file had to be fetched first), `failed` otherwise. + * far back (Safari, after the file had to be fetched first), `busy` when an + * earlier sheet is still open (the browser's InvalidStateError), `failed` + * otherwise. */ -export type FileShareOutcome = 'shared' | 'cancelled' | 'needs-gesture' | 'failed'; +export type FileShareOutcome = 'shared' | 'cancelled' | 'needs-gesture' | 'busy' | 'failed'; /** Opens the share sheet with the file; call it straight from the tap that asks for it. */ export async function shareFile(file: File, title: string): Promise { @@ -32,6 +34,7 @@ export async function shareFile(file: File, title: string): Promise