From 75d23eb6aaa567d301701ea85307892d9fe8a0ef Mon Sep 17 00:00:00 2001 From: jubnl Date: Thu, 16 Apr 2026 15:27:13 +0200 Subject: [PATCH 1/7] fix(journey): keep page mounted during in-place journey refetch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit loadJourney previously set loading=true unconditionally, causing the JourneyDetailPage guard (if loading || !current) to unmount the entire page tree on every background refetch — entry saves, settings saves, trip link/unlink, contributor invite, delete, and WS realtime events all triggered the full-page spinner flash. Now loading is only toggled on cold loads (current?.id !== id). Warm refreshes replace current silently so the hero, sidebar, map, and timeline stay mounted throughout. Closes #673. --- client/src/store/journeyStore.test.ts | 41 +++++++++++++++++++++++++++ client/src/store/journeyStore.ts | 5 ++-- 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/client/src/store/journeyStore.test.ts b/client/src/store/journeyStore.test.ts index 7b1f6760..564969ec 100644 --- a/client/src/store/journeyStore.test.ts +++ b/client/src/store/journeyStore.test.ts @@ -314,6 +314,47 @@ describe('journeyStore', () => { expect(storedEntry?.photos[0].id).toBe(201); }); + // ── loadJourney silent refresh ─────────────────────────────────────────── + + it('FE-STORE-JOURNEY-016: loadJourney does not set loading when refreshing same journey', async () => { + const existing = buildJourneyDetail({ id: 5, title: 'Old' }); + useJourneyStore.setState({ current: existing, loading: false }); + + const loadingValues: boolean[] = []; + const unsub = useJourneyStore.subscribe(s => loadingValues.push(s.loading)); + + const refreshed = buildJourneyDetail({ id: 5, title: 'Refreshed' }); + server.use( + http.get('/api/journeys/5', () => HttpResponse.json(refreshed)) + ); + + await useJourneyStore.getState().loadJourney(5); + unsub(); + + expect(loadingValues.every(v => v === false)).toBe(true); + expect(useJourneyStore.getState().current?.title).toBe('Refreshed'); + }); + + it('FE-STORE-JOURNEY-017: loadJourney sets loading on cold load (different journey)', async () => { + const existing = buildJourneyDetail({ id: 5 }); + useJourneyStore.setState({ current: existing, loading: false }); + + const loadingValues: boolean[] = []; + const unsub = useJourneyStore.subscribe(s => loadingValues.push(s.loading)); + + const other = buildJourneyDetail({ id: 99 }); + server.use( + http.get('/api/journeys/99', () => HttpResponse.json(other)) + ); + + await useJourneyStore.getState().loadJourney(99); + unsub(); + + expect(loadingValues).toContain(true); + expect(useJourneyStore.getState().current?.id).toBe(99); + expect(useJourneyStore.getState().loading).toBe(false); + }); + // ── clear ──────────────────────────────────────────────────────────────── it('FE-STORE-JOURNEY-015: clear resets state', () => { diff --git a/client/src/store/journeyStore.ts b/client/src/store/journeyStore.ts index e0a8f22b..c0464133 100644 --- a/client/src/store/journeyStore.ts +++ b/client/src/store/journeyStore.ts @@ -124,7 +124,8 @@ export const useJourneyStore = create((set, get) => ({ }, loadJourney: async (id) => { - set({ loading: true, notFound: false }) + const cold = get().current?.id !== id + if (cold) set({ loading: true, notFound: false }) try { const data = await journeyApi.get(id) set({ current: data }) @@ -134,7 +135,7 @@ export const useJourneyStore = create((set, get) => ({ } throw err } finally { - set({ loading: false }) + if (cold) set({ loading: false }) } }, From 6c1a7954605d97c6797187141667b58f7ae31cba Mon Sep 17 00:00:00 2001 From: jubnl Date: Thu, 16 Apr 2026 15:32:56 +0200 Subject: [PATCH 2/7] fix(journey): paginate Immich picker and group photos by date The /search route was looping up to 20 pages server-side, returning a blob of up to 1000 photos with no hasMore flag, which prevented the client's existing ScrollTrigger infinite scroll from ever firing. Now the route proxies the client's page param directly to Immich and returns a single page plus hasMore, enabling full library browsing. The photo picker grid now groups photos by takenAt date (already present in every asset response) with a date label above each group, restoring the date-oriented browsing from V2. Closes #674. --- client/src/pages/JourneyDetailPage.tsx | 115 +++++++++++------- server/src/routes/memories/immich.ts | 14 +-- .../tests/integration/memories-immich.test.ts | 65 +++++++--- 3 files changed, 126 insertions(+), 68 deletions(-) diff --git a/client/src/pages/JourneyDetailPage.tsx b/client/src/pages/JourneyDetailPage.tsx index 9105a554..0eb56e05 100644 --- a/client/src/pages/JourneyDetailPage.tsx +++ b/client/src/pages/JourneyDetailPage.tsx @@ -1423,6 +1423,24 @@ function ScrollTrigger({ onVisible, loading }: { onVisible: () => void; loading: ) } +// ── Photo date grouping ─────────────────────────────────────────────────── + +function groupPhotosByDate(photos: any[]): { date: string; label: string; assets: any[] }[] { + const map = new Map() + for (const asset of photos) { + const key = asset.takenAt ? asset.takenAt.slice(0, 10) : '__unknown__' + if (!map.has(key)) map.set(key, []) + map.get(key)!.push(asset) + } + return [...map.entries()].map(([date, assets]) => ({ + date, + label: date === '__unknown__' + ? 'Unknown date' + : new Date(date + 'T00:00:00').toLocaleDateString(undefined, { year: 'numeric', month: 'long', day: 'numeric' }), + assets, + })) +} + // ── Provider Picker ─────────────────────────────────────────────────────── function ProviderPicker({ provider, userId, entries, trips, existingAssetIds, onClose, onAdd }: { @@ -1732,51 +1750,60 @@ function ProviderPicker({ provider, userId, entries, trips, existingAssetIds, on

) : ( -
- {photos.map((asset: any) => { - const isSelected = selected.has(asset.id) - const alreadyAdded = existingAssetIds.has(asset.id) - return ( -
!alreadyAdded && toggleAsset(asset.id)} - className={`relative aspect-square rounded-lg overflow-hidden ${ - alreadyAdded - ? 'opacity-40 cursor-not-allowed' - : isSelected - ? 'ring-2 ring-zinc-900 dark:ring-white ring-offset-2 dark:ring-offset-zinc-900 cursor-pointer' - : 'cursor-pointer' - }`} - > - { - const img = e.currentTarget - const original = `/api/integrations/memories/${provider}/assets/0/${asset.id}/${userId}/original` - if (!img.src.includes('/original')) img.src = original - }} - /> - {alreadyAdded && ( -
- -
- )} - {isSelected && !alreadyAdded && ( -
- -
- )} - {asset.city && ( -
-

{asset.city}

-
- )} +
+ {groupPhotosByDate(photos).map(group => ( +
+

+ {group.label} +

+
+ {group.assets.map((asset: any) => { + const isSelected = selected.has(asset.id) + const alreadyAdded = existingAssetIds.has(asset.id) + return ( +
!alreadyAdded && toggleAsset(asset.id)} + className={`relative aspect-square rounded-lg overflow-hidden ${ + alreadyAdded + ? 'opacity-40 cursor-not-allowed' + : isSelected + ? 'ring-2 ring-zinc-900 dark:ring-white ring-offset-2 dark:ring-offset-zinc-900 cursor-pointer' + : 'cursor-pointer' + }`} + > + { + const img = e.currentTarget + const original = `/api/integrations/memories/${provider}/assets/0/${asset.id}/${userId}/original` + if (!img.src.includes('/original')) img.src = original + }} + /> + {alreadyAdded && ( +
+ +
+ )} + {isSelected && !alreadyAdded && ( +
+ +
+ )} + {asset.city && ( +
+

{asset.city}

+
+ )} +
+ ) + })}
- ) - })} +
+ ))} {/* Infinite scroll trigger */} {hasMore && !selectedAlbum && }
diff --git a/server/src/routes/memories/immich.ts b/server/src/routes/memories/immich.ts index 87f3fa0f..9486234a 100644 --- a/server/src/routes/memories/immich.ts +++ b/server/src/routes/memories/immich.ts @@ -60,16 +60,12 @@ router.get('/browse', authenticate, async (req: Request, res: Response) => { router.post('/search', authenticate, async (req: Request, res: Response) => { const authReq = req as AuthRequest; - const { from, to, size } = req.body; + const { from, to, size, page } = req.body; + const pageNum = Math.max(1, Number(page) || 1); const pageSize = Math.min(Number(size) || 50, 200); - const allAssets: any[] = []; - for (let page = 1; page <= 20; page++) { - const result = await searchPhotos(authReq.user.id, from, to, page, pageSize); - if (result.error) return res.status(result.status!).json({ error: result.error }); - if (result.assets) allAssets.push(...result.assets); - if (!result.hasMore) break; - } - res.json({ assets: allAssets }); + const result = await searchPhotos(authReq.user.id, from, to, pageNum, pageSize); + if (result.error) return res.status(result.status!).json({ error: result.error }); + res.json({ assets: result.assets || [], hasMore: !!result.hasMore }); }); // ── Asset Details ────────────────────────────────────────────────────────── diff --git a/server/tests/integration/memories-immich.test.ts b/server/tests/integration/memories-immich.test.ts index eb19cd48..2b3cdefb 100644 --- a/server/tests/integration/memories-immich.test.ts +++ b/server/tests/integration/memories-immich.test.ts @@ -273,18 +273,19 @@ describe('Immich browse and search', () => { expect(res.body.buckets.length).toBeGreaterThan(0); }); - it('IMMICH-042 — POST /search returns mapped assets', async () => { + it('IMMICH-042 — POST /search returns mapped assets with hasMore flag', async () => { const { user } = createUser(testDb); setImmichCredentials(testDb, user.id, 'https://immich.example.com', 'test-api-key'); const res = await request(app) .post(`${IMMICH}/search`) .set('Cookie', authCookie(user.id)) - .send({}); + .send({ page: 1, size: 50 }); expect(res.status).toBe(200); expect(Array.isArray(res.body.assets)).toBe(true); expect(res.body.assets[0]).toMatchObject({ id: 'asset-search-1', city: 'Paris', country: 'France' }); + expect(typeof res.body.hasMore).toBe('boolean'); }); it('IMMICH-043 — POST /search when upstream throws returns 502', async () => { @@ -611,43 +612,77 @@ describe('Immich syncAlbumAssets', () => { // ── searchPhotos pagination safety ──────────────────────────────────────────── -describe('Immich searchPhotos pagination safety', () => { - it('IMMICH-090 — searchPhotos stops at page 20 when hasMore is always true', async () => { +describe('Immich searchPhotos pagination pass-through', () => { + it('IMMICH-090 — POST /search proxies client page param and returns hasMore', async () => { const { user } = createUser(testDb); setImmichCredentials(testDb, user.id, 'https://immich.example.com', 'test-api-key'); - // Return a full page of 1000 items on every call, so the loop would - // run indefinitely without the page > 20 safety check. + // Return a full page so hasMore=true (items.length >= size) const fullPageResponse = { ok: true, status: 200, headers: { get: () => null }, json: () => Promise.resolve({ assets: { - items: Array.from({ length: 1000 }, (_, i) => ({ - id: `asset-${i}`, + items: Array.from({ length: 50 }, (_, i) => ({ + id: `asset-p2-${i}`, fileCreatedAt: '2024-06-01T10:00:00.000Z', - exifInfo: { city: 'Paris', country: 'France' }, + exifInfo: { city: 'Berlin', country: 'Germany' }, })), }, }), body: null, } as any; - // Clear previous call history so the count only reflects this test vi.mocked(safeFetch).mockClear(); vi.mocked(safeFetch).mockResolvedValue(fullPageResponse); const res = await request(app) .post(`${IMMICH}/search`) .set('Cookie', authCookie(user.id)) - .send({}); + .send({ page: 2, size: 50 }); expect(res.status).toBe(200); expect(Array.isArray(res.body.assets)).toBe(true); - // 20 pages × 1000 items = 20000 assets total (safety limit) - expect(res.body.assets.length).toBe(20000); - // safeFetch should have been called exactly 20 times (the safety limit) - expect(vi.mocked(safeFetch)).toHaveBeenCalledTimes(20); + // Single page returned — not 20× aggregation + expect(res.body.assets.length).toBe(50); + expect(res.body.hasMore).toBe(true); + // Immich was called exactly once + expect(vi.mocked(safeFetch)).toHaveBeenCalledTimes(1); + // page=2 was forwarded to Immich + const callBody = JSON.parse(vi.mocked(safeFetch).mock.calls[0][1]!.body as string); + expect(callBody.page).toBe(2); + }); + + it('IMMICH-091 — POST /search returns hasMore=false on last page', async () => { + const { user } = createUser(testDb); + setImmichCredentials(testDb, user.id, 'https://immich.example.com', 'test-api-key'); + + // Partial page → hasMore=false + const partialPageResponse = { + ok: true, status: 200, + headers: { get: () => null }, + json: () => Promise.resolve({ + assets: { + items: Array.from({ length: 3 }, (_, i) => ({ + id: `asset-last-${i}`, + fileCreatedAt: '2024-06-01T10:00:00.000Z', + exifInfo: { city: 'Rome', country: 'Italy' }, + })), + }, + }), + body: null, + } as any; + + vi.mocked(safeFetch).mockResolvedValue(partialPageResponse); + + const res = await request(app) + .post(`${IMMICH}/search`) + .set('Cookie', authCookie(user.id)) + .send({ page: 5, size: 50 }); + + expect(res.status).toBe(200); + expect(res.body.assets.length).toBe(3); + expect(res.body.hasMore).toBe(false); }); }); From da70388f4b932c702726b13b2c74ba806a921ef0 Mon Sep 17 00:00:00 2001 From: jubnl Date: Thu, 16 Apr 2026 15:37:24 +0200 Subject: [PATCH 3/7] fix(journey): resolve Immich photos on public share by matching trek_photos.id validateShareTokenForPhoto was querying journey_photos by jp.id but the public page sends p.photo_id (trek_photos.id) in the URL. In a fresh database the IDs coincidentally match, masking the bug. In production instances with many Immich-synced photos the trek_photos autoincrement is far ahead of journey_photos, causing a 404 for every Immich photo on the public share page. Fix: change the lookup to jp.photo_id = ? so validation is keyed on trek_photos.id, which is what the client sends and what streamPhoto needs. Updated the test helper to return trekId and added a regression test that pre-populates trek_photos to produce diverging IDs. Closes #675. --- server/src/services/journeyShareService.ts | 2 +- .../unit/services/journeyShareService.test.ts | 33 +++++++++++++++++-- 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/server/src/services/journeyShareService.ts b/server/src/services/journeyShareService.ts index 46ef6926..c85621de 100644 --- a/server/src/services/journeyShareService.ts +++ b/server/src/services/journeyShareService.ts @@ -63,7 +63,7 @@ export function validateShareTokenForPhoto(token: string, photoId: number): { jo FROM journey_photos jp JOIN trek_photos tkp ON tkp.id = jp.photo_id JOIN journey_entries je ON jp.entry_id = je.id - WHERE jp.id = ? AND je.journey_id = ? + WHERE jp.photo_id = ? AND je.journey_id = ? `).get(photoId, row.journey_id) as any; if (!photo) return null; const journey = db.prepare('SELECT user_id FROM journeys WHERE id = ?').get(row.journey_id) as any; diff --git a/server/tests/unit/services/journeyShareService.test.ts b/server/tests/unit/services/journeyShareService.test.ts index 371e170e..8027e068 100644 --- a/server/tests/unit/services/journeyShareService.test.ts +++ b/server/tests/unit/services/journeyShareService.test.ts @@ -58,7 +58,7 @@ afterAll(() => { // -- Helpers ------------------------------------------------------------------ -/** Insert a journey_photos row and return its id. */ +/** Insert a trek_photos + journey_photos row and return the trek_photos id (used as photoId in public URLs). */ function insertJourneyPhoto( entryId: number, opts: { filePath?: string; assetId?: string; ownerId?: number } = {} @@ -70,11 +70,13 @@ function insertJourneyPhoto( VALUES (?, ?, ?, ?, ?) `).run(provider, opts.assetId ?? null, filePath, opts.ownerId ?? null, Date.now()); const trekId = trekResult.lastInsertRowid as number; - const result = testDb.prepare(` + testDb.prepare(` INSERT INTO journey_photos (entry_id, photo_id, caption, sort_order, created_at) VALUES (?, ?, NULL, 0, ?) `).run(entryId, trekId, Date.now()); - return result.lastInsertRowid as number; + // Return trek_photos.id — this is p.photo_id in the public API response + // and the value the client sends to /api/public/journey/:token/photos/:photoId/:kind + return trekId; } // -- Tests -------------------------------------------------------------------- @@ -237,6 +239,31 @@ describe('validateShareTokenForPhoto', () => { expect(result).not.toBeNull(); expect(result!.ownerId).toBe(user.id); }); + + it('JOURNEY-SHARE-016: resolves correctly when trek_photos.id differs from journey_photos.id (Immich bulk-sync scenario)', () => { + // Simulate a user who has many trek_photos from Immich syncs before adding a journey photo. + // trek_photos.id will be higher than journey_photos.id — the previous bug matched on jp.id + // instead of jp.photo_id, causing a 404 for Immich photos in public shares. + const { user } = createUser(testDb); + const journey = createJourney(testDb, user.id); + const entry = createJourneyEntry(testDb, journey.id, user.id); + + // Pre-populate trek_photos to push the autoincrement higher + for (let i = 0; i < 5; i++) { + testDb.prepare(`INSERT INTO trek_photos (provider, asset_id, owner_id, created_at) VALUES ('immich', ?, ?, ?)`).run(`bulk-asset-${i}`, user.id, Date.now()); + } + + // This trek_photos row gets a high id (e.g. 6) while journey_photos id will be 1 + const trekPhotoId = insertJourneyPhoto(entry.id, { assetId: 'journey-asset-xyz', ownerId: user.id }); + const { token } = createOrUpdateJourneyShareLink(journey.id, user.id, {}); + + // photoId = trek_photos.id (6), not journey_photos.id (1) + const result = validateShareTokenForPhoto(token, trekPhotoId); + + expect(result).not.toBeNull(); + expect(result!.ownerId).toBe(user.id); + expect(result!.journeyId).toBe(journey.id); + }); }); describe('validateShareTokenForAsset', () => { From 47b7678975859e5fd7fa1fe7bf8939c33c263d73 Mon Sep 17 00:00:00 2001 From: jubnl Date: Thu, 16 Apr 2026 15:45:37 +0200 Subject: [PATCH 4/7] fix(journey): remove backdropFilter from modal overlays to fix iOS Safari PWA white screen backdrop-filter: blur() on position:fixed elements is a known Safari iOS compositing failure in standalone (PWA) mode. When the GPU layer behind a fixed overlay is uninitialized, the blur samples white instead of the actual content, overriding the semi-transparent background and rendering a fully white screen that requires a force-close to escape. The JourneySettingsDialog (bottom-sheet on mobile) was most affected due to its items-end layout, but all five modal overlays in JourneyDetailPage had the same pattern. Removed backdropFilter from all five and bumped opacity from 0.6 to 0.75 to maintain visual separation. Closes #678. --- client/src/pages/JourneyDetailPage.tsx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/client/src/pages/JourneyDetailPage.tsx b/client/src/pages/JourneyDetailPage.tsx index 0eb56e05..9786de43 100644 --- a/client/src/pages/JourneyDetailPage.tsx +++ b/client/src/pages/JourneyDetailPage.tsx @@ -1565,7 +1565,7 @@ function ProviderPicker({ provider, userId, entries, trips, existingAssetIds, on : t('journey.picker.newGallery') return ( -
+
{/* Header */} @@ -2027,7 +2027,7 @@ function EntryEditor({ entry, journeyId, tripDates, galleryPhotos, onClose, onSa } return ( -
+
@@ -2411,7 +2411,7 @@ function AddTripDialog({ journeyId, existingTripIds, onClose, onAdded }: { } return ( -
+
@@ -2508,7 +2508,7 @@ function ContributorInviteDialog({ journeyId, existingUserIds, onClose, onInvite } return ( -
+
@@ -2765,7 +2765,7 @@ function JourneySettingsDialog({ journey, onClose, onSaved, onOpenInvite }: { } return ( -
{ if (e.target === e.currentTarget) e.preventDefault() }}> +
{ if (e.target === e.currentTarget) e.preventDefault() }}>
e.stopPropagation()}>
From 1b7ea2c87d3b400e5ee5186c26ef8fd8711b2346 Mon Sep 17 00:00:00 2001 From: jubnl Date: Thu, 16 Apr 2026 15:54:07 +0200 Subject: [PATCH 5/7] fix(journey): replace window.open with srcdoc iframe overlay for PDF preview Rewrites downloadJourneyBookPDF to render the preview in an in-page srcdoc iframe overlay instead of calling window.open(), which Safari iOS PWA blocks in async callbacks. Matches the existing TripPDF pattern. Fixes #679. --- .../components/PDF/JourneyBookPDF.test.tsx | 52 ++++++++++--------- client/src/components/PDF/JourneyBookPDF.tsx | 51 +++++++++++------- 2 files changed, 60 insertions(+), 43 deletions(-) diff --git a/client/src/components/PDF/JourneyBookPDF.test.tsx b/client/src/components/PDF/JourneyBookPDF.test.tsx index bb43e711..1e8c5810 100644 --- a/client/src/components/PDF/JourneyBookPDF.test.tsx +++ b/client/src/components/PDF/JourneyBookPDF.test.tsx @@ -1,8 +1,8 @@ // FE-COMP-JOURNEYPDF-001 to FE-COMP-JOURNEYPDF-006 // // JourneyBookPDF.tsx exports an async function `downloadJourneyBookPDF(journey)` -// that opens a new browser window and writes a full HTML document into it. -// It does NOT render a React component. Tests verify window.open behaviour. +// that renders a PDF preview in an srcdoc iframe overlay (Safari-safe pattern). +// Tests verify the overlay DOM structure and HTML content. import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest'; @@ -77,55 +77,57 @@ function buildJourney(overrides: Partial = {}): JourneyDetail { } as unknown as JourneyDetail; } -// ── Mock window.open ───────────────────────────────────────────────────────── +// ── Helpers to inspect the overlay ─────────────────────────────────────────── -let mockWindow: { - document: { write: ReturnType; close: ReturnType }; - focus: ReturnType; -}; +function getOverlay(): HTMLElement | null { + return document.getElementById('journey-pdf-overlay'); +} -beforeEach(() => { - mockWindow = { - document: { write: vi.fn(), close: vi.fn() }, - focus: vi.fn(), - }; - vi.spyOn(window, 'open').mockReturnValue(mockWindow as any); -}); +function getIframe(): HTMLIFrameElement | null { + return getOverlay()?.querySelector('iframe') ?? null; +} + +// ── Setup ──────────────────────────────────────────────────────────────────── afterEach(() => { + document.getElementById('journey-pdf-overlay')?.remove(); vi.restoreAllMocks(); }); // ── Tests ──────────────────────────────────────────────────────────────────── describe('downloadJourneyBookPDF', () => { - it('FE-COMP-JOURNEYPDF-001: opens a new window', async () => { + it('FE-COMP-JOURNEYPDF-001: appends overlay to document body', async () => { await downloadJourneyBookPDF(buildJourney()); - expect(window.open).toHaveBeenCalledWith('', '_blank'); + expect(getOverlay()).not.toBeNull(); + expect(document.body.contains(getOverlay())).toBe(true); }); - it('FE-COMP-JOURNEYPDF-002: writes HTML to the new window', async () => { + it('FE-COMP-JOURNEYPDF-002: overlay contains an iframe with srcdoc HTML', async () => { await downloadJourneyBookPDF(buildJourney()); - expect(mockWindow.document.write).toHaveBeenCalledTimes(1); - const html = mockWindow.document.write.mock.calls[0][0] as string; + const iframe = getIframe(); + expect(iframe).not.toBeNull(); + const html = iframe!.srcdoc; expect(html).toContain(''); expect(html).toContain(''); }); - it('FE-COMP-JOURNEYPDF-003: closes the document after writing', async () => { + it('FE-COMP-JOURNEYPDF-003: overlay has close and save buttons', async () => { await downloadJourneyBookPDF(buildJourney()); - expect(mockWindow.document.close).toHaveBeenCalledTimes(1); + const overlay = getOverlay()!; + expect(overlay.querySelector('#journey-pdf-close')).not.toBeNull(); + expect(overlay.querySelector('#journey-pdf-save')).not.toBeNull(); }); it('FE-COMP-JOURNEYPDF-004: HTML contains the journey title', async () => { await downloadJourneyBookPDF(buildJourney()); - const html = mockWindow.document.write.mock.calls[0][0] as string; + const html = getIframe()!.srcdoc; expect(html).toContain('Iceland Ring Road'); }); it('FE-COMP-JOURNEYPDF-005: HTML contains entry content', async () => { await downloadJourneyBookPDF(buildJourney()); - const html = mockWindow.document.write.mock.calls[0][0] as string; + const html = getIframe()!.srcdoc; expect(html).toContain('Golden Circle'); // Story text is rendered via markdown expect(html).toContain('An incredible day of geysers and waterfalls.'); @@ -137,8 +139,8 @@ describe('downloadJourneyBookPDF', () => { it('FE-COMP-JOURNEYPDF-006: handles empty entries gracefully', async () => { const journey = buildJourney({ entries: [] }); await downloadJourneyBookPDF(journey); - expect(window.open).toHaveBeenCalled(); - const html = mockWindow.document.write.mock.calls[0][0] as string; + expect(getOverlay()).not.toBeNull(); + const html = getIframe()!.srcdoc; expect(html).toContain('Iceland Ring Road'); // No entry pages, but cover and closing page are still present expect(html).toContain('Journey Book'); diff --git a/client/src/components/PDF/JourneyBookPDF.tsx b/client/src/components/PDF/JourneyBookPDF.tsx index 80d38333..97de76ec 100644 --- a/client/src/components/PDF/JourneyBookPDF.tsx +++ b/client/src/components/PDF/JourneyBookPDF.tsx @@ -249,23 +249,9 @@ export async function downloadJourneyBookPDF(journey: JourneyDetail) { .entry-photo-single, .entry-photo-duo, .entry-photo-trio { page-break-after: avoid; } } - .print-bar { - position: fixed; top: 0; left: 0; right: 0; z-index: 9999; - background: rgba(15,23,42,0.95); backdrop-filter: blur(12px); - padding: 12px 24px; display: flex; align-items: center; justify-content: center; gap: 12px; - } - .print-bar button { padding: 8px 24px; border-radius: 10px; font-size: 13px; font-weight: 600; cursor: pointer; font-family: inherit; border: none; } - .print-bar .btn-save { background: white; color: #0f172a; } - .print-bar .btn-close { background: rgba(255,255,255,0.1); color: rgba(255,255,255,0.7); border: 1px solid rgba(255,255,255,0.15); } - .print-bar .info { font-size: 11px; color: rgba(255,255,255,0.4); } -
- ${esc(journey.title)} · ${totalPages} pages - - -
@@ -299,8 +285,37 @@ export async function downloadJourneyBookPDF(journey: JourneyDetail) { ` - const win = window.open('', '_blank') - if (!win) return - win.document.write(html) - win.document.close() + // Render in a fixed overlay + srcdoc iframe — same pattern as TripPDF. + // This avoids window.open() which Safari iOS blocks in async callbacks + // and window.close() which doesn't work reliably in standalone PWA mode. + const overlay = document.createElement('div') + overlay.id = 'journey-pdf-overlay' + overlay.style.cssText = 'position:fixed;inset:0;background:rgba(0,0,0,0.75);z-index:9999;display:flex;align-items:center;justify-content:center;padding:8px;' + overlay.onclick = (e) => { if (e.target === overlay) overlay.remove() } + + const card = document.createElement('div') + card.style.cssText = 'width:100%;max-width:1100px;height:95vh;background:#fff;border-radius:12px;overflow:hidden;display:flex;flex-direction:column;box-shadow:0 20px 60px rgba(0,0,0,0.35);' + + const header = document.createElement('div') + header.style.cssText = 'display:flex;align-items:center;justify-content:space-between;padding:10px 20px;border-bottom:1px solid #e4e4e7;flex-shrink:0;background:#0f172a;' + header.innerHTML = ` + ${esc(journey.title)} · ${totalPages} pages +
+ + +
+ ` + + const iframe = document.createElement('iframe') + iframe.style.cssText = 'flex:1;width:100%;border:none;' + iframe.sandbox = 'allow-same-origin allow-modals' + iframe.srcdoc = html + + card.appendChild(header) + card.appendChild(iframe) + overlay.appendChild(card) + document.body.appendChild(overlay) + + header.querySelector('#journey-pdf-close')!.onclick = () => overlay.remove() + header.querySelector('#journey-pdf-save')!.onclick = () => { iframe.contentWindow?.print() } } From 84574020f26e4451cb3f5befe11a35a6254416bf Mon Sep 17 00:00:00 2001 From: jubnl Date: Thu, 16 Apr 2026 15:55:20 +0200 Subject: [PATCH 6/7] fix(journey): increase PDF preview button touch targets for mobile Raises button min-height to 44px and bumps padding/font-size to meet Apple HIG minimum touch-target guidelines on iOS PWA. Fixes #680. --- client/src/components/PDF/JourneyBookPDF.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/client/src/components/PDF/JourneyBookPDF.tsx b/client/src/components/PDF/JourneyBookPDF.tsx index 97de76ec..bdf4133a 100644 --- a/client/src/components/PDF/JourneyBookPDF.tsx +++ b/client/src/components/PDF/JourneyBookPDF.tsx @@ -297,12 +297,12 @@ export async function downloadJourneyBookPDF(journey: JourneyDetail) { card.style.cssText = 'width:100%;max-width:1100px;height:95vh;background:#fff;border-radius:12px;overflow:hidden;display:flex;flex-direction:column;box-shadow:0 20px 60px rgba(0,0,0,0.35);' const header = document.createElement('div') - header.style.cssText = 'display:flex;align-items:center;justify-content:space-between;padding:10px 20px;border-bottom:1px solid #e4e4e7;flex-shrink:0;background:#0f172a;' + header.style.cssText = 'display:flex;align-items:center;justify-content:space-between;padding:8px 16px;border-bottom:1px solid #e4e4e7;flex-shrink:0;background:#0f172a;' header.innerHTML = ` ${esc(journey.title)} · ${totalPages} pages
- - + +
` From d5d63aa9791a2242cb2a29b507a3a246df1c0606 Mon Sep 17 00:00:00 2001 From: jubnl Date: Thu, 16 Apr 2026 16:01:06 +0200 Subject: [PATCH 7/7] test(journey): fix FE-PAGE-JOURNEYDETAIL-027 flaky spinner assertion Pre-seed the store into loading state before render instead of relying on timing. RTL's render() flushes all microtasks via act(), so the MSW response lands before render() returns, leaving no observable loading window. --- client/src/pages/JourneyDetailPage.test.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/client/src/pages/JourneyDetailPage.test.tsx b/client/src/pages/JourneyDetailPage.test.tsx index 3cd44406..4b7aad9d 100644 --- a/client/src/pages/JourneyDetailPage.test.tsx +++ b/client/src/pages/JourneyDetailPage.test.tsx @@ -674,7 +674,10 @@ describe('JourneyDetailPage', () => { // ── FE-PAGE-JOURNEYDETAIL-027 ────────────────────────────────────────── describe('FE-PAGE-JOURNEYDETAIL-027: Shows loading spinner before data loads', () => { it('renders a spinner while journey data is loading', () => { - // Do NOT await the waitFor -- we check the loading state before data arrives + // Pre-seed the store into a loading state (current: null, loading: true). + // We can't rely on render() timing because RTL wraps in act(), which flushes + // all microtasks including the MSW response before render() returns. + useJourneyStore.setState({ loading: true, current: null }); render(); // The spinner has animate-spin class on a div const spinner = document.querySelector('.animate-spin');