From d118423570cc05dccbd455a1fad294a2ecb34e65 Mon Sep 17 00:00:00 2001 From: Maurice Date: Fri, 9 Oct 2026 17:45:00 +0200 Subject: [PATCH] fix(journey): the desktop gallery Upload button shows the files counting up again Uploading photos into a journey gallery on desktop gave no sign that anything was happening until the toast at the end; the Upload button stayed clickable the whole time. Up to v4.0.0 the button sat inside the gallery and showed a spinner with "Uploading done/total" while it was disabled. When the button moved up into the page header (onRegisterUpload) that indicator was dropped, and the count kept being tracked with no reader. useGalleryUpload now also returns the progress it tracks, the gallery hands it up through onUploadProgress into useJourneyDetail, and the page's Upload button is disabled and shows the spinner with journey.editor.uploadingProgress while the files go, as it did before. Red before the fix: src/pages/JourneyDetailPage.wiring.test.tsx FE-JRN-DETWIRE-043 failed with "expected undefined to be [Function Mock]": the gallery was never given an onUploadProgress callback and the button never showed a count. FE-JRN-GALLERY-021 and FE-JRN-GALLERYUPLOAD-007 cover the gallery and the hook. No existing assertions changed. --- client/scripts/eslint-baseline.json | 2 +- .../JourneyDetailPageGalleryView.test.tsx | 24 ++++++++++++--- .../Journey/JourneyDetailPageGalleryView.tsx | 7 +++-- .../Journey/useGalleryUpload.test.ts | 30 ++++++++++++++++++- .../components/Journey/useGalleryUpload.ts | 7 +++-- client/src/pages/JourneyDetailPage.tsx | 20 ++++++++++--- .../pages/JourneyDetailPage.wiring.test.tsx | 11 ++++++- .../pages/journeyDetail/useJourneyDetail.ts | 3 ++ 8 files changed, 88 insertions(+), 16 deletions(-) diff --git a/client/scripts/eslint-baseline.json b/client/scripts/eslint-baseline.json index 24b467f14..5eb8f9cf9 100644 --- a/client/scripts/eslint-baseline.json +++ b/client/scripts/eslint-baseline.json @@ -2,7 +2,7 @@ "src": { "@typescript-eslint/no-explicit-any": 155, "@typescript-eslint/no-unused-expressions": 2, - "@typescript-eslint/no-unused-vars": 159, + "@typescript-eslint/no-unused-vars": 158, "no-empty": 19, "no-useless-assignment": 7, "no-useless-escape": 1, diff --git a/client/src/components/Journey/JourneyDetailPageGalleryView.test.tsx b/client/src/components/Journey/JourneyDetailPageGalleryView.test.tsx index 61b8f24ee..57fefb760 100644 --- a/client/src/components/Journey/JourneyDetailPageGalleryView.test.tsx +++ b/client/src/components/Journey/JourneyDetailPageGalleryView.test.tsx @@ -1,4 +1,4 @@ -// FE-JRN-GALLERY-001 to FE-JRN-GALLERY-020 +// FE-JRN-GALLERY-001 to FE-JRN-GALLERY-021 import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest' import { act, useRef, useState } from 'react' @@ -79,11 +79,12 @@ const entries: JourneyEntry[] = [ /** Mirrors the page wiring: the gallery reports its connected providers up, and the host renders one button per provider next to Upload. The picker's Add goes through the same hook the page takes from useJourneyDetail. */ -function GalleryHarness({ gallery, onPhotoClick, onRefresh, onRegisterUpload }: { +function GalleryHarness({ gallery, onPhotoClick, onRefresh, onRegisterUpload, onUploadProgress }: { gallery: GalleryPhoto[] onPhotoClick: (photos: GalleryPhoto[], index: number) => void onRefresh: () => void onRegisterUpload?: (fn: () => void) => void + onUploadProgress?: (progress: { done: number; total: number } | null) => void }) { const [providers, setProviders] = useState<{ id: string; name: string }[]>([]) const browseRef = useRef<((provider: string) => void) | null>(null) @@ -103,6 +104,7 @@ function GalleryHarness({ gallery, onPhotoClick, onRefresh, onRegisterUpload }: onRefresh={onRefresh} onAddProviderPhotos={addPickedPhotos} onRegisterUpload={onRegisterUpload} + onUploadProgress={onUploadProgress} onRegisterProviders={(p, browse) => { setProviders(p); browseRef.current = browse }} /> @@ -112,10 +114,11 @@ function GalleryHarness({ gallery, onPhotoClick, onRefresh, onRegisterUpload }: function mountGallery(gallery: GalleryPhoto[], onRegisterUpload?: (fn: () => void) => void) { const onPhotoClick = vi.fn() const onRefresh = vi.fn() + const onUploadProgress = vi.fn() const utils = render( - , + , ) - return { ...utils, onPhotoClick, onRefresh } + return { ...utils, onPhotoClick, onRefresh, onUploadProgress } } function useConnectedImmich() { @@ -349,6 +352,19 @@ describe('GalleryView', () => { expect(input.value).toBe('') }) + it('FE-JRN-GALLERY-021: hands the upload count up while the files go, and clears it after', async () => { + const { container, onRefresh, onUploadProgress } = mountGallery([]) + expect(onUploadProgress).toHaveBeenLastCalledWith(null) + onUploadProgress.mockClear() + + const input = container.querySelector('input[type="file"]') as HTMLInputElement + fireEvent.change(input, { target: { files: [new File(['a'], 'a.jpg', { type: 'image/jpeg' })] } }) + + await waitFor(() => expect(onRefresh).toHaveBeenCalledTimes(1)) + await waitFor(() => expect(onUploadProgress).toHaveBeenLastCalledWith(null)) + expect(onUploadProgress).toHaveBeenCalledWith({ done: 0, total: 1 }) + }) + it('FE-JRN-GALLERY-015: reports partially failed uploads but still refreshes', async () => { const failedFile = new File(['b'], 'b.jpg', { type: 'image/jpeg' }) uploadGalleryPhotos.mockResolvedValueOnce({ succeeded: [], failed: [failedFile] }) diff --git a/client/src/components/Journey/JourneyDetailPageGalleryView.tsx b/client/src/components/Journey/JourneyDetailPageGalleryView.tsx index 2768c688a..f7e7522e2 100644 --- a/client/src/components/Journey/JourneyDetailPageGalleryView.tsx +++ b/client/src/components/Journey/JourneyDetailPageGalleryView.tsx @@ -12,7 +12,7 @@ import { useGalleryUpload } from './useGalleryUpload' import { ScrollTrigger } from './JourneyDetailPageScrollTrigger' import EmptyState from '../shared/EmptyState' -export function GalleryView({ entries, gallery, journeyId, userId, trips, onPhotoClick, onRefresh, onAddProviderPhotos, onRegisterUpload, onRegisterProviders }: { +export function GalleryView({ entries, gallery, journeyId, userId, trips, onPhotoClick, onRefresh, onAddProviderPhotos, onRegisterUpload, onRegisterProviders, onUploadProgress }: { entries: JourneyEntry[] gallery: GalleryPhoto[] journeyId: number @@ -23,13 +23,16 @@ export function GalleryView({ entries, gallery, journeyId, userId, trips, onPhot /** What the picker's Add does: the host's useProviderPhotoAdds, shared with the phone screen. */ onAddProviderPhotos: (journeyId: number, provider: string, groups: ProviderPhotoGroup[], entryId: number | null) => Promise onRegisterUpload?: (fn: () => void) => void + /** The upload's file count while it runs, null once it is through: the page's Upload button shows it. */ + onUploadProgress?: (progress: { done: number; total: number } | null) => void onRegisterProviders?: (providers: { id: string; name: string }[], browse: (provider: string) => void) => void }) { const { t } = useTranslation() const [showPicker, setShowPicker] = useState(false) const [pickerProvider, setPickerProvider] = useState(null) - const { uploading: galleryUploading, handleGalleryUpload } = + const { progress: galleryProgress, handleGalleryUpload } = useGalleryUpload({ journeyId, onUploaded: onRefresh, trackProgress: true }) + useEffect(() => { onUploadProgress?.(galleryProgress) }, [galleryProgress, onUploadProgress]) const toast = useToast() // The providers enabled AND connected for the current user, handed up: the page diff --git a/client/src/components/Journey/useGalleryUpload.test.ts b/client/src/components/Journey/useGalleryUpload.test.ts index 802aa0a7e..bb9bcf6f9 100644 --- a/client/src/components/Journey/useGalleryUpload.test.ts +++ b/client/src/components/Journey/useGalleryUpload.test.ts @@ -1,4 +1,4 @@ -// FE-JRN-GALLERYUPLOAD-001 to FE-JRN-GALLERYUPLOAD-006: the gallery upload behind both +// FE-JRN-GALLERYUPLOAD-001 to FE-JRN-GALLERYUPLOAD-007: the gallery upload behind both // the desktop gallery and the phone journey screen. import { act, renderHook } from '@testing-library/react'; import type React from 'react'; @@ -111,4 +111,32 @@ describe('useGalleryUpload', () => { expect(toast.success).toHaveBeenCalledWith('view:journey.photosUploaded'); expect(addToast).not.toHaveBeenCalled(); }); + + it('FE-JRN-GALLERYUPLOAD-007: progress counts the files up while they go and is gone after', async () => { + let finish: () => void = () => {}; + let report: (p: unknown) => void = () => {}; + upload.mockImplementationOnce( + (_id: number, _files: File[], cbs?: { onProgress?: (p: unknown) => void }) => + new Promise((resolve) => { + report = (p) => cbs?.onProgress?.(p); + finish = () => resolve({ succeeded: [], failed: [] }); + }) + ); + const { result } = setup({ trackProgress: true }); + expect(result.current.progress).toBeNull(); + let running: Promise = Promise.resolve(); + await act(async () => { + running = result.current.handleGalleryUpload( + changeEvent([new File(['a'], 'a.jpg'), new File(['b'], 'b.jpg')]).event + ); + }); + expect(result.current.progress).toEqual({ done: 0, total: 2 }); + act(() => report({ done: 1, total: 2 })); + expect(result.current.progress).toEqual({ done: 1, total: 2 }); + await act(async () => { + finish(); + await running; + }); + expect(result.current.progress).toBeNull(); + }); }); diff --git a/client/src/components/Journey/useGalleryUpload.ts b/client/src/components/Journey/useGalleryUpload.ts index 124abad5e..1048ea4b9 100644 --- a/client/src/components/Journey/useGalleryUpload.ts +++ b/client/src/components/Journey/useGalleryUpload.ts @@ -14,8 +14,9 @@ export interface GalleryUploadOptions { /** Reload the journey once the upload is through, whatever came of it. */ onUploaded: () => void; /** - * Hand the upload a progress callback, as the desktop gallery always has. The count - * is only tracked; no view shows it. + * Hand the upload a progress callback, so `progress` counts the files up while they go + * (the desktop gallery's upload button shows it). Without it the count stays at none + * done until the batch is through. */ trackProgress?: boolean; /** @@ -76,5 +77,5 @@ export function useGalleryUpload({ e.target.value = ''; }; - return { uploading: progress !== null, handleGalleryUpload }; + return { uploading: progress !== null, progress, handleGalleryUpload }; } diff --git a/client/src/pages/JourneyDetailPage.tsx b/client/src/pages/JourneyDetailPage.tsx index a8fb00109..483c51950 100644 --- a/client/src/pages/JourneyDetailPage.tsx +++ b/client/src/pages/JourneyDetailPage.tsx @@ -11,7 +11,7 @@ import EmptyState from '../components/shared/EmptyState' import { Outlet } from 'react-router' import { ArrowLeft, MoreHorizontal, List, Grid, MapPin, - Plus, ChevronUp, ChevronDown, Eye, EyeOff, BookOpen, Image, Search, X, + Plus, ChevronUp, ChevronDown, Eye, EyeOff, BookOpen, Image, Search, X, Loader2, } from 'lucide-react' import MobileMapTimeline from '../components/Journey/MobileMapTimeline' import JourneyDayDawarich from '../components/Journey/JourneyDayDawarich' @@ -56,6 +56,7 @@ function JourneyDetailPageDesktop() { query, setQuery, dismissSuggestion, restoreSuggestions, openAtEntryId, dawarichByDate, dawarichBusyId, acceptDawarich, dismissDawarich, mapRef, fullMapRef, galleryUploadRef, galleryProviders, setGalleryProviders, galleryBrowseRef, + galleryUploadProgress, setGalleryUploadProgress, activeLocationId, handleMarkerClick, handleLocationClick, mapEntries, sidebarMapItems, tripDates, isMobile, tracks, mapPhotos, openMapPhotos, feedEdge, scrollFeedTo, @@ -145,11 +146,21 @@ function JourneyDetailPageDesktop() { ))} ) : null @@ -600,6 +611,7 @@ function JourneyDetailPageDesktop() { )} { galleryUploadRef.current = fn }} + onUploadProgress={setGalleryUploadProgress} onRegisterProviders={(providers, browse) => { setGalleryProviders(providers); galleryBrowseRef.current = browse }} entries={current.entries} gallery={current.gallery || []} diff --git a/client/src/pages/JourneyDetailPage.wiring.test.tsx b/client/src/pages/JourneyDetailPage.wiring.test.tsx index b56c6dce1..3c5f435eb 100644 --- a/client/src/pages/JourneyDetailPage.wiring.test.tsx +++ b/client/src/pages/JourneyDetailPage.wiring.test.tsx @@ -1,4 +1,4 @@ -// FE-JRN-DETWIRE-001 to FE-JRN-DETWIRE-042 +// FE-JRN-DETWIRE-001 to FE-JRN-DETWIRE-043 import { describe, it, expect, beforeEach, vi } from 'vitest'; import { render, screen, fireEvent, waitFor, within } from '../../tests/helpers/render'; import { journeyApi } from '../api/client'; @@ -106,6 +106,7 @@ function buildHook(over: Record = {}): Record acceptDawarich: vi.fn(async () => {}), dismissDawarich: vi.fn(), mapRef: { current: null }, fullMapRef: { current: null }, galleryUploadRef: { current: null }, galleryProviders: [], setGalleryProviders: vi.fn(), galleryBrowseRef: { current: null }, + galleryUploadProgress: null, setGalleryUploadProgress: vi.fn(), activeLocationId: null, handleMarkerClick: vi.fn(), handleLocationClick: vi.fn(), mapEntries: [], sidebarMapItems: [], tripDates: new Set(), isMobile: false, feedEdge: { atTop: true, atBottom: true }, scrollFeedTo: vi.fn(), @@ -601,4 +602,12 @@ describe('JourneyDetailPage wiring', () => { process.off('unhandledRejection', unhandled); } }); + + it('FE-JRN-DETWIRE-043: the gallery upload button counts the files up while they go', () => { + const { hook } = setup({ view: 'gallery', galleryUploadProgress: { done: 1, total: 3 } }); + expect(mocks.captured.gallery.onUploadProgress).toBe(hook.setGalleryUploadProgress); + const button = screen.getByText('journey.editor.uploadingProgress').closest('button')!; + expect(button).toBeDisabled(); + expect(screen.queryByText('common.upload')).not.toBeInTheDocument(); + }); }); diff --git a/client/src/pages/journeyDetail/useJourneyDetail.ts b/client/src/pages/journeyDetail/useJourneyDetail.ts index d0287fcb8..00bdd849e 100644 --- a/client/src/pages/journeyDetail/useJourneyDetail.ts +++ b/client/src/pages/journeyDetail/useJourneyDetail.ts @@ -40,6 +40,8 @@ export function useJourneyDetail() { const fullMapRef = useRef(null) /** The gallery hands its file picker up here, so the hero button can open it. */ const galleryUploadRef = useRef<(() => void) | null>(null) + /** The gallery's upload count while it runs, for that same hero button. */ + const [galleryUploadProgress, setGalleryUploadProgress] = useState<{ done: number; total: number } | null>(null) /** Same deal for photo providers: the gallery probes which ones are connected and reports them up, so the header can render an Immich/ Synology button next to Upload. */ @@ -561,6 +563,7 @@ export function useJourneyDetail() { dawarichByDate, dawarichBusyId: dawarich.busyId, acceptDawarich, dismissDawarich, openAtEntryId, mapRef, fullMapRef, galleryUploadRef, galleryProviders, setGalleryProviders, galleryBrowseRef, + galleryUploadProgress, setGalleryUploadProgress, activeLocationId, handleMarkerClick, handleLocationClick, mapEntries, sidebarMapItems, tripDates, isMobile, tracks: mapTracks, dawarichTrail, mapPhotos, openMapPhotos,