From a1c7ea209b31a05f5feeee96c2d18447da9e6574 Mon Sep 17 00:00:00 2001 From: Maurice Date: Thu, 29 Oct 2026 17:12:27 +0100 Subject: [PATCH] fix(journey): open the entry when its marker card is tapped on the phone On the phone journey screen and the public share page on a phone, a tap on a numbered pin brings up the entry's card above it, but the card could not be tapped: the GL popup and the Leaflet tooltip both let the pointer fall through to the map, and no phone host gave the card anything to do. The map now takes onMarkerCardClick, and both renderers make the card open that entry once a host wires it. The GL popup handles it in its delegated listener next to the photo strip, which keeps opening the photos where the desktop asks for that. Leaflet handles it on the tooltip element itself: a press that reached the map container let Leaflet focus the map, which blurred the pin and closed the card before its click landed. The phone screen and the timeline open the entry through the carousel hook, the same way a tap on the active card does. The desktop asides keep the card as a hover label. The unused hideMarkerTooltip prop goes, together with the docs that still said the phone hides the card. Closes #2885 --- .../components/Journey/JourneyMap.test.tsx | 70 +++++++++++++++- client/src/components/Journey/JourneyMap.tsx | 80 ++++++++++++------ .../src/components/Journey/JourneyMapAuto.tsx | 3 +- .../components/Journey/JourneyMapGL.test.tsx | 82 ++++++++++++++++++- .../src/components/Journey/JourneyMapGL.tsx | 34 ++++---- .../Journey/MobileMapTimeline.test.tsx | 12 ++- .../components/Journey/MobileMapTimeline.tsx | 3 +- .../src/components/Journey/journeyMapPopup.ts | 19 ++++- .../Journey/useJourneyCarouselSync.test.ts | 11 ++- .../Journey/useJourneyCarouselSync.ts | 11 +++ .../mobile/screens/journey/MJourneyDetail.tsx | 3 +- .../mobile/journey/MJourneyDetail.test.tsx | 12 ++- 12 files changed, 289 insertions(+), 51 deletions(-) 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({