From d609aefe8f66d453fc9faecf5aeb12e05f4a1cef Mon Sep 17 00:00:00 2001 From: xvilo Date: Mon, 7 Sep 2026 12:38:43 +0900 Subject: [PATCH] fix(client): keep Immich picker photo order stable and chronological by default sortProviderPhotos previously passed photos through unchanged whenever there was no valid journey location, leaving order entirely up to Immich's search API (not guaranteed chronological). It now falls back to newest-taken-first. The picker also re-sorted the entire accumulated photo list on every render, so loading another page could reorder tiles already on screen when a location context was set -- newly loaded photos could land above the fold instead of at the bottom, making it look like nothing happened. Each fetched batch (search page or album) is now sorted once at fetch time and appended, so earlier pages never get reshuffled by later ones. --- .../JourneyDetailPageProviderPicker.tsx | 18 +++++++----------- .../JourneyDetailPage.helpers.test.ts | 10 +++++++++- .../journeyDetail/JourneyDetailPage.helpers.ts | 18 ++++++++++++++++-- 3 files changed, 32 insertions(+), 14 deletions(-) diff --git a/client/src/components/Journey/JourneyDetailPageProviderPicker.tsx b/client/src/components/Journey/JourneyDetailPageProviderPicker.tsx index fd4200b03..d74af8777 100644 --- a/client/src/components/Journey/JourneyDetailPageProviderPicker.tsx +++ b/client/src/components/Journey/JourneyDetailPageProviderPicker.tsx @@ -94,7 +94,7 @@ export function ProviderPicker({ setSearchPage(page); try { const data = await memoriesApi.search(provider, { from, to, page, size: 50 }, signal); - const assets = data.assets || []; + const assets = sortProviderPhotos(data.assets || [], contextLocation); setPhotos((prev) => (append ? [...prev, ...assets] : assets)); setHasMore(!!data.hasMore); } catch { @@ -119,7 +119,8 @@ export function ProviderPicker({ setPhotos([]); setHasMore(false); try { - setPhotos((await memoriesApi.albumPhotos(provider, album.id, album.passphrase, signal)).assets || []); + const assets = (await memoriesApi.albumPhotos(provider, album.id, album.passphrase, signal)).assets || []; + setPhotos(sortProviderPhotos(assets, contextLocation)); } catch { /* ignore */ } @@ -149,11 +150,6 @@ export function ProviderPicker({ if (customFrom && customTo) searchPhotos(customFrom, customTo); }; - const sortedPhotos = useMemo( - () => sortProviderPhotos(photos, contextLocation), - [photos, contextLocation?.lat, contextLocation?.lng] - ); - const toggleAsset = (id: string) => { setSelected((prev) => { const next = new Map(prev); @@ -414,9 +410,9 @@ export function ProviderPicker({ {/* Select all bar — sticky above grid */} {!loading && - sortedPhotos.length > 0 && + photos.length > 0 && (() => { - const selectable = sortedPhotos.filter((a: any) => !existingAssetIds.has(a.id)); + const selectable = photos.filter((a: any) => !existingAssetIds.has(a.id)); const allSelected = selectable.length > 0 && selectable.every((a: any) => selected.has(a.id)); if (selectable.length === 0) return null; return ( @@ -464,7 +460,7 @@ export function ProviderPicker({
- ) : sortedPhotos.length === 0 ? ( + ) : photos.length === 0 ? (

{filter === 'trip' && !tripRange.from @@ -474,7 +470,7 @@ export function ProviderPicker({

) : (
- {groupPhotosByDate(sortedPhotos).map((group) => ( + {groupPhotosByDate(photos).map((group) => (
{(!embedded || filter !== 'day') && (

diff --git a/client/src/pages/journeyDetail/JourneyDetailPage.helpers.test.ts b/client/src/pages/journeyDetail/JourneyDetailPage.helpers.test.ts index 5b3eecad9..42258516e 100644 --- a/client/src/pages/journeyDetail/JourneyDetailPage.helpers.test.ts +++ b/client/src/pages/journeyDetail/JourneyDetailPage.helpers.test.ts @@ -16,7 +16,15 @@ describe('Journey provider photo ranking', () => { ]); }); - it('preserves provider order when the selected entry has no valid location', () => { + it('falls back to newest-taken-first when the selected entry has no valid location', () => { + const photos = [ + { id: 'older', takenAt: '2026-03-14T09:00:00Z' }, + { id: 'newer', takenAt: '2026-03-16T09:00:00Z' }, + ]; + expect(sortProviderPhotos(photos, { lat: 200, lng: 12 }).map((photo) => photo.id)).toEqual(['newer', 'older']); + }); + + it('keeps original relative order for photos without a valid location and without takenAt', () => { const photos = [{ id: 'first' }, { id: 'second' }]; expect(sortProviderPhotos(photos, { lat: 200, lng: 12 }).map((photo) => photo.id)).toEqual(['first', 'second']); }); diff --git a/client/src/pages/journeyDetail/JourneyDetailPage.helpers.ts b/client/src/pages/journeyDetail/JourneyDetailPage.helpers.ts index 11c6857da..607c2fe69 100644 --- a/client/src/pages/journeyDetail/JourneyDetailPage.helpers.ts +++ b/client/src/pages/journeyDetail/JourneyDetailPage.helpers.ts @@ -132,10 +132,24 @@ export function distanceBetweenGeoPoints(a: GeoPoint, b: GeoPoint): number { /** * Keep every asset, but put assets with usable GPS nearest to the selected - * Journey location. The original provider order is the stable fallback. + * Journey location. Without a usable location, fall back to newest-taken-first + * (provider search order isn't guaranteed chronological), with a stable index + * tiebreak so photos missing `takenAt` keep a deterministic relative order. */ export function sortProviderPhotos(photos: T[], location?: GeoPoint | null): T[] { - if (!isValidGeoPoint(location)) return photos; + if (!isValidGeoPoint(location)) { + return photos + .map((photo, index) => ({ photo, index })) + .sort((a, b) => { + const at = a.photo.takenAt; + const bt = b.photo.takenAt; + if (!at && !bt) return a.index - b.index; + if (!at) return 1; + if (!bt) return -1; + return bt.localeCompare(at) || a.index - b.index; + }) + .map((item) => item.photo); + } return photos .map((photo, index) => ({