diff --git a/client/src/components/shared/PlaceAvatar.test.tsx b/client/src/components/shared/PlaceAvatar.test.tsx index 24871e47c..ddfdeb500 100644 --- a/client/src/components/shared/PlaceAvatar.test.tsx +++ b/client/src/components/shared/PlaceAvatar.test.tsx @@ -1,5 +1,5 @@ import { render, screen, fireEvent, act } from '../../../tests/helpers/render'; -import { getCached, isLoading, fetchPhoto, onThumbReady } from '../../services/photoService'; +import { getCached, isLoading, fetchPhoto, onThumbReady, onPhotoLoaded } from '../../services/photoService'; // Mock photoService — all functions are no-ops / return null vi.mock('../../services/photoService', () => ({ @@ -7,6 +7,7 @@ vi.mock('../../services/photoService', () => ({ isLoading: vi.fn(() => false), fetchPhoto: vi.fn(), onThumbReady: vi.fn(() => () => {}), + onPhotoLoaded: vi.fn(() => () => {}), })); // Mock IntersectionObserver as a class constructor @@ -34,6 +35,7 @@ beforeEach(() => { vi.mocked(isLoading).mockReturnValue(false); vi.mocked(fetchPhoto).mockReset(); vi.mocked(onThumbReady).mockReturnValue(() => {}); + vi.mocked(onPhotoLoaded).mockReset().mockReturnValue(() => {}); }); afterEach(() => { @@ -182,4 +184,79 @@ describe('PlaceAvatar', () => { render(); expect(mockObserve).not.toHaveBeenCalled(); }); + + /** The callback the self-heal handed to fetchPhoto. */ + function photoCallback() { + const callback = vi.mocked(fetchPhoto).mock.calls[0]?.[5]; + if (!callback) throw new Error('fetchPhoto was called without a callback'); + return callback; + } + + // A proxy image_url whose cached file is gone answers 204; the refetch then + // hands back the very same URL once the server has the photo again. + const proxyPlace = { + ...basePlaceNoImage, + google_place_id: 'G1', + image_url: '/api/maps/place-photo/G1/bytes', + }; + + it('FE-COMP-AVATAR-017: loads the image again when the self-heal returns the same proxy URL', () => { + const { container } = render(); + const before = container.querySelector('img') as HTMLImageElement; + fireEvent.error(before); + + expect(vi.mocked(fetchPhoto)).toHaveBeenCalledWith('refetch:G1', 'G1', proxyPlace.lat, proxyPlace.lng, proxyPlace.name, expect.any(Function)); + const callback = photoCallback(); + act(() => callback({ photoUrl: proxyPlace.image_url, thumbDataUrl: null })); + + const after = container.querySelector('img') as HTMLImageElement; + expect(after).not.toBe(before); + expect(after.getAttribute('src')).toBe(proxyPlace.image_url); + }); + + it('FE-COMP-AVATAR-018: ends on the category icon when the reloaded image fails too', () => { + const { container } = render(); + fireEvent.error(container.querySelector('img') as HTMLImageElement); + const callback = photoCallback(); + act(() => callback({ photoUrl: proxyPlace.image_url, thumbDataUrl: null })); + fireEvent.error(container.querySelector('img') as HTMLImageElement); + + expect(container.querySelector('img')).toBeNull(); + expect(container.querySelector('svg')).toBeTruthy(); + expect(vi.mocked(fetchPhoto)).toHaveBeenCalledTimes(1); + }); + + it('FE-COMP-AVATAR-019: ends on the category icon when the self-heal finds no photo', () => { + const { container } = render(); + fireEvent.error(container.querySelector('img') as HTMLImageElement); + const callback = photoCallback(); + act(() => callback({ photoUrl: null, thumbDataUrl: null })); + + expect(container.querySelector('img')).toBeNull(); + }); + + it('FE-COMP-AVATAR-020: a reused avatar gets the self-heal again for the next place', () => { + const { container, rerender } = render(); + fireEvent.error(container.querySelector('img') as HTMLImageElement); + + const next = { ...proxyPlace, id: 2, google_place_id: 'G2', image_url: '/api/maps/place-photo/G2/bytes' }; + rerender(); + fireEvent.error(container.querySelector('img') as HTMLImageElement); + + expect(vi.mocked(fetchPhoto)).toHaveBeenLastCalledWith('refetch:G2', 'G2', next.lat, next.lng, next.name, expect.any(Function)); + }); + + it('FE-COMP-AVATAR-021: takes the photo of a fetch another view started, even without a thumb', () => { + vi.mocked(isLoading).mockReturnValue(true); + const { container } = render(); + act(() => { + observerInstance?.callback([{ isIntersecting: true }]); + }); + + expect(vi.mocked(onPhotoLoaded)).toHaveBeenCalledWith('G3', expect.any(Function)); + const listener = vi.mocked(onPhotoLoaded).mock.calls[0][1]; + act(() => listener({ photoUrl: '/photo/G3.jpg', thumbDataUrl: null })); + + expect((container.querySelector('img') as HTMLImageElement).getAttribute('src')).toBe('/photo/G3.jpg'); + }); }); diff --git a/client/src/components/shared/PlaceAvatar.tsx b/client/src/components/shared/PlaceAvatar.tsx index cc55d85a6..538f5d74d 100644 --- a/client/src/components/shared/PlaceAvatar.tsx +++ b/client/src/components/shared/PlaceAvatar.tsx @@ -1,6 +1,6 @@ import React, { useState, useEffect, useRef } from 'react' import { getCategoryIcon } from './categoryIcons' -import { getCached, isLoading, fetchPhoto, onThumbReady } from '../../services/photoService' +import { getCached, isLoading, fetchPhoto, onThumbReady, onPhotoLoaded } from '../../services/photoService' import { useAuthStore } from '../../store/authStore' import type { Place } from '../../types' @@ -17,6 +17,9 @@ interface PlaceAvatarProps { export default React.memo(function PlaceAvatar({ place, size = 32, category }: PlaceAvatarProps) { const [photoSrc, setPhotoSrc] = useState(place.image_url || null) + // Bumped to remount the when the photo should be loaded again from the + // same URL, which a src change alone cannot do. + const [loadAttempt, setLoadAttempt] = useState(0) const [visible, setVisible] = useState(false) const imageUrlFailed = useRef(false) const ref = useRef(null) @@ -39,6 +42,8 @@ export default React.memo(function PlaceAvatar({ place, size = 32, category }: P }, [place.id]) useEffect(() => { + // A new place or a new picture gets its own self-heal below. + imageUrlFailed.current = false if (!visible) return if (place.image_url) { setPhotoSrc(place.image_url); return } if (!placesPhotosEnabled) return @@ -57,7 +62,12 @@ export default React.memo(function PlaceAvatar({ place, size = 32, category }: P } if (isLoading(cacheKey)) { - return onThumbReady(cacheKey, thumb => setPhotoSrc(thumb)) + // Another view started this fetch. Take its photo as soon as it lands, + // and the thumb when that follows; a failed thumb must not leave the + // category icon standing in for a photo that loaded. + const offPhoto = onPhotoLoaded(cacheKey, entry => setPhotoSrc(entry.thumbDataUrl || entry.photoUrl)) + const offThumb = onThumbReady(cacheKey, thumb => setPhotoSrc(thumb)) + return () => { offPhoto(); offThumb() } } fetchPhoto(cacheKey, photoId || `coords:${place.lat}:${place.lng}`, place.lat, place.lng, place.name, @@ -83,6 +93,7 @@ export default React.memo(function PlaceAvatar({ place, size = 32, category }: P return (
{place.name} has to be remounted to ask once more; + // setting an unchanged src would leave it broken (5.0.0-pre.1 + // feedback). A second failure ends on the category icon. fetchPhoto(cacheKey, photoId, place.lat ?? undefined, place.lng ?? undefined, place.name, - entry => { setPhotoSrc(entry.thumbDataUrl || entry.photoUrl) } + entry => { + setPhotoSrc(entry.thumbDataUrl || entry.photoUrl) + setLoadAttempt(n => n + 1) + } ) } else { setPhotoSrc(null)