diff --git a/client/src/components/Journey/JourneyMap.test.tsx b/client/src/components/Journey/JourneyMap.test.tsx index a05695dac..e6206a01e 100644 --- a/client/src/components/Journey/JourneyMap.test.tsx +++ b/client/src/components/Journey/JourneyMap.test.tsx @@ -1,4 +1,4 @@ -// FE-COMP-JOURNEYMAP-001 to FE-COMP-JOURNEYMAP-058 +// FE-COMP-JOURNEYMAP-001 to FE-COMP-JOURNEYMAP-061 vi.mock('../../api/websocket', () => ({ connect: vi.fn(), @@ -12,9 +12,18 @@ vi.mock('../../api/websocket', () => ({ // Leaflet does not work in jsdom — mock the entire library vi.mock('leaflet', () => { + // The card a pin binds; `once('add')` is where its element gets its tap. + const mockTooltip = { once: vi.fn().mockReturnThis() }; + const domEvent = { + on: vi.fn((el: HTMLElement, types: string, fn: (e: Event) => void) => { + types.split(' ').forEach(type => el.addEventListener(type, fn)); + }), + stopPropagation: vi.fn((e: Event) => e.stopPropagation()), + }; const mockMarker = { addTo: vi.fn().mockReturnThis(), bindTooltip: vi.fn().mockReturnThis(), + getTooltip: vi.fn(() => mockTooltip), on: vi.fn().mockReturnThis(), setIcon: vi.fn(), setZIndexOffset: vi.fn(), @@ -57,6 +66,7 @@ vi.mock('leaflet', () => { // The map builds its own attribution control so it can collapse to the credit // alone (see the GL twin: the phone lays a carousel over the bottom edge). control: { attribution: vi.fn(() => ({ addTo: vi.fn() })) }, + DomEvent: domEvent, }, map: vi.fn(() => mockMap), tileLayer: vi.fn(() => ({ addTo: vi.fn(), setUrl: vi.fn() })), @@ -65,6 +75,7 @@ vi.mock('leaflet', () => { divIcon: vi.fn(() => ({})), latLngBounds: vi.fn(() => ({})), control: { attribution: vi.fn(() => ({ addTo: vi.fn() })) }, + DomEvent: domEvent, }; }); @@ -182,6 +193,63 @@ describe('JourneyMap', () => { ); }); + describe('the marker card as a way into the entry (#2885)', () => { + /** Builds each card's element the way Leaflet does on its first open, inside the map. */ + function openCards() { + const tooltip = mockedMarker().getTooltip(); + const mapClick = vi.fn(); + const mapPress = vi.fn(); + return vi.mocked(tooltip.once).mock.calls.map(([event, onAdd]: [string, (e: unknown) => void]) => { + expect(event).toBe('add'); + const el = document.createElement('div'); + el.innerHTML = '
card
'; + const map = document.createElement('div'); + map.appendChild(el); + map.addEventListener('click', mapClick); + map.addEventListener('mousedown', mapPress); + onAdd({ target: { getElement: () => el } }); + return { title: el.firstElementChild as HTMLElement, mapClick, mapPress }; + }); + } + + it('FE-COMP-JOURNEYMAP-059: a tap on a pin's card opens that entry and never reaches the map', () => { + const onMarkerCardClick = vi.fn(); + render(); + + expect(mockedMarker().bindTooltip).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ className: 'map-tooltip trek-journey-tooltip is-openable' }), + ); + const [first, second] = openCards(); + second.title.click(); + first.title.click(); + + expect(onMarkerCardClick.mock.calls).toEqual([['e2'], ['e1']]); + expect(first.mapClick).not.toHaveBeenCalled(); + }); + + it('FE-COMP-JOURNEYMAP-060: pressing the card keeps the focus, so the pin does not close it first', () => { + render(); + const [card] = openCards(); + + const press = new MouseEvent('mousedown', { bubbles: true, cancelable: true }); + card.title.dispatchEvent(press); + + expect(press.defaultPrevented).toBe(true); + expect(card.mapPress).not.toHaveBeenCalled(); + }); + + it('FE-COMP-JOURNEYMAP-061: without a handler the card stays a label the map's drags pass through', () => { + render(); + + expect(mockedMarker().bindTooltip).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ className: 'map-tooltip trek-journey-tooltip' }), + ); + expect(mockedMarker().getTooltip().once).not.toHaveBeenCalled(); + }); + }); + it('FE-COMP-JOURNEYMAP-006: exposes imperative handle (focusMarker)', () => { const ref = React.createRef(); render( diff --git a/client/src/components/Journey/JourneyMap.tsx b/client/src/components/Journey/JourneyMap.tsx index 1b6d72985..7c227dca7 100644 --- a/client/src/components/Journey/JourneyMap.tsx +++ b/client/src/components/Journey/JourneyMap.tsx @@ -66,15 +66,14 @@ interface Props { onMarkerClick?: (id: string, type?: string) => void fullScreen?: boolean /** - * Leave the marker labels off. + * Open the entry a marker card names when the card itself is tapped. * - * On the phone the map sits above a carousel whose active card already carries - * the entry's name, and a tooltip on touch is a tap-to-open box rather than a - * hover hint — so it says the same thing twice and covers the map to do it - * (discussion #2299). On desktop the label is the only name a marker has, so - * this stays off there. + * On a phone the card is what a tap on a pin brings up, so it has to lead + * somewhere (#2885). Wiring it is what makes the card take the pointer; without + * it the card stays a label the map's drags pass through, as on desktop, where + * it is a hover card the mouse cannot reach. */ - hideMarkerTooltip?: boolean + onMarkerCardClick?: (entryId: string) => void paddingBottom?: number /** CARTO key from the share payload: the public journey has no settings store to read. */ cartoApiKey?: string @@ -185,6 +184,29 @@ function panToMarker(map: L.Map, marker: L.Marker): void { }) } +/** + * Let a tap on a marker card open its entry (#2885). + * + * Handled on the card's own element rather than through Leaflet's interactive + * tooltip, and kept away from the map. A press that reaches the map container + * has Leaflet's keyboard handler focus it, which blurs the pin that opened the + * card, and Leaflet closes a pin's card on blur, so the card was gone before its + * click landed. Holding the mouse press back from the browser's focus change + * matters for the same reason; a touch press is left alone, since cancelling it + * would cancel the click as well. + */ +function makeCardOpenEntry(el: HTMLElement, open: () => void): void { + L.DomEvent.on(el, 'mousedown', (ev) => { + L.DomEvent.stopPropagation(ev) + ev.preventDefault() + }) + L.DomEvent.on(el, 'touchstart pointerdown', L.DomEvent.stopPropagation) + L.DomEvent.on(el, 'click', (ev) => { + L.DomEvent.stopPropagation(ev) + open() + }) +} + const EMPTY_TRAIL: { lat: number; lng: number }[] = [] const EMPTY_TRACKS: JourneyTrack[] = [] /** Fallback when a track carries no colour of its own, matching the planner's default. */ @@ -201,12 +223,12 @@ function fitOptions(paddingBottom: number | undefined): L.FitBoundsOptions { } function JourneyMap( - { entries, photos, onPhotoClick, trail, tracks, height = 220, dark, activeMarkerId, onMarkerClick, fullScreen, paddingBottom, cartoApiKey, hideMarkerTooltip, layerVisibility = EVERY_LAYER, marks, onMarksPick, plannedEntryIds = NO_PLANNED, ref }: Props, + { entries, photos, onPhotoClick, trail, tracks, height = 220, dark, activeMarkerId, onMarkerClick, fullScreen, paddingBottom, cartoApiKey, onMarkerCardClick, layerVisibility = EVERY_LAYER, marks, onMarksPick, plannedEntryIds = NO_PLANNED, ref }: Props, ) { - // Read through a ref: the flag is fixed per surface, and putting it in the + // Read through a ref: the handler is fixed per surface, and putting it in the // marker effect's deps would rebuild every marker for nothing. - const hideMarkerTooltipRef = useRef(hideMarkerTooltip) - hideMarkerTooltipRef.current = hideMarkerTooltip + const onMarkerCardClickRef = useRef(onMarkerCardClick) + onMarkerCardClickRef.current = onMarkerCardClick const stableTrail = trail || EMPTY_TRAIL const stableTracks = tracks || EMPTY_TRACKS const mapTileUrl = useSettingsStore(s => s.settings.map_tile_url) @@ -428,21 +450,27 @@ function JourneyMap( // tells them (discussion #2299). The builder escapes everything that came // from a person, which matters most here — this map is what the public // journey page renders. - if (!hideMarkerTooltipRef.current) { - ensureJourneyPopupStyle() - marker.bindTooltip( - journeyPopupHtml({ - title: item.label || item.locationName || 'Entry', - place: item.label ? item.locationName : '', - date: formatMarkerDate(item.time), - photoUrls: item.photoUrls, - }), - { - direction: 'top', - offset: [0, -MARKER_H], - className: 'map-tooltip trek-journey-tooltip', - }, - ) + ensureJourneyPopupStyle() + const openable = !!onMarkerCardClickRef.current + marker.bindTooltip( + journeyPopupHtml({ + title: item.label || item.locationName || 'Entry', + place: item.label ? item.locationName : '', + date: formatMarkerDate(item.time), + photoUrls: item.photoUrls, + }), + { + direction: 'top', + offset: [0, -MARKER_H], + className: `map-tooltip trek-journey-tooltip${openable ? ' is-openable' : ''}`, + }, + ) + // The card's element is built on its first open and kept for every later one. + if (openable) { + marker.getTooltip()?.once('add', (e: L.LeafletEvent) => { + const el = (e.target as L.Tooltip).getElement() + if (el) makeCardOpenEntry(el, () => onMarkerCardClickRef.current?.(item.id)) + }) } marker.on('click', () => { diff --git a/client/src/components/Journey/JourneyMapAuto.tsx b/client/src/components/Journey/JourneyMapAuto.tsx index 494cc05ad..723f2b6cf 100644 --- a/client/src/components/Journey/JourneyMapAuto.tsx +++ b/client/src/components/Journey/JourneyMapAuto.tsx @@ -39,7 +39,8 @@ interface Props { onMarkerClick?: (id: string, type?: string) => void fullScreen?: boolean paddingBottom?: number - hideMarkerTooltip?: boolean + /** Open the entry a marker card names when the card is tapped, both renderers. */ + onMarkerCardClick?: (entryId: string) => void /** Open an entry's photos from the marker card's thumbnail strip. GL renderer only. */ onMarkerPhotoClick?: (entryId: string, photoIndex: number) => void /** Geotagged photos as thumbnails on the map, both renderers (#2453). */ diff --git a/client/src/components/Journey/JourneyMapGL.test.tsx b/client/src/components/Journey/JourneyMapGL.test.tsx index 9248a7b61..e3dab65f1 100644 --- a/client/src/components/Journey/JourneyMapGL.test.tsx +++ b/client/src/components/Journey/JourneyMapGL.test.tsx @@ -1,4 +1,4 @@ -// FE-COMP-JMAPGL-001 to FE-COMP-JMAPGL-043, FE-COMP-JMAPGL-2453 +// FE-COMP-JMAPGL-001 to FE-COMP-JMAPGL-048, FE-COMP-JMAPGL-2453 type Spy = ReturnType @@ -507,6 +507,86 @@ describe('JourneyMapGL', () => { expect(document.querySelectorAll('#trek-journey-popup-style')).toHaveLength(1) }) + describe('the marker card as a way into the entry (#2885)', () => { + const withPhotos = [ + { ...entries[0], photoUrls: ['/a.jpg', '/b.jpg'] }, + entries[1], + ] + + // The stub's setHTML records the markup; the card's content is put in by hand. + function openCard(props: Record, id = 'e1') { + withToken() + const ref = React.createRef() + render() + act(() => { ref.current?.highlightMarker(id) }) + const popup = gl.popups[0] + popup.element.innerHTML = String(popup.setHTML.mock.calls.at(-1)?.[0]) + const mapClick = vi.fn() + const host = document.createElement('div') + host.appendChild(popup.element) + host.addEventListener('click', mapClick) + return { ref, popup, mapClick } + } + + function tap(popup: PopupStub, selector: string) { + const target = popup.element.querySelector(selector) as HTMLElement + target.click() + } + + it('FE-COMP-JMAPGL-044: a tap anywhere on the card opens its entry and stays off the map', () => { + const onMarkerCardClick = vi.fn() + const { popup, mapClick } = openCard({ onMarkerCardClick }) + + expect(popup.options.className).toContain('is-openable') + tap(popup, '.trek-journey-popup-title') + + expect(onMarkerCardClick).toHaveBeenCalledWith('e1') + expect(mapClick).not.toHaveBeenCalled() + }) + + it('FE-COMP-JMAPGL-045: with a photo handler a thumbnail opens the photo, not the entry', () => { + const onMarkerCardClick = vi.fn() + const onMarkerPhotoClick = vi.fn() + const { popup } = openCard({ onMarkerCardClick, onMarkerPhotoClick }) + + tap(popup, '[data-shot="1"]') + + expect(onMarkerPhotoClick).toHaveBeenCalledWith('e1', 1) + expect(onMarkerCardClick).not.toHaveBeenCalled() + }) + + it('FE-COMP-JMAPGL-046: without a photo handler a thumbnail opens the entry like the rest of the card', () => { + const onMarkerCardClick = vi.fn() + const { popup } = openCard({ onMarkerCardClick }) + + tap(popup, '[data-shot]') + + expect(onMarkerCardClick).toHaveBeenCalledWith('e1') + }) + + it('FE-COMP-JMAPGL-047: without a handler the card stays a label the tap passes through', () => { + const { popup, mapClick } = openCard({}) + + expect(popup.options.className).toBe('trek-journey-popup') + tap(popup, '.trek-journey-popup-title') + + expect(mapClick).toHaveBeenCalled() + }) + + it('FE-COMP-JMAPGL-048: the reused card opens the entry it shows now', () => { + const onMarkerCardClick = vi.fn() + const { ref, popup } = openCard({ onMarkerCardClick }) + + act(() => { ref.current?.highlightMarker('e2') }) + popup.element.innerHTML = String(popup.setHTML.mock.calls.at(-1)?.[0]) + tap(popup, '.trek-journey-popup-title') + + expect(popup.element.classList.contains('is-openable')).toBe(true) + expect(popup.element.classList.contains('is-interactive')).toBe(false) + expect(onMarkerCardClick).toHaveBeenCalledWith('e2') + }) + }) + it('FE-COMP-JMAPGL-023: focusMarker eases over with the 3D pitch and no zoom of its own', () => { withToken() const ref = React.createRef() diff --git a/client/src/components/Journey/JourneyMapGL.tsx b/client/src/components/Journey/JourneyMapGL.tsx index 064a2fb9b..b0f887481 100644 --- a/client/src/components/Journey/JourneyMapGL.tsx +++ b/client/src/components/Journey/JourneyMapGL.tsx @@ -43,8 +43,8 @@ interface Props { activeMarkerId?: string | null onMarkerClick?: (id: string, type?: string) => void fullScreen?: boolean - /** See the Leaflet twin: no marker labels where a carousel already names the entry. */ - hideMarkerTooltip?: boolean + /** See the Leaflet twin: a tap on the card opens the entry it names (#2885). */ + onMarkerCardClick?: (entryId: string) => void /** * Open the entry's photos from the card's thumbnail strip. * @@ -194,13 +194,13 @@ function fitOptions(paddingBottom: number | undefined, tilted: boolean | undefin } function JourneyMapGL( - { entries, trail, tracks, height = 220, dark, activeMarkerId, onMarkerClick, fullScreen, paddingBottom, glProvider = 'mapbox-gl', gl, hideMarkerTooltip, onMarkerPhotoClick, photos, onPhotoClick, layerVisibility = EVERY_LAYER, marks, onMarksPick, plannedEntryIds = NO_PLANNED, ref }: Props, + { entries, trail, tracks, height = 220, dark, activeMarkerId, onMarkerClick, fullScreen, paddingBottom, glProvider = 'mapbox-gl', gl, onMarkerCardClick, onMarkerPhotoClick, photos, onPhotoClick, layerVisibility = EVERY_LAYER, marks, onMarksPick, plannedEntryIds = NO_PLANNED, ref }: Props, ) { - const hideMarkerTooltipRef = useRef(hideMarkerTooltip) - hideMarkerTooltipRef.current = hideMarkerTooltip + const onMarkerCardClickRef = useRef(onMarkerCardClick) + onMarkerCardClickRef.current = onMarkerCardClick const onMarkerPhotoClickRef = useRef(onMarkerPhotoClick) onMarkerPhotoClickRef.current = onMarkerPhotoClick - /** Which entry the card is currently describing, for the delegated photo click. */ + /** Which entry the card is currently describing, for the delegated card and photo clicks. */ const popupItemIdRef = useRef(null) const stableTrail = trail || EMPTY_TRAIL const stableTracks = tracks || EMPTY_TRACKS @@ -252,9 +252,6 @@ function JourneyMapGL( mapLangRef.current = mapLang const showPopup = useCallback((id: string) => { - // See `hideMarkerTooltip` on the Leaflet twin: below the carousel the card - // already says this, and the popup only covers the map to repeat it. - if (hideMarkerTooltipRef.current) return const item = itemsRef.current.find(i => i.id === id) if (!item || !mapRef.current) return ensureJourneyPopupStyle() @@ -272,6 +269,7 @@ function JourneyMapGL( const offset: [number, number] = [0, -46] popupItemIdRef.current = item.id const interactive = !!onMarkerPhotoClickRef.current && item.photoUrls.length > 0 + const openable = !!onMarkerCardClickRef.current if (popupRef.current) { popupRef.current.setLngLat([item.lng, item.lat]) popupRef.current.setHTML(html) @@ -280,6 +278,7 @@ function JourneyMapGL( if (el) { el.classList.toggle('trek-dark', !!darkRef.current) el.classList.toggle('is-interactive', interactive) + el.classList.toggle('is-openable', openable) } } else { popupRef.current = new gl.Popup({ @@ -288,7 +287,7 @@ function JourneyMapGL( closeOnMove: false, anchor: 'bottom', offset, - className: `trek-journey-popup${darkRef.current ? ' trek-dark' : ''}${interactive ? ' is-interactive' : ''}`, + className: `trek-journey-popup${darkRef.current ? ' trek-dark' : ''}${interactive ? ' is-interactive' : ''}${openable ? ' is-openable' : ''}`, maxWidth: '280px', }) .setLngLat([item.lng, item.lat]) @@ -298,11 +297,18 @@ function JourneyMapGL( // strip on every entry, so a listener on the thumbnails themselves would have // to be rebound each time and would leak the ones it forgot. popupRef.current.getElement()?.addEventListener('click', (ev: Event) => { - const shot = (ev.target as HTMLElement | null)?.closest(`[${SHOT_INDEX_ATTR}]`) const entryId = popupItemIdRef.current - if (!shot || !entryId) return - ev.stopPropagation() - onMarkerPhotoClickRef.current?.(entryId, Number(shot.getAttribute(SHOT_INDEX_ATTR))) + if (!entryId) return + const shot = (ev.target as HTMLElement | null)?.closest(`[${SHOT_INDEX_ATTR}]`) + const onPhoto = onMarkerPhotoClickRef.current + const onCard = onMarkerCardClickRef.current + if (shot && onPhoto) { + ev.stopPropagation() + onPhoto(entryId, Number(shot.getAttribute(SHOT_INDEX_ATTR))) + } else if (onCard) { + ev.stopPropagation() + onCard(entryId) + } }) } }, [gl]) diff --git a/client/src/components/Journey/MobileMapTimeline.test.tsx b/client/src/components/Journey/MobileMapTimeline.test.tsx index f35817bbd..4c6227c78 100644 --- a/client/src/components/Journey/MobileMapTimeline.test.tsx +++ b/client/src/components/Journey/MobileMapTimeline.test.tsx @@ -1,4 +1,4 @@ -// FE-COMP-JMAPTL-001 to FE-COMP-JMAPTL-016 +// FE-COMP-JMAPTL-001 to FE-COMP-JMAPTL-018 const mapHandle = vi.hoisted(() => ({ highlightMarker: vi.fn(), @@ -190,6 +190,16 @@ describe('MobileMapTimeline', () => { expect(Element.prototype.scrollIntoView).toHaveBeenCalled() }) + it('FE-COMP-JMAPTL-018: a tap on a marker card opens that entry straight away (#2885)', () => { + const onEntryClick = vi.fn() + renderTimeline({ onEntryClick }) + const onMarkerCardClick = lastMapProps.current.onMarkerCardClick as (id: string) => void + + act(() => { onMarkerCardClick('2') }) + + expect(onEntryClick).toHaveBeenCalledWith(entries[1]) + }) + it('FE-COMP-JMAPTL-007: a marker click for an unknown entry is ignored', () => { renderTimeline() const onMarkerClick = lastMapProps.current.onMarkerClick as (id: string) => void diff --git a/client/src/components/Journey/MobileMapTimeline.tsx b/client/src/components/Journey/MobileMapTimeline.tsx index cf6202013..28acc53c8 100644 --- a/client/src/components/Journey/MobileMapTimeline.tsx +++ b/client/src/components/Journey/MobileMapTimeline.tsx @@ -70,7 +70,7 @@ export default function MobileMapTimeline({ }: Props) { const { mapRef, carouselRef, cardRefs, activeIndex, setActiveIndex, scrubberDays, - syncMapToCard: syncMapToCarousel, handleMarkerClick, handleCardTap, jumpToDay, + syncMapToCard: syncMapToCarousel, handleMarkerClick, handleCardTap, openEntryById, jumpToDay, } = useJourneyCarouselSync({ entries, mapEntries, onOpenEntry: onEntryClick }) // The delayed initial focus reads both through refs so it always works off // the current map entries, not the ones from the render that armed the timer. @@ -169,6 +169,7 @@ export default function MobileMapTimeline({ dark={dark} activeMarkerId={activeEntryId} onMarkerClick={handleMarkerClick} + onMarkerCardClick={openEntryById} fullScreen paddingBottom={250} cartoApiKey={cartoApiKey} diff --git a/client/src/components/Journey/journeyMapPopup.ts b/client/src/components/Journey/journeyMapPopup.ts index 94412931e..10e935563 100644 --- a/client/src/components/Journey/journeyMapPopup.ts +++ b/client/src/components/Journey/journeyMapPopup.ts @@ -93,11 +93,24 @@ export function ensureJourneyPopupStyle(): void { s.textContent = ` .mapboxgl-popup.trek-journey-popup, .maplibregl-popup.trek-journey-popup { pointer-events: none; animation: trek-journey-popup-in 180ms ease-out; } - /* The card as a whole never takes the pointer — it sits over the map and would - swallow drags. The photo strip is the one exception, and only where the host - wired a handler for it. */ + /* The card sits over the map and takes the pointer only where the host gave it + an action: the whole card where it opens its entry (the phone, where a tap on + a pin is what brings it up), the photo strip where it opens the pictures. + Anywhere else it stays a label the map's drags pass through. */ + .mapboxgl-popup.trek-journey-popup.is-openable .mapboxgl-popup-content, + .maplibregl-popup.trek-journey-popup.is-openable .maplibregl-popup-content { pointer-events: auto; cursor: pointer; } .trek-journey-popup.is-interactive .trek-journey-popup-shots { pointer-events: auto; cursor: zoom-in; } .trek-journey-popup.is-interactive .trek-journey-popup-shot:hover { filter: brightness(1.06); } + /* The shared map-tooltip style turns the pointer off on every Leaflet tooltip. */ + .leaflet-tooltip.map-tooltip.trek-journey-tooltip.is-openable { pointer-events: auto; cursor: pointer; } + /* The press shows on what the card holds: Leaflet positions the tooltip with its + own transform and sets its opacity inline, so the shell itself is off limits. */ + .mapboxgl-popup.trek-journey-popup.is-openable .mapboxgl-popup-content > *, + .maplibregl-popup.trek-journey-popup.is-openable .maplibregl-popup-content > *, + .leaflet-tooltip.trek-journey-tooltip.is-openable > * { transition: opacity 120ms ease-out; } + .mapboxgl-popup.trek-journey-popup.is-openable .mapboxgl-popup-content:active > *, + .maplibregl-popup.trek-journey-popup.is-openable .maplibregl-popup-content:active > *, + .leaflet-tooltip.trek-journey-tooltip.is-openable:active > * { opacity: 0.6; } .mapboxgl-popup.trek-journey-popup .mapboxgl-popup-content, .maplibregl-popup.trek-journey-popup .maplibregl-popup-content, .leaflet-tooltip.trek-journey-tooltip { diff --git a/client/src/components/Journey/useJourneyCarouselSync.test.ts b/client/src/components/Journey/useJourneyCarouselSync.test.ts index 5eec893ff..1204664f4 100644 --- a/client/src/components/Journey/useJourneyCarouselSync.test.ts +++ b/client/src/components/Journey/useJourneyCarouselSync.test.ts @@ -1,4 +1,4 @@ -// FE-JRN-CAROUSEL-001 to FE-JRN-CAROUSEL-007: the card carousel over the journey map, +// FE-JRN-CAROUSEL-001 to FE-JRN-CAROUSEL-008: the card carousel over the journey map, // behind both the phone journey screen and the narrow desktop and public timeline. import { act, renderHook } from '@testing-library/react'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; @@ -136,4 +136,13 @@ describe('useJourneyCarouselSync', () => { expect(hook.result.current.activeIndex).toBe(1); expect(map.highlightMarker).toHaveBeenCalledWith(null); }); + + it('FE-JRN-CAROUSEL-008: a marker card opens its entry by id, wherever the carousel stands (#2885)', () => { + const { result, props } = setup(); + act(() => result.current.openEntryById('3')); + expect(props.onOpenEntry).toHaveBeenCalledWith(ENTRIES[2]); + expect(result.current.activeIndex).toBe(0); + act(() => result.current.openEntryById('99')); + expect(props.onOpenEntry).toHaveBeenCalledTimes(1); + }); }); diff --git a/client/src/components/Journey/useJourneyCarouselSync.ts b/client/src/components/Journey/useJourneyCarouselSync.ts index 6a8b052e2..39adf1ce1 100644 --- a/client/src/components/Journey/useJourneyCarouselSync.ts +++ b/client/src/components/Journey/useJourneyCarouselSync.ts @@ -123,6 +123,16 @@ export function useJourneyCarouselSync({ [activeIndex, onOpenEntry, scrollCardIntoCenter, syncMapOnTap, syncMapToCard] ); + // The marker card on the map names one entry, so a tap on it opens that entry + // straight away, the same way a tap on the active carousel card does (#2885). + const openEntryById = useCallback( + (id: string) => { + const entry = entries.find((e) => String(e.id) === id); + if (entry) onOpenEntry(entry); + }, + [entries, onOpenEntry] + ); + // The day bar lands on the first entry of that day, which is where a reader who // asked for "day nine" means. const jumpToDay = useCallback( @@ -146,6 +156,7 @@ export function useJourneyCarouselSync({ syncMapToCard, handleMarkerClick, handleCardTap, + openEntryById, jumpToDay, }; } diff --git a/client/src/mobile/screens/journey/MJourneyDetail.tsx b/client/src/mobile/screens/journey/MJourneyDetail.tsx index af40f67a4..a03b9c9ad 100644 --- a/client/src/mobile/screens/journey/MJourneyDetail.tsx +++ b/client/src/mobile/screens/journey/MJourneyDetail.tsx @@ -103,7 +103,7 @@ export default function MJourneyDetail({ journeyId, embedded = false }: JourneyD const { mapRef, carouselRef, cardRefs, activeIndex, setActiveIndex, scrubberDays, - syncMapToCard, handleMarkerClick, handleCardTap, jumpToDay, + syncMapToCard, handleMarkerClick, handleCardTap, openEntryById, jumpToDay, } = useJourneyCarouselSync({ entries, mapEntries: sidebarMapItems, onOpenEntry: setEditingEntry, syncMapOnTap: true }) // Initial focus — give Leaflet time to initialise and fit bounds first. Opens on @@ -317,6 +317,7 @@ export default function MJourneyDetail({ journeyId, embedded = false }: JourneyD dark={dark} activeMarkerId={entries[activeIndex] ? String(entries[activeIndex].id) : null} onMarkerClick={handleMarkerClick} + onMarkerCardClick={openEntryById} fullScreen paddingBottom={250} /> diff --git a/client/tests/unit/mobile/journey/MJourneyDetail.test.tsx b/client/tests/unit/mobile/journey/MJourneyDetail.test.tsx index e5aa97188..6e3bf24d7 100644 --- a/client/tests/unit/mobile/journey/MJourneyDetail.test.tsx +++ b/client/tests/unit/mobile/journey/MJourneyDetail.test.tsx @@ -1,4 +1,4 @@ -// FE-MOB-JDET-001 to FE-MOB-JDET-045 +// FE-MOB-JDET-001 to FE-MOB-JDET-046 import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import { render, screen, fireEvent, waitFor, act } from '../../../helpers/render'; import MJourneyDetail from '../../../../src/mobile/screens/journey/MJourneyDetail'; @@ -30,11 +30,15 @@ vi.mock('../../../../src/components/Journey/JourneyMapAuto', async () => { })); const entries = props.entries as { id: string }[]; const onMarkerClick = props.onMarkerClick as ((id: string) => void) | undefined; + const onMarkerCardClick = props.onMarkerCardClick as ((id: string) => void) | undefined; return (
{entries.map(e => ( ))} + {onMarkerCardClick && entries.map(e => ( + + ))}
); }); @@ -282,6 +286,12 @@ describe('MJourneyDetail', () => { expect(screen.getByTestId('journey-map')).toHaveAttribute('data-active', '2'); }); + it('FE-MOB-JDET-046: a tap on a marker card opens that entry in the sheet (#2885)', () => { + const { hook } = setup(); + fireEvent.click(screen.getByRole('button', { name: 'marker-card-2' })); + expect(hook.setEditingEntry).toHaveBeenCalledWith(expect.objectContaining({ id: 2 })); + }); + it('FE-MOB-JDET-007: a marker without a matching entry leaves the selection alone', () => { const current = buildDetail(); setup({