From 31061751b019c471130b4f736bcd1e0e949f83fc Mon Sep 17 00:00:00 2001 From: Maurice Date: Tue, 8 Sep 2026 14:39:49 +0200 Subject: [PATCH] fix(client): sort the picker grid in a memo again, not inside the fetches Sorting inside searchPhotos and loadAlbumPhotos looks equivalent and is not. contextLocation is live editor state. The place search and "use my location" write it while the picker stays mounted, and the picker key is only provider plus date, so nothing remounts it. Open an entry with no location, let the photos load, then search for Rome: the caption flips to nearest-first and the grid does not move. The appending search made it worse than that. It sorted only the page it had just fetched before appending, so from page two on the grid was a sequence of sorted blocks rather than a sorted list, and a photo from page two landed under a day heading page one had already closed. FE-JRN-PICKER-021 pins it and is red without the memo. The video badge also gets z-10 and pointer-events-none. It shares a stacking context with the city gradient that follows it, so the later sibling was winning on the geotagged tiles; pointer-events-none is what the eight existing video overlays in the journey code all set. --- .../JourneyDetailPageProviderPicker.test.tsx | 35 ++++++++++++++++++- .../JourneyDetailPageProviderPicker.tsx | 25 +++++++++---- 2 files changed, 52 insertions(+), 8 deletions(-) diff --git a/client/src/components/Journey/JourneyDetailPageProviderPicker.test.tsx b/client/src/components/Journey/JourneyDetailPageProviderPicker.test.tsx index ed202c6e4..b3ecb7c69 100644 --- a/client/src/components/Journey/JourneyDetailPageProviderPicker.test.tsx +++ b/client/src/components/Journey/JourneyDetailPageProviderPicker.test.tsx @@ -1,4 +1,4 @@ -// FE-JRN-PICKER-001 to FE-JRN-PICKER-020 +// FE-JRN-PICKER-001 to FE-JRN-PICKER-021 import { describe, it, expect, beforeEach, vi } from 'vitest' import { http, HttpResponse, delay } from 'msw' @@ -324,6 +324,39 @@ describe('ProviderPicker', () => { expect(range.to.endsWith('-09')).toBe(true) }) + it('FE-JRN-PICKER-021: a location picked after the photos loaded reorders the grid', async () => { + // contextLocation is live editor state: the place search and "use my location" + // write it while this picker stays mounted, and the key is only provider plus + // date. Sorting inside the fetch instead of in a memo leaves the grid frozen + // while the caption above it already claims nearest-first. + searchReturns([ + asset('rome', { lat: 41.9, lng: 12.5, takenAt: '2026-03-15T09:00:00.000Z' }), + asset('helsinki', { lat: 60.17, lng: 24.94, takenAt: '2026-03-15T11:00:00.000Z' }), + ]) + const props = { + provider: 'immich', + userId: 42, + entries, + trips, + existingAssetIds: new Set(), + onClose: vi.fn(), + onAdd: vi.fn(async () => {}), + } as React.ComponentProps + const { container, rerender } = render() + await screen.findByText('March 15, 2026') + + const order = () => Array.from(container.querySelectorAll('img')) + .map(img => (img.getAttribute('src') || '').split('/assets/0/')[1]?.split('/')[0]) + .filter(Boolean) + + // Nothing to be near yet, so newest first. + expect(order()).toEqual(['helsinki', 'rome']) + + rerender() + + await waitFor(() => expect(order()).toEqual(['rome', 'helsinki'])) + }) + it('FE-JRN-PICKER-020: closes through the header button and the backdrop', async () => { const user = userEvent.setup() const { container, onClose } = mountPicker() diff --git a/client/src/components/Journey/JourneyDetailPageProviderPicker.tsx b/client/src/components/Journey/JourneyDetailPageProviderPicker.tsx index c194db067..16c7191ab 100644 --- a/client/src/components/Journey/JourneyDetailPageProviderPicker.tsx +++ b/client/src/components/Journey/JourneyDetailPageProviderPicker.tsx @@ -95,7 +95,7 @@ export function ProviderPicker({ setSearchPage(page); try { const data = await memoriesApi.search(provider, { from, to, page, size: 50 }, signal); - const assets = sortProviderPhotos(data.assets || [], contextLocation); + const assets = data.assets || []; setPhotos((prev) => (append ? [...prev, ...assets] : assets)); setHasMore(!!data.hasMore); } catch { @@ -121,7 +121,7 @@ export function ProviderPicker({ setHasMore(false); try { const assets = (await memoriesApi.albumPhotos(provider, album.id, album.passphrase, signal)).assets || []; - setPhotos(sortProviderPhotos(assets, contextLocation)); + setPhotos(assets); } catch { /* ignore */ } @@ -151,6 +151,17 @@ export function ProviderPicker({ if (customFrom && customTo) searchPhotos(customFrom, customTo); }; + // Sorted here rather than in the two fetches above. contextLocation is live + // editor state: the place search and "use my location" write it while this + // picker stays mounted, and its key is only provider plus date, so a fetch-time + // sort would leave the grid frozen while the caption already says "near Rome". + // It also keeps the ranking global instead of per page, which a sort inside + // the appending search cannot do. + const sortedPhotos = useMemo( + () => sortProviderPhotos(photos, contextLocation), + [photos, contextLocation?.lat, contextLocation?.lng], + ); + const toggleAsset = (id: string) => { setSelected((prev) => { const next = new Map(prev); @@ -411,9 +422,9 @@ export function ProviderPicker({ {/* Select all bar — sticky above grid */} {!loading && - photos.length > 0 && + sortedPhotos.length > 0 && (() => { - const selectable = photos.filter((a: any) => !existingAssetIds.has(a.id)); + const selectable = sortedPhotos.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 ( @@ -461,7 +472,7 @@ export function ProviderPicker({
- ) : photos.length === 0 ? ( + ) : sortedPhotos.length === 0 ? (

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

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

@@ -520,7 +531,7 @@ export function ProviderPicker({

)} {asset.mediaType === 'video' && ( -
+
)}