From e11fb8f7c10a3da8a90f11629c7337015859483d Mon Sep 17 00:00:00 2001 From: Florian Date: Tue, 3 Nov 2026 22:03:38 +0000 Subject: [PATCH 1/2] fix(web): keep the long-press pin instead of a menu that closes on release On a phone, holding a finger on the map opened the desktop right-click menu under it, and the menu closed again as soon as the finger lifted. MapLibre 6.16 makes up its own contextmenu event when a finger rests on the map for 500 ms. It is a plain MouseEvent with no pointer type, so the context menu's touch check let it through. The menu then closed on the lift: its click-away listener reacts to touchend, and the touchend of the finger that opened it counted as a tap outside. Touch never opens that menu now; the long-press drops the gray pin, whose card carries the same actions. A shared check in lib/mapTouch treats an event with no pointer information as touch while a finger is down or has just lifted, wherever on the page it touched, so MapLibre's event is recognised whichever component asks. The click some browsers send when a long-press lifts is now swallowed counting from the lift, not from when the pin dropped. Before, a hold longer than about 1.5 s let that click through as a tap, which cleared the pin it had just dropped. A new touch ends that window, so the next tap still clears the pin. --- .../components/map/MapClickHandler.test.tsx | 17 +++++ .../src/components/map/MapClickHandler.tsx | 4 +- .../components/map/MapContextMenu.test.tsx | 31 ++++++++ .../web/src/components/map/MapContextMenu.tsx | 12 ++-- apps/web/src/lib/mapTouch.test.ts | 55 ++++++++++++--- apps/web/src/lib/mapTouch.ts | 70 ++++++++++++++----- apps/web/src/test/setup.ts | 4 ++ 7 files changed, 160 insertions(+), 33 deletions(-) diff --git a/apps/web/src/components/map/MapClickHandler.test.tsx b/apps/web/src/components/map/MapClickHandler.test.tsx index 7608e5f8c..b2bd3971f 100644 --- a/apps/web/src/components/map/MapClickHandler.test.tsx +++ b/apps/web/src/components/map/MapClickHandler.test.tsx @@ -70,6 +70,7 @@ beforeEach(() => { }); afterEach(() => { + touch("touchend", []); vi.useRealTimers(); }); @@ -114,6 +115,22 @@ describe("MapClickHandler on a touch screen", () => { expect(useMapClickStore.getState().clickedLngLat).toEqual([7, 51]); }); + it("keeps the pin when the finger lifts long after it dropped", () => { + vi.useFakeTimers(); + render(); + touch("touchstart", [[20, 30]]); + vi.advanceTimersByTime(LONG_PRESS_MS + 2500); + touch("touchend", []); + click("touch"); + expect(useMapClickStore.getState().clickedLngLat).toEqual([7, 51]); + + // The next tap is a tap of its own, and clears the pin. + touch("touchstart", [[20, 30]]); + touch("touchend", []); + click("touch"); + expect(useMapClickStore.getState().clickedLngLat).toBeNull(); + }); + it("leaves a long-press to the parking picker while it is choosing a spot", () => { vi.useFakeTimers(); useParkingStore.setState({ picking: true }); diff --git a/apps/web/src/components/map/MapClickHandler.tsx b/apps/web/src/components/map/MapClickHandler.tsx index 4be7eef4d..1e863052d 100644 --- a/apps/web/src/components/map/MapClickHandler.tsx +++ b/apps/web/src/components/map/MapClickHandler.tsx @@ -21,7 +21,7 @@ import { bindMapMarkerActivation } from "@/integration-api/map/mapMarkerActivati import { closePlaceSelection } from "@/lib/closeSurfaces"; import { haptics } from "@/lib/haptics"; import { initializeMapClickModes } from "@/lib/mapClickModes"; -import { bindMapLongPress, consumeLongPressClick, isTouchClick } from "@/lib/mapTouch"; +import { bindMapLongPress, cameFromTouch, consumeLongPressClick } from "@/lib/mapTouch"; // Configure before any map listener mounts. Read live mode state at the first // callback for each click; capture may clear that state before later callbacks. @@ -106,7 +106,7 @@ export function MapClickHandler() { } } - if (isTouchClick(e.originalEvent)) { + if (cameFromTouch(e.originalEvent)) { dismissOnTap(); return; } diff --git a/apps/web/src/components/map/MapContextMenu.test.tsx b/apps/web/src/components/map/MapContextMenu.test.tsx index ab5bf8b6a..da6c2ee2b 100644 --- a/apps/web/src/components/map/MapContextMenu.test.tsx +++ b/apps/web/src/components/map/MapContextMenu.test.tsx @@ -313,6 +313,37 @@ describe("MapContextMenu opening", () => { }); } + it("leaves MapLibre's own long-press contextmenu to the dropped pin", () => { + render(); + act(() => useMapClickStore.setState({ clickedLngLat: [1, 2] })); + const fingers = (type: string, count: number) => { + const event = new Event(type, { bubbles: true }); + Object.defineProperty(event, "touches", { + value: Array.from({ length: count }, () => ({ clientX: 120, clientY: 140 })), + }); + document.body.dispatchEvent(event); + }; + fingers("touchstart", 1); + // What MapLibre fires once a finger has rested on the map: a plain + // MouseEvent, with no pointer type or source capabilities to tell touch. + act(() => { + fake.emit("contextmenu", { + point: { x: 40, y: 50 }, + lngLat: { lng: 6.08, lat: 50.77 }, + originalEvent: new MouseEvent("contextmenu", { + clientX: 120, + clientY: 140, + button: 2, + cancelable: true, + }), + }); + }); + fingers("touchend", 0); + + expect(screen.queryByRole("menu")).toBeNull(); + expect(useMapClickStore.getState().clickedLngLat).toEqual([1, 2]); + }); + for (const status of ["navigating", "rerouting"] as const) { it(`suppresses the native menu without opening or mutating state while ${status}`, () => { useNavigationStore.setState({ status }); diff --git a/apps/web/src/components/map/MapContextMenu.tsx b/apps/web/src/components/map/MapContextMenu.tsx index 1716c7a6e..5020758bf 100644 --- a/apps/web/src/components/map/MapContextMenu.tsx +++ b/apps/web/src/components/map/MapContextMenu.tsx @@ -44,6 +44,7 @@ import Popover from "@/components/ui/Popover"; import { INTERACTIVE_LAYER_IDS } from "@/integration-api/map/interactiveLayers"; import { useMap } from "@/integration-api/map/MapContext"; import { openLocationDetails, routeFromOrTo, shareLocationLink } from "@/lib/locationActions"; +import { cameFromTouch } from "@/lib/mapTouch"; import { findStylePoiAtPoint, getStylePoiLayerIds, type StylePoiTarget } from "./mapStylePoiTarget"; interface MapContextTarget { @@ -59,13 +60,6 @@ function formatContextCoordinates([lng, lat]: [number, number]): string { return `${lat.toFixed(6)}, ${lng.toFixed(6)}`; } -function cameFromTouch(event: MouseEvent & { pointerType?: string }): boolean { - const capabilities = ( - event as MouseEvent & { sourceCapabilities?: { firesTouchEvents?: boolean } } - ).sourceCapabilities; - return event.pointerType === "touch" || capabilities?.firesTouchEvents === true; -} - export function MapContextMenu(): React.ReactNode { const t = useTranslations("mapContextMenu"); const tParking = useTranslations("parking"); @@ -200,6 +194,10 @@ export function MapContextMenu(): React.ReactNode { originalEvent.preventDefault(); return; } + // Touch never opens this menu: a long-press drops the pin, whose card + // carries these actions (MapClickHandler). That includes the contextmenu + // MapLibre makes up for a held finger, which would otherwise open the + // menu under it only for the click-away listener to close it on the lift. if (cameFromTouch(originalEvent)) return; originalEvent.preventDefault(); captureTarget([event.lngLat.lng, event.lngLat.lat], event.point, { diff --git a/apps/web/src/lib/mapTouch.test.ts b/apps/web/src/lib/mapTouch.test.ts index f58eb0aef..f12d518b7 100644 --- a/apps/web/src/lib/mapTouch.test.ts +++ b/apps/web/src/lib/mapTouch.test.ts @@ -2,7 +2,7 @@ import type * as maplibregl from "maplibre-gl"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { bindMapLongPress, consumeLongPressClick, isTouchClick, LONG_PRESS_MS } from "./mapTouch"; +import { bindMapLongPress, cameFromTouch, consumeLongPressClick, LONG_PRESS_MS } from "./mapTouch"; function touch(target: EventTarget, type: string, points: Array<[number, number]>): void { const event = new Event(type, { bubbles: true }); @@ -35,6 +35,7 @@ beforeEach(() => { }); afterEach(() => { + touch(canvas, "touchend", []); unbind(); container.remove(); consumeLongPressClick(); @@ -83,26 +84,64 @@ describe("bindMapLongPress", () => { it("swallows the one click a long-press may leave behind", () => { touch(canvas, "touchstart", [[30, 40]]); vi.advanceTimersByTime(LONG_PRESS_MS); + touch(canvas, "touchend", []); + expect(consumeLongPressClick()).toBe(true); + expect(consumeLongPressClick()).toBe(false); + }); + + it("swallows it however long the finger was held, counting from the lift", () => { + touch(canvas, "touchstart", [[30, 40]]); + vi.advanceTimersByTime(LONG_PRESS_MS + 3000); + touch(canvas, "touchend", []); + vi.advanceTimersByTime(20); expect(consumeLongPressClick()).toBe(true); + }); + + it("lets the next tap through when the long-press left no click behind", () => { + touch(canvas, "touchstart", [[30, 40]]); + vi.advanceTimersByTime(LONG_PRESS_MS); + touch(canvas, "touchend", []); + touch(canvas, "touchstart", [[30, 40]]); + touch(canvas, "touchend", []); + expect(consumeLongPressClick()).toBe(false); + }); + + it("leaves a plain tap's click alone", () => { + touch(canvas, "touchstart", [[30, 40]]); + vi.advanceTimersByTime(100); + touch(canvas, "touchend", []); expect(consumeLongPressClick()).toBe(false); }); }); -describe("isTouchClick", () => { - it("trusts a click that says it came from touch", () => { - expect(isTouchClick({ pointerType: "touch" })).toBe(true); - expect(isTouchClick({ sourceCapabilities: { firesTouchEvents: true } })).toBe(true); +describe("cameFromTouch", () => { + it("trusts an event that says it came from touch", () => { + expect(cameFromTouch({ pointerType: "touch" })).toBe(true); + expect(cameFromTouch({ sourceCapabilities: { firesTouchEvents: true } })).toBe(true); }); it("treats a click right after a touch on the map as that touch (Safari)", () => { touch(canvas, "touchstart", [[30, 40]]); touch(canvas, "touchend", []); - expect(isTouchClick({})).toBe(true); + expect(cameFromTouch({})).toBe(true); }); - it("treats a mouse click long after any touch as a mouse click", () => { + it("treats MapLibre's long-press contextmenu, a plain MouseEvent, as touch while the finger is down", () => { touch(canvas, "touchstart", [[30, 40]]); vi.advanceTimersByTime(5000); - expect(isTouchClick({ pointerType: "mouse" })).toBe(false); + expect(cameFromTouch(new MouseEvent("contextmenu", { button: 2 }))).toBe(true); + }); + + it("sees a touch that never reached the map", () => { + touch(document.body, "touchstart", [[30, 40]]); + expect(cameFromTouch(new MouseEvent("contextmenu"))).toBe(true); + }); + + it("treats a mouse event long after any touch as the mouse", () => { + touch(canvas, "touchstart", [[30, 40]]); + touch(canvas, "touchend", []); + vi.advanceTimersByTime(5000); + expect(cameFromTouch({ pointerType: "mouse" })).toBe(false); + expect(cameFromTouch(new MouseEvent("contextmenu", { button: 2 }))).toBe(false); }); }); diff --git a/apps/web/src/lib/mapTouch.ts b/apps/web/src/lib/mapTouch.ts index 4060f0209..2451f8ca8 100644 --- a/apps/web/src/lib/mapTouch.ts +++ b/apps/web/src/lib/mapTouch.ts @@ -8,30 +8,61 @@ const LONG_PRESS_TOLERANCE_PX = 10; const TOUCH_CLICK_WINDOW_MS = 1000; let lastTouchAt = Number.NEGATIVE_INFINITY; -let lastLongPressAt = Number.NEGATIVE_INFINITY; +/** Fingers on the screen right now. */ +let touchesDown = 0; +/** Whether the finger still down has already made a long-press. */ +let longPressHeld = false; +/** When the finger of the last long-press lifted. */ +let longPressReleasedAt = Number.NEGATIVE_INFINITY; + +function noteTouches(event: TouchEvent): void { + lastTouchAt = performance.now(); + touchesDown = event.touches.length; +} + +// Page-wide, so any code asking `cameFromTouch` sees every touch, whichever +// component is mounted; capturing, so the count is current for the map's own +// listeners. Never removed: the page lives as long as they do. +if (typeof window !== "undefined") { + for (const type of ["touchstart", "touchend", "touchcancel"] as const) { + window.addEventListener(type, noteTouches, { capture: true, passive: true }); + } +} /** - * Whether a map click came from a finger. Chrome and Firefox say so on the - * click itself; Safari does not, so a touch that just happened on the map is - * taken as the source too. + * Whether a map click or context-menu event came from a finger. Chrome and + * Firefox say so on the event itself. Safari does not, and neither does the + * `contextmenu` MapLibre makes up for a touch long-press (a plain MouseEvent), + * so a finger on the screen, or one that just lifted, is taken as the source too. */ -export function isTouchClick(event: unknown): boolean { +export function cameFromTouch(event: unknown): boolean { const pointerEvent = event as - | { pointerType?: string; sourceCapabilities?: { firesTouchEvents?: boolean } } + | { pointerType?: string; sourceCapabilities?: { firesTouchEvents?: boolean } | null } | undefined; if (pointerEvent?.pointerType === "touch") return true; if (pointerEvent?.sourceCapabilities?.firesTouchEvents === true) return true; - return performance.now() - lastTouchAt < TOUCH_CLICK_WINDOW_MS; + return touchesDown > 0 || performance.now() - lastTouchAt < TOUCH_CLICK_WINDOW_MS; +} + +/** Forgets every touch seen so far, as a fresh page would. */ +export function resetMapTouchForTests(): void { + lastTouchAt = Number.NEGATIVE_INFINITY; + touchesDown = 0; + longPressHeld = false; + longPressReleasedAt = Number.NEGATIVE_INFINITY; } /** - * Whether this click is the one some browsers still fire when the finger of a - * long-press lifts. Consumes the long-press, so a later click goes through. + * Whether this click is the one some browsers (iOS Safari among them) still + * fire when the finger of a long-press lifts — however long it was held, as + * the window opens at the lift. Consumes it, so a later click goes through. + * A new touch closes the window: its click is a tap of its own. */ export function consumeLongPressClick(): boolean { - const recent = performance.now() - lastLongPressAt < TOUCH_CLICK_WINDOW_MS; - lastLongPressAt = Number.NEGATIVE_INFINITY; - return recent; + const pending = longPressHeld || performance.now() - longPressReleasedAt < TOUCH_CLICK_WINDOW_MS; + longPressHeld = false; + longPressReleasedAt = Number.NEGATIVE_INFINITY; + return pending; } /** @@ -56,7 +87,10 @@ export function bindMapLongPress( }; const onTouchStart = (event: TouchEvent) => { - lastTouchAt = performance.now(); + if (event.touches.length === 1) { + longPressHeld = false; + longPressReleasedAt = Number.NEGATIVE_INFINITY; + } cancel(); if (event.touches.length !== 1 || event.target !== canvas) return; const touch = event.touches[0]; @@ -67,7 +101,7 @@ export function bindMapLongPress( const rect = canvas.getBoundingClientRect(); const lngLat = map.unproject([start.x - rect.left, start.y - rect.top]); start = null; - lastLongPressAt = performance.now(); + longPressHeld = true; onLongPress([lngLat.lng, lngLat.lat], event); }, LONG_PRESS_MS); }; @@ -84,8 +118,11 @@ export function bindMapLongPress( } }; - const onTouchEnd = () => { - lastTouchAt = performance.now(); + const onTouchEnd = (event: TouchEvent) => { + if (event.touches.length === 0 && longPressHeld) { + longPressHeld = false; + longPressReleasedAt = performance.now(); + } cancel(); }; @@ -95,6 +132,7 @@ export function bindMapLongPress( container.addEventListener("touchcancel", onTouchEnd, { passive: true }); return () => { cancel(); + longPressHeld = false; container.removeEventListener("touchstart", onTouchStart); container.removeEventListener("touchmove", onTouchMove); container.removeEventListener("touchend", onTouchEnd); diff --git a/apps/web/src/test/setup.ts b/apps/web/src/test/setup.ts index 04151c316..242019b46 100644 --- a/apps/web/src/test/setup.ts +++ b/apps/web/src/test/setup.ts @@ -5,6 +5,7 @@ import "@testing-library/jest-dom/vitest"; import { afterEach, vi } from "vitest"; import { isBackNavigationIdle, resetBackNavigationForTests } from "@/lib/backNavigation"; +import { resetMapTouchForTests } from "@/lib/mapTouch"; import { settleHistory } from "./history"; // jsdom does not implement Element.scrollIntoView; components that scroll a @@ -24,4 +25,7 @@ afterEach(async () => { if (typeof window === "undefined") return; if (!vi.isFakeTimers() && !isBackNavigationIdle()) await settleHistory(); resetBackNavigationForTests(); + // A touch one test made would otherwise pass for the source of the next + // test's mouse events. + resetMapTouchForTests(); }); From 861179802a64a90e554002d2af56721958a202b5 Mon Sep 17 00:00:00 2001 From: Florian Date: Tue, 3 Nov 2026 23:06:52 +0000 Subject: [PATCH 2/2] feat(web): give every touch screen the pin card's actions and copy choice Touch never opens the right-click menu, so the dropped-pin card is where its actions live on a touch screen. The card chose its layout by width, though: a tablet or a phone held sideways got the compact desktop card with only Directions, leaving From here, Share, Save parking and Copy out of reach. - The action row shows on any touch-first screen ((pointer: coarse)). Wider than a phone, the card keeps the desktop card's place, 480 px wide, and the row wraps instead of scrolling. A mouse-driven desktop keeps its compact card and right-click menu. - Copy offers the coordinates, the address and a place's name, as the menu's Copy location does; with only one of them it copies directly. - A long-press on a basemap label recognises that place, as right-click does: the card shows its name, routes under it, and Details opens it. The menu and the card now describe a location through one helper (components/map/mapLocation), so the two can't drift apart. --- .../web/src/components/map/MapContextMenu.tsx | 76 ++++------ .../src/components/map/mapLocation.test.ts | 57 +++++++ apps/web/src/components/map/mapLocation.ts | 96 ++++++++++++ .../panels/MapClickFloatingCard.test.tsx | 108 +++++++++++++- .../panels/MapClickFloatingCard.tsx | 139 +++++++++++++----- docs/docs/features/places.md | 11 +- 6 files changed, 394 insertions(+), 93 deletions(-) create mode 100644 apps/web/src/components/map/mapLocation.test.ts create mode 100644 apps/web/src/components/map/mapLocation.ts diff --git a/apps/web/src/components/map/MapContextMenu.tsx b/apps/web/src/components/map/MapContextMenu.tsx index 5020758bf..6a9ed57d3 100644 --- a/apps/web/src/components/map/MapContextMenu.tsx +++ b/apps/web/src/components/map/MapContextMenu.tsx @@ -20,8 +20,6 @@ import { Typography, } from "@mui/material"; import { - coordinateId, - createPlace, isLiveNavigationStatus, type Place, useMapClickStore, @@ -45,21 +43,21 @@ import { INTERACTIVE_LAYER_IDS } from "@/integration-api/map/interactiveLayers"; import { useMap } from "@/integration-api/map/MapContext"; import { openLocationDetails, routeFromOrTo, shareLocationLink } from "@/lib/locationActions"; import { cameFromTouch } from "@/lib/mapTouch"; -import { findStylePoiAtPoint, getStylePoiLayerIds, type StylePoiTarget } from "./mapStylePoiTarget"; +import { + describeMapLocation, + type MapLocation, + mapLocationCopyRows, + mapLocationPlace, +} from "./mapLocation"; +import { findStylePoiAtPoint, getStylePoiLayerIds } from "./mapStylePoiTarget"; -interface MapContextTarget { - coordinates: [number, number]; +interface MapContextTarget extends MapLocation { anchorPosition: { top: number; left: number }; - poi: StylePoiTarget | null; } type ActionId = "from" | "to" | "copy" | "details" | "parking" | "share"; const ACTION_ORDER: ActionId[] = ["from", "to", "copy", "details", "parking", "share"]; -function formatContextCoordinates([lng, lat]: [number, number]): string { - return `${lat.toFixed(6)}, ${lng.toFixed(6)}`; -} - export function MapContextMenu(): React.ReactNode { const t = useTranslations("mapContextMenu"); const tParking = useTranslations("parking"); @@ -98,32 +96,15 @@ export function MapContextMenu(): React.ReactNode { const coordinates = target?.coordinates ?? null; const { data: reverseGeo } = useReverseGeocoding(coordinates, locale); - const coordinateLabel = target ? formatContextCoordinates(target.coordinates) : ""; - const title = target?.poi?.name ?? reverseGeo?.city ?? reverseGeo?.address ?? coordinateLabel; - const address = reverseGeo?.address ?? coordinateLabel; - const actionLabel = target?.poi?.name ?? reverseGeo?.address ?? coordinateLabel; + const text = target ? describeMapLocation(target, reverseGeo) : null; + const title = text?.title ?? ""; + const address = text?.address ?? ""; + const actionLabel = text?.routeLabel ?? ""; - const place = useMemo(() => { - if (!target) return null; - if (target.poi) { - return createPlace({ - primaryScheme: "stylePoi", - ids: { stylePoi: target.poi.featureId }, - name: target.poi.name, - address: reverseGeo?.address ?? target.poi.name, - coordinates: target.poi.coordinates, - category: target.poi.category, - rawCategory: target.poi.rawCategory, - }); - } - return createPlace({ - primaryScheme: "coordinate", - ids: { coordinate: coordinateId(target.coordinates) }, - name: title, - address, - coordinates: target.coordinates, - }); - }, [address, reverseGeo?.address, target, title]); + const place = useMemo( + () => (target ? mapLocationPlace(target, reverseGeo) : null), + [reverseGeo, target], + ); const restoreCanvasFocus = useCallback(() => { if (restoreFocusFrameRef.current !== null) cancelAnimationFrame(restoreFocusFrameRef.current); @@ -305,20 +286,17 @@ export function MapContextMenu(): React.ReactNode { setFeedback(outcome === "saved" ? tParking("savedToast") : tParking("locationUnavailable")); }; - const copyRows = useMemo(() => { - if (!target) return []; - const candidates = [ - { label: t("coordinates"), value: coordinateLabel }, - ...(target.poi?.name ? [{ label: t("name"), value: target.poi.name }] : []), - ...(reverseGeo?.address ? [{ label: t("address"), value: reverseGeo.address }] : []), - ]; - const values = new Set(); - return candidates.filter(({ value }) => { - if (values.has(value)) return false; - values.add(value); - return true; - }); - }, [coordinateLabel, reverseGeo?.address, t, target]); + const copyRows = useMemo( + () => + target + ? mapLocationCopyRows(target, reverseGeo, { + coordinates: t("coordinates"), + name: t("name"), + address: t("address"), + }) + : [], + [reverseGeo, t, target], + ); const copyValue = async (value: string) => { const targetGeneration = targetGenerationRef.current; diff --git a/apps/web/src/components/map/mapLocation.test.ts b/apps/web/src/components/map/mapLocation.test.ts new file mode 100644 index 000000000..a961702fc --- /dev/null +++ b/apps/web/src/components/map/mapLocation.test.ts @@ -0,0 +1,57 @@ +import { describe, expect, it } from "vitest"; +import { describeMapLocation, mapLocationCopyRows, mapLocationPlace } from "./mapLocation"; + +const POINT: [number, number] = [6.083, 50.776]; +const LABELS = { coordinates: "Coordinates", name: "Name", address: "Address" }; +const CAFE = { + featureId: "42", + name: "Café Kaiser", + coordinates: [6.0831, 50.7761] as [number, number], +}; + +describe("describeMapLocation", () => { + it("falls back from the place's name to the city, the address and the coordinates", () => { + const bare = { coordinates: POINT, poi: null }; + expect(describeMapLocation(bare, null).title).toBe("50.776000, 6.083000"); + expect(describeMapLocation(bare, { address: "Markt 1" }).title).toBe("Markt 1"); + expect(describeMapLocation(bare, { address: "Markt 1", city: "Aachen" }).title).toBe("Aachen"); + expect(describeMapLocation({ coordinates: POINT, poi: CAFE }, { city: "Aachen" }).title).toBe( + "Café Kaiser", + ); + }); + + it("routes under the place's name, else the address", () => { + expect( + describeMapLocation({ coordinates: POINT, poi: null }, { address: "Markt 1", city: "Aachen" }) + .routeLabel, + ).toBe("Markt 1"); + expect( + describeMapLocation({ coordinates: POINT, poi: CAFE }, { address: "Markt 1" }).routeLabel, + ).toBe("Café Kaiser"); + }); +}); + +describe("mapLocationPlace", () => { + it("is the basemap place when there is one, else the point", () => { + expect(mapLocationPlace({ coordinates: POINT, poi: CAFE }, null).id).toBe("stylePoi:42"); + expect(mapLocationPlace({ coordinates: POINT, poi: null }, null).coordinates).toEqual(POINT); + }); +}); + +describe("mapLocationCopyRows", () => { + it("lists coordinates, name and address without repeating a value", () => { + const rows = mapLocationCopyRows( + { coordinates: POINT, poi: { ...CAFE, name: "Markt 1" } }, + { address: "Markt 1" }, + LABELS, + ); + expect(rows).toEqual([ + { label: "Coordinates", value: "50.776000, 6.083000" }, + { label: "Name", value: "Markt 1" }, + ]); + }); + + it("offers only the coordinates for a bare point with no address yet", () => { + expect(mapLocationCopyRows({ coordinates: POINT, poi: null }, null, LABELS)).toHaveLength(1); + }); +}); diff --git a/apps/web/src/components/map/mapLocation.ts b/apps/web/src/components/map/mapLocation.ts new file mode 100644 index 000000000..06608de56 --- /dev/null +++ b/apps/web/src/components/map/mapLocation.ts @@ -0,0 +1,96 @@ +import { coordinateId, createPlace, type Place } from "@openmapx/core"; +import type { StylePoiTarget } from "./mapStylePoiTarget"; + +/** + * A point the user picked on the map, by right-click or by long-press, and the + * basemap place under it if there is one. The desktop context menu and the + * dropped-pin card describe it the same way through these helpers. + */ +export interface MapLocation { + coordinates: [number, number]; + poi: StylePoiTarget | null; +} + +interface ReverseGeocoded { + address?: string | null; + city?: string | null; +} + +export interface MapLocationText { + /** Latitude, longitude to six decimals. */ + coordinates: string; + /** What the location is called: the place's name, else where it is. */ + title: string; + /** Its address, else its coordinates. */ + address: string; + /** The label a route to or from it carries. */ + routeLabel: string; +} + +export function formatMapCoordinates([lng, lat]: [number, number]): string { + return `${lat.toFixed(6)}, ${lng.toFixed(6)}`; +} + +export function describeMapLocation( + { coordinates, poi }: MapLocation, + reverse: ReverseGeocoded | null | undefined, +): MapLocationText { + const label = formatMapCoordinates(coordinates); + return { + coordinates: label, + title: poi?.name || reverse?.city || reverse?.address || label, + address: reverse?.address || label, + routeLabel: poi?.name || reverse?.address || label, + }; +} + +/** The location as a place: the basemap place under it, else the bare point. */ +export function mapLocationPlace( + location: MapLocation, + reverse: ReverseGeocoded | null | undefined, +): Place { + const text = describeMapLocation(location, reverse); + const { poi } = location; + if (poi) { + return createPlace({ + primaryScheme: "stylePoi", + ids: { stylePoi: poi.featureId }, + name: poi.name, + address: reverse?.address || poi.name, + coordinates: poi.coordinates, + category: poi.category, + rawCategory: poi.rawCategory, + }); + } + return createPlace({ + primaryScheme: "coordinate", + ids: { coordinate: coordinateId(location.coordinates) }, + name: text.title, + address: text.address, + coordinates: location.coordinates, + }); +} + +export interface MapLocationCopyRow { + label: string; + value: string; +} + +/** What can be copied about the location, without repeating a value. */ +export function mapLocationCopyRows( + location: MapLocation, + reverse: ReverseGeocoded | null | undefined, + labels: { coordinates: string; name: string; address: string }, +): MapLocationCopyRow[] { + const candidates = [ + { label: labels.coordinates, value: formatMapCoordinates(location.coordinates) }, + ...(location.poi?.name ? [{ label: labels.name, value: location.poi.name }] : []), + ...(reverse?.address ? [{ label: labels.address, value: reverse.address }] : []), + ]; + const seen = new Set(); + return candidates.filter(({ value }) => { + if (seen.has(value)) return false; + seen.add(value); + return true; + }); +} diff --git a/apps/web/src/components/panels/MapClickFloatingCard.test.tsx b/apps/web/src/components/panels/MapClickFloatingCard.test.tsx index 347173320..93dabc811 100644 --- a/apps/web/src/components/panels/MapClickFloatingCard.test.tsx +++ b/apps/web/src/components/panels/MapClickFloatingCard.test.tsx @@ -1,10 +1,27 @@ // @vitest-environment jsdom import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { render, screen, userEvent, waitFor } from "@/test"; +import { render, screen, userEvent, waitFor, within } from "@/test"; vi.mock("next-intl", async () => (await import("@/test/intl")).mockNextIntl()); -vi.mock("@/integration-api/map/MapContext", () => ({ useMap: () => ({ flyTo: vi.fn() }) })); +vi.mock("@/integration-api/map/MapContext", () => ({ + useMap: () => ({ + flyTo: vi.fn(), + mapRef: { current: { project: () => ({ x: 120, y: 140 }) } }, + }), +})); +const basemap = vi.hoisted(() => ({ + poi: null as null | { + featureId: string; + name: string; + coordinates: [number, number]; + category?: string; + }, +})); +vi.mock("@/components/map/mapStylePoiTarget", () => ({ + getStylePoiLayerIds: () => ["poi-label"], + findStylePoiAtPoint: () => basemap.poi, +})); vi.mock("@openmapx/core", async (importOriginal) => ({ ...(await importOriginal()), useReverseGeocoding: () => ({ @@ -22,14 +39,22 @@ vi.mock("@/lib/deepLink", async (importOriginal) => ({ shareUrl: share.shareUrl, })); -import { PANEL, useDirectionsStore, useMapClickStore, useSidebarStore } from "@openmapx/core"; +import { + PANEL, + useDirectionsStore, + useMapClickStore, + usePlaceStore, + useSidebarStore, +} from "@openmapx/core"; import { MapClickFloatingCard } from "./MapClickFloatingCard"; const PIN: [number, number] = [6.083, 50.776]; -function asPhone(phone: boolean): void { +function asDevice(device: "phone" | "touch tablet" | "desktop"): void { vi.stubGlobal("matchMedia", (query: string) => ({ - matches: phone && query.includes("max-width"), + matches: + (device === "phone" && query.includes("max-width")) || + (device !== "desktop" && query.includes("pointer: coarse")), media: query, addEventListener: () => {}, removeEventListener: () => {}, @@ -38,7 +63,15 @@ function asPhone(phone: boolean): void { })); } +function installClipboard() { + const writeText = vi.fn().mockResolvedValue(undefined); + Object.defineProperty(navigator, "clipboard", { configurable: true, value: { writeText } }); + return writeText; +} + beforeEach(() => { + basemap.poi = null; + usePlaceStore.getState().setSelectedPlace(null); useMapClickStore.getState().setClickedLngLat(PIN); useDirectionsStore.getState().close(); useSidebarStore.setState({ activeSidebarId: null, activeDetailId: null, collapsed: false }); @@ -51,7 +84,7 @@ afterEach(() => { }); describe("MapClickFloatingCard on a phone", () => { - beforeEach(() => asPhone(true)); + beforeEach(() => asDevice("phone")); it("offers the context menu's actions as a row", () => { render(); @@ -92,8 +125,69 @@ describe("MapClickFloatingCard on a phone", () => { }); }); +describe("MapClickFloatingCard on a touch screen wider than a phone", () => { + beforeEach(() => asDevice("touch tablet")); + + it("offers the same row of actions", () => { + render(); + for (const name of ["mapContextMenu.fromHere", "common.share", "parking.saveParking"]) { + expect(screen.getByRole("button", { name })).toBeDefined(); + } + }); +}); + +describe("MapClickFloatingCard's Copy", () => { + beforeEach(() => asDevice("phone")); + + it("offers the coordinates and the address, as the right-click menu does", async () => { + const writeText = installClipboard(); + render(); + await userEvent.click(screen.getByRole("button", { name: "common.copy" })); + const menu = await screen.findByRole("menu"); + expect(within(menu).getByText("50.776000, 6.083000")).toBeDefined(); + await userEvent.click(within(menu).getByText("Marktplatz 1")); + expect(writeText).toHaveBeenCalledWith("Marktplatz 1"); + expect(await screen.findByText("mapContextMenu.copied")).toBeDefined(); + }); +}); + +describe("MapClickFloatingCard on a basemap place", () => { + beforeEach(() => { + asDevice("phone"); + basemap.poi = { + featureId: "42", + name: "Café Kaiser", + coordinates: [6.0831, 50.7761], + category: "cafe", + }; + }); + + it("names the place and opens it, as right-click does", async () => { + render(); + expect(screen.getByText("Café Kaiser")).toBeDefined(); + await userEvent.click(screen.getByRole("button", { name: "common.details" })); + const selected = usePlaceStore.getState().selectedPlace; + expect(selected?.name).toBe("Café Kaiser"); + expect(selected?.id).toBe("stylePoi:42"); + }); + + it("offers its name to copy", async () => { + installClipboard(); + render(); + await userEvent.click(screen.getByRole("button", { name: "common.copy" })); + const menu = await screen.findByRole("menu"); + expect(within(menu).getByText("Café Kaiser")).toBeDefined(); + }); + + it("routes to it under its name", async () => { + render(); + await userEvent.click(screen.getByRole("button", { name: "mapContextMenu.fromHere" })); + expect(useDirectionsStore.getState().waypoints[0].label).toBe("Café Kaiser"); + }); +}); + describe("MapClickFloatingCard on a desktop", () => { - beforeEach(() => asPhone(false)); + beforeEach(() => asDevice("desktop")); it("keeps the compact card without the action row", () => { render(); diff --git a/apps/web/src/components/panels/MapClickFloatingCard.tsx b/apps/web/src/components/panels/MapClickFloatingCard.tsx index a816acad7..3061e97e1 100644 --- a/apps/web/src/components/panels/MapClickFloatingCard.tsx +++ b/apps/web/src/components/panels/MapClickFloatingCard.tsx @@ -11,6 +11,8 @@ import TripOriginIcon from "@mui/icons-material/TripOrigin"; import Box from "@mui/material/Box"; import Button from "@mui/material/Button"; import IconButton from "@mui/material/IconButton"; +import ListItemText from "@mui/material/ListItemText"; +import MenuItem from "@mui/material/MenuItem"; import Paper from "@mui/material/Paper"; import Skeleton from "@mui/material/Skeleton"; import Snackbar from "@mui/material/Snackbar"; @@ -18,8 +20,6 @@ import { useTheme } from "@mui/material/styles"; import Typography from "@mui/material/Typography"; import useMediaQuery from "@mui/material/useMediaQuery"; import { - coordinateId, - createPlace, PANEL, type Place, useMapClickStore, @@ -29,9 +29,18 @@ import { useSidebarStore, } from "@openmapx/core"; import { useLocale, useTranslations } from "next-intl"; -import type { ReactNode } from "react"; +import type { MouseEvent, ReactNode } from "react"; import { useMemo, useState, useSyncExternalStore } from "react"; +import { + describeMapLocation, + type MapLocation, + mapLocationCopyRows, + mapLocationPlace, +} from "@/components/map/mapLocation"; +import { findStylePoiAtPoint, getStylePoiLayerIds } from "@/components/map/mapStylePoiTarget"; import { useSaveParking } from "@/components/panels/parking/useSaveParking"; +import Menu from "@/components/ui/Menu"; +import { INTERACTIVE_LAYER_IDS } from "@/integration-api/map/interactiveLayers"; import { useMap } from "@/integration-api/map/MapContext"; import { BRAND } from "@/integration-api/runtime/theme"; import { routeFromOrTo, shareLocationLink } from "@/lib/locationActions"; @@ -65,34 +74,53 @@ export function MapClickFloatingCard() { const locale = useLocale(); const theme = useTheme(); const isMobile = useMediaQuery(theme.breakpoints.down("sm")); + // Touch-first screens wider than a phone (tablets, a phone held sideways) + // get the same actions: touch has no right-click menu to fall back on. + const touchFirst = useMediaQuery("(pointer: coarse)"); const { clickedLngLat, setClickedLngLat } = useMapClickStore(); const { setQuery, setIsFocused } = useSearchStore(); - const { flyTo } = useMap(); + const { flyTo, mapRef } = useMap(); const { saveAt } = useSaveParking(); const [feedback, setFeedback] = useState(null); + const [copyAnchor, setCopyAnchor] = useState(null); const phoneCardBottom = usePhoneCardBottom(); const { data: reverseGeo, isLoading } = useReverseGeocoding(clickedLngLat, locale); useBackDismiss(clickedLngLat !== null, () => setClickedLngLat(null)); - const coordLabel = clickedLngLat - ? `${clickedLngLat[1].toFixed(6)}, ${clickedLngLat[0].toFixed(6)}` - : ""; - const placeName = reverseGeo?.city || reverseGeo?.address || coordLabel; - const address = reverseGeo?.address ?? coordLabel; + // The basemap place under the pin, as the right-click menu finds it, read + // once when the pin lands: the pin stands where the finger or pointer was. + const location = useMemo(() => { + if (!clickedLngLat) return null; + const map = mapRef?.current; + const poi = map + ? findStylePoiAtPoint( + map, + map.project(clickedLngLat), + getStylePoiLayerIds(map), + INTERACTIVE_LAYER_IDS, + ) + : null; + return { coordinates: clickedLngLat, poi }; + }, [clickedLngLat, mapRef]); + const text = location ? describeMapLocation(location, reverseGeo) : null; + const coordLabel = text?.coordinates ?? ""; + const placeName = text?.title ?? ""; const place = useMemo( + () => (location ? mapLocationPlace(location, reverseGeo) : null), + [location, reverseGeo], + ); + const copyRows = useMemo( () => - clickedLngLat - ? createPlace({ - primaryScheme: "coordinate", - ids: { coordinate: coordinateId(clickedLngLat) }, - name: placeName, - address, - coordinates: clickedLngLat, + location + ? mapLocationCopyRows(location, reverseGeo, { + coordinates: tm("coordinates"), + name: tm("name"), + address: tm("address"), }) - : null, - [address, clickedLngLat, placeName], + : [], + [location, reverseGeo, tm], ); const feedbackToast = ( @@ -104,11 +132,11 @@ export function MapClickFloatingCard() { /> ); - if (!clickedLngLat || !place) return feedbackToast; + if (!clickedLngLat || !place || !location || !text) return feedbackToast; const handleCoordClick = () => { flyTo(clickedLngLat, 15); - setQuery(coordLabel); + setQuery(location.poi ? text.title : coordLabel); setIsFocused(false); usePlaceStore.getState().setSelectedPlace(place); useSidebarStore.getState().openSidebar(PANEL.PLACE); @@ -116,7 +144,7 @@ export function MapClickFloatingCard() { }; const handleRoute = (endpoint: "origin" | "destination") => { - routeFromOrTo(endpoint, clickedLngLat, address); + routeFromOrTo(endpoint, clickedLngLat, text.routeLabel); setClickedLngLat(null); }; @@ -126,16 +154,24 @@ export function MapClickFloatingCard() { else if (result === "copied") setFeedback(tm("linkCopied")); }; - const handleCopy = async () => { + const copyValue = async (value: string) => { + setCopyAnchor(null); try { if (!navigator.clipboard?.writeText) throw new Error("Clipboard unavailable"); - await navigator.clipboard.writeText(coordLabel); + await navigator.clipboard.writeText(value); setFeedback(tm("copied")); } catch { setFeedback(tm("copyFailed")); } }; + // One thing to copy goes straight to the clipboard; more offer a choice, as + // the right-click menu's Copy location does. + const handleCopy = (button: HTMLElement) => { + if (copyRows.length > 1) setCopyAnchor(button); + else void copyValue(copyRows[0]?.value ?? coordLabel); + }; + const handleSaveParking = async () => { const outcome = await saveAt(clickedLngLat, { address: reverseGeo?.address ?? null }); setFeedback(outcome === "saved" ? tParking("savedToast") : tParking("locationUnavailable")); @@ -157,8 +193,8 @@ export function MapClickFloatingCard() { ); - if (isMobile) { - // A dropped pin is where a phone user lands after a long-press, so the + if (isMobile || touchFirst) { + // A dropped pin is where a touch user lands after a long-press, so the // card carries what the desktop context menu offers, as a row of actions // within thumb reach instead of a menu under the finger. return ( @@ -169,12 +205,17 @@ export function MapClickFloatingCard() { aria-label={tm("ariaLabel", { location: placeName })} sx={{ position: "absolute", - left: CARD_GAP, - right: CARD_GAP, - bottom: phoneCardBottom, zIndex: 11, borderRadius: 3, overflow: "hidden", + ...(isMobile + ? { left: CARD_GAP, right: CARD_GAP, bottom: phoneCardBottom } + : { + bottom: 24, + left: "50%", + transform: "translateX(-50%)", + width: "min(480px, calc(100% - 32px))", + }), }} > @@ -217,9 +258,15 @@ export function MapClickFloatingCard() { px: 2, pt: 1.5, pb: 1.5, - overflowX: "auto", - scrollbarWidth: "none", - "&::-webkit-scrollbar": { display: "none" }, + // A phone scrolls the row under the thumb; wider screens have + // the room to show every action at once. + ...(isMobile + ? { + overflowX: "auto", + scrollbarWidth: "none", + "&::-webkit-scrollbar": { display: "none" }, + } + : { flexWrap: "wrap" }), }} > } label={tc("copy")} - onClick={() => void handleCopy()} + onClick={(event) => handleCopy(event.currentTarget)} + menuOpen={copyRows.length > 1 ? copyAnchor !== null : undefined} /> } label={tc("details")} onClick={handleCoordClick} /> + setCopyAnchor(null)} + anchorOrigin={{ vertical: "top", horizontal: "left" }} + transformOrigin={{ vertical: "bottom", horizontal: "left" }} + slotProps={{ paper: { sx: { minWidth: 240, maxWidth: "calc(100vw - 16px)" } } }} + > + {copyRows.map((row) => ( + void copyValue(row.value)} + sx={{ minHeight: "48px !important" }} + > + + + ))} + {feedbackToast} ); @@ -334,17 +400,24 @@ function Action({ label, onClick, primary, + menuOpen, }: { icon: ReactNode; label: string; - onClick: () => void; + onClick: (event: MouseEvent) => void; primary?: boolean; + /** Set on a button that opens a menu: whether that menu is open. */ + menuOpen?: boolean; }) { return (