mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-11-13 15:51:00 +00:00
feat(roadtrip): show the place photo on the stop sheet and the stop dialog (#2629)
On the road a stop's picture is often what decides whether it is worth the detour. The phone's stop sheet shows the place's picture full width under its name and the desktop stop dialog shows it in place of the kind tile; both open the place's photos on tap. The picture is the place's own image, else the provider photo for its Google or OSM id through the shared photo cache.
This commit is contained in:
@@ -8,8 +8,10 @@ import type { CorridorPoi } from './useCorridorPois'
|
||||
|
||||
// The dialog carries the charging panel, which reads the open availability data for a
|
||||
// station that is not on the trip yet. Both of its collaborators are stubbed: the trip
|
||||
// is whichever one is open, and the data comes off the network.
|
||||
vi.mock('../../store/tripStore', () => ({ useTripStore: (select: (s: { trip: { id: number } }) => unknown) => select({ trip: { id: 1 } }) }))
|
||||
// is whichever one is open, and the data comes off the network. The trip's places are
|
||||
// what a stop being edited shows its picture from (#2629).
|
||||
const tripState = vi.hoisted(() => ({ trip: { id: 1 }, places: [] as Array<Record<string, unknown>> }))
|
||||
vi.mock('../../store/tripStore', () => ({ useTripStore: (select: (s: typeof tripState) => unknown) => select(tripState) }))
|
||||
vi.mock('../../repo/chargingRepo', () => ({ chargingRepo: { read: vi.fn(), readAt: vi.fn() } }))
|
||||
|
||||
const wrap = (ui: React.ReactElement) => render(<TranslationProvider>{ui}</TranslationProvider>)
|
||||
@@ -21,7 +23,10 @@ const charging = {
|
||||
tariffs: [], pricesUnavailable: false,
|
||||
}
|
||||
|
||||
beforeEach(() => { vi.clearAllMocks() })
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
tripState.places = []
|
||||
})
|
||||
|
||||
function poi(over: Partial<CorridorPoi> = {}): CorridorPoi {
|
||||
return {
|
||||
@@ -394,6 +399,22 @@ describe('editing a roadtrip stop', () => {
|
||||
expect(screen.getByRole('button', { name: 'Overnight' })).toHaveAttribute('aria-pressed', 'true')
|
||||
expect(screen.getByRole('textbox', { name: 'Check-in' })).toHaveValue('15:45')
|
||||
})
|
||||
|
||||
it('FE-ROADTRIP-STOPPOPUP-035: a stop on the trip shows its picture in place of the tile, and it opens the photos (#2629)', () => {
|
||||
tripState.places = [{ id: 7, name: 'Aral Autohof', lat: 53.1, lng: 10.2, image_url: '/uploads/places/aral.jpg' }]
|
||||
wrap(<RoadtripStopPopup draft={draft({ editing: { placeId: 7, stopType: 'fuel', dwellMinutes: 10 } })} {...noop} />)
|
||||
|
||||
const trigger = screen.getByRole('button', { name: 'Show photos' })
|
||||
expect(trigger.querySelector('img')).toHaveAttribute('src', '/uploads/places/aral.jpg')
|
||||
})
|
||||
|
||||
it('FE-ROADTRIP-STOPPOPUP-036: a hit from the search is no place yet and keeps the tile of its kind', () => {
|
||||
tripState.places = [{ id: 7, name: 'Aral Autohof', lat: 53.1, lng: 10.2, image_url: '/uploads/places/aral.jpg' }]
|
||||
const { container } = wrap(<RoadtripStopPopup draft={draft()} {...noop} />)
|
||||
|
||||
expect(screen.queryByRole('button', { name: 'Show photos' })).toBeNull()
|
||||
expect(container.ownerDocument.querySelector('img[src="/uploads/places/aral.jpg"]')).toBeNull()
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
|
||||
@@ -5,7 +5,9 @@ import { PANEL, Segmented } from '../shared/dialogParts'
|
||||
import CustomTimePicker from '../shared/CustomTimePicker'
|
||||
import StayPortals from './StayPortals'
|
||||
import ChargingInfo from './ChargingInfo'
|
||||
import StopPhoto from './StopPhoto'
|
||||
import { useTranslation } from '../../i18n/TranslationContext'
|
||||
import { useTripStore } from '../../store/tripStore'
|
||||
import { safeExternalHref } from '../../utils/safeUrl'
|
||||
import { StopKindChips, StopStayChips } from './StopChips'
|
||||
import type { CorridorPoi } from './useCorridorPois'
|
||||
@@ -94,6 +96,10 @@ export default function RoadtripStopPopup({
|
||||
const [night, setNight] = useState<boolean>(() => nightByDefault(draft))
|
||||
const [checkIn, setCheckIn] = useState(() => checkInByDefault(draft))
|
||||
const labelId = useId()
|
||||
// A stop already on the trip shows its picture in place of the kind's tile (#2629); a
|
||||
// hit from the search is not a place yet and keeps the tile.
|
||||
const editedPlaceId = draft?.editing?.placeId
|
||||
const place = useTripStore(s => (editedPlaceId == null ? null : s.places.find(p => p.id === editedPlaceId) ?? null))
|
||||
|
||||
// The popup stays mounted while the draft is swapped for another one, so every
|
||||
// answer is seeded again from the new draft, the way the phone sheet does it.
|
||||
@@ -137,12 +143,18 @@ export default function RoadtripStopPopup({
|
||||
header={(
|
||||
<DialogHeader
|
||||
tile={(
|
||||
<DialogTile>
|
||||
{/* The road-trip palette, the same one the map pin and the result row use. */}
|
||||
<span style={{ color: tone }}>
|
||||
{kind ? <kind.Icon size={20} strokeWidth={1.9} aria-hidden /> : <ParkingSquare size={20} strokeWidth={1.9} aria-hidden />}
|
||||
</span>
|
||||
</DialogTile>
|
||||
<StopPhoto
|
||||
place={place}
|
||||
className="h-[46px] w-[46px] flex-none rounded-[14px] bg-surface-card shadow-sm"
|
||||
fallback={(
|
||||
<DialogTile>
|
||||
{/* The road-trip palette, the same one the map pin and the result row use. */}
|
||||
<span style={{ color: tone }}>
|
||||
{kind ? <kind.Icon size={20} strokeWidth={1.9} aria-hidden /> : <ParkingSquare size={20} strokeWidth={1.9} aria-hidden />}
|
||||
</span>
|
||||
</DialogTile>
|
||||
)}
|
||||
/>
|
||||
)}
|
||||
tint={NEUTRAL_TINT}
|
||||
labelId={labelId}
|
||||
|
||||
@@ -0,0 +1,49 @@
|
||||
import { useState, type ReactNode } from 'react';
|
||||
|
||||
import { useTranslation } from '../../i18n';
|
||||
import type { Place } from '../../types';
|
||||
import PlacePhotoGallery from '../Planner/PlacePhotoGallery';
|
||||
import { usePlacePhoto } from '../shared/usePlacePhoto';
|
||||
|
||||
interface StopPhotoProps {
|
||||
/** The trip's place behind the stop; a stop not saved yet has none. */
|
||||
place: Place | null;
|
||||
/** The frame: its size, corners and spacing. The picture fills it. */
|
||||
className: string;
|
||||
/** What stands in while there is no picture. */
|
||||
fallback?: ReactNode;
|
||||
}
|
||||
|
||||
/**
|
||||
* A stop's picture on the road (#2629): what helps decide, from the passenger
|
||||
* seat, whether a place is worth the detour. A tap opens the place's photos, the
|
||||
* same gallery the place sheet and the inspector open. A picture that fails to
|
||||
* load gives way to the fallback rather than leaving a broken image behind.
|
||||
*/
|
||||
export default function StopPhoto({ place, className, fallback = null }: StopPhotoProps) {
|
||||
const { t, language } = useTranslation();
|
||||
const photo = usePlacePhoto(place);
|
||||
const [broken, setBroken] = useState<string | null>(null);
|
||||
|
||||
if (!place || !photo || photo === broken) return <>{fallback}</>;
|
||||
// The corners are the frame's, inherited all the way down, so the focus ring of the
|
||||
// gallery trigger follows the picture whatever shape the caller gave it.
|
||||
return (
|
||||
<div className={className}>
|
||||
<PlacePhotoGallery
|
||||
place={place}
|
||||
language={language}
|
||||
label={t('places.openPhotos')}
|
||||
className="h-full w-full rounded-[inherit]"
|
||||
>
|
||||
<img
|
||||
src={photo}
|
||||
alt=""
|
||||
decoding="async"
|
||||
onError={() => setBroken(photo)}
|
||||
className="block h-full w-full rounded-[inherit] object-cover"
|
||||
/>
|
||||
</PlacePhotoGallery>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
@@ -0,0 +1,65 @@
|
||||
// FE-SHARED-PLACEPHOTO-001 to FE-SHARED-PLACEPHOTO-005
|
||||
import { renderHook } from '@testing-library/react';
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
import { resetAllStores, seedStore } from '../../../tests/helpers/store';
|
||||
import { fetchPhoto, getCached } from '../../services/photoService';
|
||||
import { useAuthStore } from '../../store/authStore';
|
||||
import { usePlacePhoto } from './usePlacePhoto';
|
||||
|
||||
vi.mock('../../services/photoService', () => ({ fetchPhoto: vi.fn(), getCached: vi.fn() }));
|
||||
|
||||
const PROXY = '/api/maps/place-photo/ChIJ123/bytes';
|
||||
|
||||
beforeEach(() => {
|
||||
resetAllStores();
|
||||
vi.clearAllMocks();
|
||||
vi.mocked(getCached).mockReturnValue(undefined);
|
||||
vi.mocked(fetchPhoto).mockImplementation((_key, _id, _lat, _lng, _name, callback) => {
|
||||
callback?.({ photoUrl: PROXY, thumbDataUrl: null });
|
||||
});
|
||||
});
|
||||
|
||||
describe('usePlacePhoto (#2629)', () => {
|
||||
it("FE-SHARED-PLACEPHOTO-001: the place's own picture wins and asks the provider for nothing", () => {
|
||||
const { result } = renderHook(() =>
|
||||
usePlacePhoto({ name: 'Dom', image_url: '/uploads/places/dom.jpg', google_place_id: 'ChIJ123' })
|
||||
);
|
||||
expect(result.current).toBe('/uploads/places/dom.jpg');
|
||||
expect(fetchPhoto).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('FE-SHARED-PLACEPHOTO-002: without one, the provider picture is asked by its id through the shared cache', () => {
|
||||
const { result } = renderHook(() =>
|
||||
usePlacePhoto({ name: 'Dom', google_place_id: 'ChIJ123', lat: 50.9, lng: 6.9 })
|
||||
);
|
||||
expect(fetchPhoto).toHaveBeenCalledWith('ChIJ123', 'ChIJ123', 50.9, 6.9, 'Dom', expect.any(Function));
|
||||
expect(result.current).toBe(PROXY);
|
||||
});
|
||||
|
||||
it('FE-SHARED-PLACEPHOTO-003: a place known only by its position gets no picture of its surroundings', () => {
|
||||
const { result } = renderHook(() => usePlacePhoto({ name: 'Somewhere', lat: 50.9, lng: 6.9 }));
|
||||
expect(fetchPhoto).not.toHaveBeenCalled();
|
||||
expect(result.current).toBeNull();
|
||||
});
|
||||
|
||||
it('FE-SHARED-PLACEPHOTO-004: with photos switched off nothing is asked and nothing is shown', () => {
|
||||
seedStore(useAuthStore, { placesPhotosEnabled: false });
|
||||
const { result } = renderHook(() => usePlacePhoto({ name: 'Dom', osm_id: 'way:42' }));
|
||||
expect(fetchPhoto).not.toHaveBeenCalled();
|
||||
expect(result.current).toBeNull();
|
||||
});
|
||||
|
||||
it("FE-SHARED-PLACEPHOTO-005: the next place never shows the previous one's picture while its own loads", () => {
|
||||
vi.mocked(fetchPhoto).mockImplementation((key, _id, _lat, _lng, _name, callback) => {
|
||||
if (key === 'ChIJ123') callback?.({ photoUrl: PROXY, thumbDataUrl: null });
|
||||
});
|
||||
const { result, rerender } = renderHook(({ id }) => usePlacePhoto({ name: 'Stop', google_place_id: id }), {
|
||||
initialProps: { id: 'ChIJ123' },
|
||||
});
|
||||
expect(result.current).toBe(PROXY);
|
||||
|
||||
rerender({ id: 'ChIJ456' });
|
||||
expect(result.current).toBeNull();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,52 @@
|
||||
import { useEffect, useState } from 'react';
|
||||
|
||||
import { fetchPhoto, getCached } from '../../services/photoService';
|
||||
import { useAuthStore } from '../../store/authStore';
|
||||
import { photoCacheKey } from '../Map/placePhoto';
|
||||
|
||||
/** What a place's picture is found by. */
|
||||
interface PhotoPlace {
|
||||
name: string;
|
||||
image_url?: string | null;
|
||||
google_place_id?: string | null;
|
||||
osm_id?: string | null;
|
||||
lat?: number | null;
|
||||
lng?: number | null;
|
||||
}
|
||||
|
||||
/**
|
||||
* A place's picture at full size, for a view that shows it large (#2629).
|
||||
*
|
||||
* The place's own picture wins, as everywhere else. Without one, the picture its
|
||||
* provider has for its Google or OSM id is asked through the shared photo cache,
|
||||
* so a place the list or the map already shows costs no second request. A place
|
||||
* known only by its position gets none: the proxy would answer with whatever is
|
||||
* near those coordinates, the same street for every stop along it. Null while it
|
||||
* loads, when there is none and while photos are switched off.
|
||||
*/
|
||||
export function usePlacePhoto(place: PhotoPlace | null): string | null {
|
||||
const enabled = useAuthStore((s) => s.placesPhotosEnabled);
|
||||
const own = place?.image_url || null;
|
||||
const providerId = place?.google_place_id || place?.osm_id || null;
|
||||
const key = place && !own && providerId ? photoCacheKey(place) : '';
|
||||
const lat = place?.lat ?? undefined;
|
||||
const lng = place?.lng ?? undefined;
|
||||
const name = place?.name;
|
||||
// Names the place it belongs to, so the previous place's picture never shows for the next.
|
||||
const [loaded, setLoaded] = useState<{ key: string; url: string | null } | null>(null);
|
||||
|
||||
useEffect(() => {
|
||||
if (!enabled || !key || !providerId) return;
|
||||
let cancelled = false;
|
||||
fetchPhoto(key, providerId, lat, lng, name, (entry) => {
|
||||
if (!cancelled) setLoaded({ key, url: entry.photoUrl });
|
||||
});
|
||||
return () => {
|
||||
cancelled = true;
|
||||
};
|
||||
}, [enabled, key, providerId, lat, lng, name]);
|
||||
|
||||
if (own) return own;
|
||||
if (!enabled || !key) return null;
|
||||
return loaded?.key === key ? loaded.url : (getCached(key)?.photoUrl ?? null);
|
||||
}
|
||||
@@ -11,6 +11,7 @@ import { useTranslation } from '../../../../i18n'
|
||||
import { useSettingsStore } from '../../../../store/settingsStore'
|
||||
import { useTripStore } from '../../../../store/tripStore'
|
||||
import ChargingInfo from '../../../../components/Roadtrip/ChargingInfo'
|
||||
import StopPhoto from '../../../../components/Roadtrip/StopPhoto'
|
||||
import { formatDurationShort } from '../../../../components/Roadtrip/roadtripModel'
|
||||
import { roadtripRows, stageOf, type StopRow } from '../../../../components/Roadtrip/roadtripRowModel'
|
||||
import { readStay, shownStay } from '../../../../components/Roadtrip/stayReading'
|
||||
@@ -336,6 +337,11 @@ export default function MRtStopSheet({ planner, shell }: MTripSheetsProps) {
|
||||
</div>
|
||||
|
||||
<div className="min-h-0 flex-1 overflow-y-auto px-[18px] pb-[18px]">
|
||||
{/* ── The place's picture, which is often what decides whether a stop is worth it
|
||||
when the day runs short (#2629). Full width under the name, where the eye lands
|
||||
first; a tap opens the rest of its photos. Absent when the place has none. ── */}
|
||||
<StopPhoto place={place} className="mt-[14px] h-[140px] w-full rounded-[16px] bg-[color:var(--m-ic)]" />
|
||||
|
||||
{/* ── The two clock readings, and where they come from ── */}
|
||||
{(arrive || leave) && (
|
||||
<div className="mt-[14px] rounded-[16px] bg-[color:var(--m-inner)] px-3.5 py-3">
|
||||
|
||||
@@ -8,7 +8,7 @@ import { useTripStore } from '../../../../src/store/tripStore'
|
||||
import { resetAllStores, seedStore } from '../../../helpers/store'
|
||||
import { fireEvent, render, screen, waitFor } from '../../../helpers/render'
|
||||
|
||||
// FE-MOB-RTSTOP-001 to FE-MOB-RTSTOP-038
|
||||
// FE-MOB-RTSTOP-001 to FE-MOB-RTSTOP-041
|
||||
//
|
||||
// The sheet renders inside the real TranslationProvider, so the copy is asserted
|
||||
// in English. `planner.t` from the fixture is never consulted here.
|
||||
@@ -557,4 +557,20 @@ describe('MRtStopSheet', () => {
|
||||
expect(screen.getByText('The travel day ends before 14:00, so the drive goes on the next morning.')).toBeInTheDocument()
|
||||
})
|
||||
})
|
||||
|
||||
describe('the place picture (#2629)', () => {
|
||||
it('FE-MOB-RTSTOP-040: a place with a picture shows it under the name, and a tap opens its photos', () => {
|
||||
const withPhoto = PLACES.map(p => (p.id === 203 ? { ...p, image_url: '/uploads/places/bremen.jpg' } : p))
|
||||
renderSheet({}, {}, withPhoto as Place[])
|
||||
const trigger = screen.getByRole('button', { name: 'Show photos' })
|
||||
expect(trigger.querySelector('img')).toHaveAttribute('src', '/uploads/places/bremen.jpg')
|
||||
})
|
||||
|
||||
it('FE-MOB-RTSTOP-041: a place without one, or known only by its position, shows no picture and asks for none', () => {
|
||||
renderSheet()
|
||||
expect(screen.getByRole('dialog')).toBeInTheDocument()
|
||||
expect(screen.queryByRole('button', { name: 'Show photos' })).not.toBeInTheDocument()
|
||||
expect(screen.getByRole('dialog').querySelector('img')).toBeNull()
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user