From d46b6fb6468b3e916d99835ac072cbedd4500ad2 Mon Sep 17 00:00:00 2001 From: Maurice Date: Sat, 17 Oct 2026 02:07:22 +0000 Subject: [PATCH] refactor(tours): the map stage's track pick and the planner's focus points in hooks of their own The tour map stage and the trip planner page hook had grown past the complexity and length the lint ratchet holds them to. Marking a spot on a recorded paddle track moves into useTrackLinePick, and what the planner map frames into usePlannerFocusPoints, each with its own tests. The planner's viewport snapshot no longer reads a ref while rendering, so the count of that warning goes down by one. --- client/scripts/eslint-baseline.json | 2 +- .../pages/tripPlanner/TourPlannerMapStage.tsx | 26 ++---- .../tripPlanner/usePlannerFocusPoints.test.ts | 83 +++++++++++++++++++ .../tripPlanner/usePlannerFocusPoints.ts | 63 ++++++++++++++ .../tripPlanner/useTrackLinePick.test.ts | 67 +++++++++++++++ .../src/pages/tripPlanner/useTrackLinePick.ts | 31 +++++++ .../pages/tripPlanner/useTripPlannerPage.ts | 40 +-------- 7 files changed, 255 insertions(+), 57 deletions(-) create mode 100644 client/src/pages/tripPlanner/usePlannerFocusPoints.test.ts create mode 100644 client/src/pages/tripPlanner/usePlannerFocusPoints.ts create mode 100644 client/src/pages/tripPlanner/useTrackLinePick.test.ts create mode 100644 client/src/pages/tripPlanner/useTrackLinePick.ts diff --git a/client/scripts/eslint-baseline.json b/client/scripts/eslint-baseline.json index 92a3c91aa..af2c39824 100644 --- a/client/scripts/eslint-baseline.json +++ b/client/scripts/eslint-baseline.json @@ -8,7 +8,7 @@ "trek/no-swallowed-catch": 133, "max-params": 35, "trek/store-without-selector": 69, - "react-hooks/refs": 560, + "react-hooks/refs": 559, "react-hooks/set-state-in-effect": 197, "@typescript-eslint/no-floating-promises": 167, "react-hooks/exhaustive-deps": 121, diff --git a/client/src/pages/tripPlanner/TourPlannerMapStage.tsx b/client/src/pages/tripPlanner/TourPlannerMapStage.tsx index 6cfe3e926..cb008aac8 100644 --- a/client/src/pages/tripPlanner/TourPlannerMapStage.tsx +++ b/client/src/pages/tripPlanner/TourPlannerMapStage.tsx @@ -16,11 +16,10 @@ import { isRtlLanguage, useTranslation } from '../../i18n'; import { usePaddleObstacleStore } from '../../store/paddleObstacleStore'; import { useSettingsStore } from '../../store/settingsStore'; import { publishTourFerrataCrossings } from '../../store/tourFerrataCrossingStore'; -import { pickOnTrack, refuseTrackPick, useTrackMarkToolStore } from '../../store/trackMarkToolStore'; -import { projectOntoTrack, TRACK_PICK_TOLERANCE_M } from '../../utils/paddleTrackMarks'; import { useFerrataAdopt } from './useFerrataAdopt'; import { useTourMapPoints } from './useTourMapPoints'; import { useTourStageLayers } from './useTourStageLayers'; +import { useTrackLinePick } from './useTrackLinePick'; import type { useTripPlanner } from './useTripPlanner'; import type { useTripPlannerPage } from './useTripPlannerPage'; @@ -149,21 +148,8 @@ export function TourPlannerMapStage({ guard: closingPress.guard, previewPoints: tourMap.previewPoints, }); - const onAdd = waypoints.onAdd; // PAD-03 on a recorded paddle track: with a marking tool on, a click on the line marks the spot on it. - const trackPick = useTrackMarkToolStore((state) => state.pick); - const trackLine = tourPlanner.mode.type === 'view-gpx' ? analysis?.routeCoordinates : null; - const pickOnLine = canPlaceEdit && trackPick !== null && !!trackLine?.length; - const onTrackClick = closingPress.guard( - pickOnLine - ? (lat: number, lng: number) => { - const projected = projectOntoTrack(trackLine, { lat, lng }); - if (!projected || projected.offM > TRACK_PICK_TOLERANCE_M) refuseTrackPick(); - else pickOnTrack(projected.atM); - } - : undefined - ); - const onAddOrMark = onAdd ?? onTrackClick; + const { pickOnLine, onAddOrMark } = useTrackLinePick(tourPlanner, canPlaceEdit, closingPress.guard, waypoints.onAdd); // What each panel covers of the map, a collapsed one its rail. const leftInset = leftHidden ? 56 : leftPanelPx; const rightInset = rightHidden ? 56 : rightPanelPx; @@ -249,10 +235,10 @@ export function TourPlannerMapStage({ onOutsidePress={closingPress.arm} insetEnd={(isRtlLanguage(language) ? leftInset : rightInset) + MAP_LAYER_SWITCHER_INSET} {...ferrataCardPlacement(terrain3d, bottomRowTop, terrainLegend)} - extra={ - paddleChecklist - ? (detail) => - : undefined + extra={(detail) => + paddleChecklist ? ( + + ) : null } /> {/* "Point set", with the way back, above the bottom row and between the panels. */} diff --git a/client/src/pages/tripPlanner/usePlannerFocusPoints.test.ts b/client/src/pages/tripPlanner/usePlannerFocusPoints.test.ts new file mode 100644 index 000000000..54783238f --- /dev/null +++ b/client/src/pages/tripPlanner/usePlannerFocusPoints.test.ts @@ -0,0 +1,83 @@ +// FE-TP-FOCUSPOINTS-001 to FE-TP-FOCUSPOINTS-004: what the tour planner's map frames: a browsed wanderer trail, +// the recorded track or the open tour's control points, and with nothing open the area the plan map last showed, +// else the trip's places. +import { act, renderHook } from '@testing-library/react'; +import { describe, expect, it } from 'vitest'; + +import type { TourPlannerController } from '../../components/Tours/planner/useTourPlanner'; +import type { Place } from '../../types'; +import { usePlannerFocusPoints } from './usePlannerFocusPoints'; + +const PLACES = [ + { id: 1, lat: 48, lng: 11 }, + { id: 2, lat: Number.NaN, lng: 11 }, +] as unknown as Place[]; + +function planner(overrides: Partial> = {}): TourPlannerController { + return { + mode: { type: 'neutral' }, + readOnlyGpxAnalysis: null, + waypoints: [], + newDraftGeneration: 0, + ...overrides, + } as unknown as TourPlannerController; +} + +const closed = { open: false, line: null }; + +describe('usePlannerFocusPoints', () => { + it('FE-TP-FOCUSPOINTS-001: with nothing open the trip places with a position are framed', () => { + const { result } = renderHook(() => usePlannerFocusPoints('tour-planner', PLACES, planner(), closed)); + expect(result.current.focusPoints).toEqual([[48, 11]]); + }); + + it('FE-TP-FOCUSPOINTS-002: the area the plan map last showed wins once the planner opens again', () => { + const { result, rerender } = renderHook(({ tab }) => usePlannerFocusPoints(tab, PLACES, planner(), closed), { + initialProps: { tab: 'plan' }, + }); + act(() => { + result.current.lastPlanMapViewportRef.current = { south: 49, west: 12, north: 49.5, east: 12.5 }; + }); + rerender({ tab: 'tour-planner' }); + expect(result.current.focusPoints).toEqual([ + [49, 12], + [49.5, 12.5], + ]); + }); + + it('FE-TP-FOCUSPOINTS-003: a recorded track, a draft and a saved tour frame their own line or points', () => { + const line: [number, number][] = [ + [49.2, 12], + [49.2, 12.01], + ]; + const waypoints = [ + { id: 'a', lat: 47, lng: 11 }, + { id: 'b', lat: 47.1, lng: 11.1 }, + ]; + const gpx = planner({ mode: { type: 'view-gpx' }, readOnlyGpxAnalysis: { routeCoordinates: line } }); + expect( + renderHook(() => usePlannerFocusPoints('tour-planner', PLACES, gpx, closed)).result.current.focusPoints + ).toBe(line); + for (const type of ['new-draft', 'edit-saved']) { + const open = planner({ mode: { type }, waypoints }); + expect( + renderHook(() => usePlannerFocusPoints('tour-planner', PLACES, open, closed)).result.current.focusPoints + ).toEqual([ + [47, 11], + [47.1, 11.1], + ]); + } + }); + + it('FE-TP-FOCUSPOINTS-004: a browsed wanderer trail frames its line, the fallback while it has none', () => { + const trail: [number, number][] = [[46, 10]]; + expect( + renderHook(() => usePlannerFocusPoints('tour-planner', PLACES, planner(), { open: true, line: trail })).result + .current.focusPoints + ).toBe(trail); + expect( + renderHook(() => usePlannerFocusPoints('tour-planner', PLACES, planner(), { open: true, line: null })).result + .current.focusPoints + ).toEqual([[48, 11]]); + }); +}); diff --git a/client/src/pages/tripPlanner/usePlannerFocusPoints.ts b/client/src/pages/tripPlanner/usePlannerFocusPoints.ts new file mode 100644 index 000000000..b882772af --- /dev/null +++ b/client/src/pages/tripPlanner/usePlannerFocusPoints.ts @@ -0,0 +1,63 @@ +import { useEffect, useMemo, useRef, useState } from 'react'; +import type { Bbox } from '../../components/Map/usePoiExplore'; +import type { TourPlannerController } from '../../components/Tours/planner/useTourPlanner'; +import type { WandererBrowser } from '../../components/Tours/wanderer/useWandererBrowser'; +import type { Place } from '../../types'; + +/** + * What the TOUR-PLANNER map frames: a browsed wanderer trail, the recorded + * track or the control points of the open tour, and with nothing open the + * area the plan map last showed (or the trip's places). The plan map reports + * its viewport into the returned ref. + */ +export function usePlannerFocusPoints( + activeTab: string, + places: readonly Place[], + tourPlanner: TourPlannerController, + wandererBrowser: Pick +) { + const lastPlanMapViewportRef = useRef(null); + // Nothing reported yet when the planner first opens; the effect takes the area the plan map last showed. + const [plannerViewportSnapshot, setPlannerViewportSnapshot] = useState(null); + useEffect(() => { + if (activeTab === 'tour-planner') setPlannerViewportSnapshot(lastPlanMapViewportRef.current); + }, [activeTab, tourPlanner.newDraftGeneration]); + + const plannerFallbackFocusPoints = useMemo<[number, number][]>(() => { + const bounds = plannerViewportSnapshot; + if (bounds) { + const latitudeSpan = bounds.north - bounds.south; + const longitudeSpan = bounds.east - bounds.west; + if (latitudeSpan > 0 && latitudeSpan < 160 && longitudeSpan > 0 && longitudeSpan < 340) { + return [ + [bounds.south, bounds.west], + [bounds.north, bounds.east], + ]; + } + } + return places + .filter((place) => Number.isFinite(place.lat) && Number.isFinite(place.lng)) + .map((place) => [place.lat, place.lng] as [number, number]); + }, [places, plannerViewportSnapshot]); + + const focusPoints = useMemo<[number, number][]>(() => { + if (wandererBrowser.open) return wandererBrowser.line ?? plannerFallbackFocusPoints; + if (tourPlanner.mode.type === 'view-gpx') return tourPlanner.readOnlyGpxAnalysis?.routeCoordinates ?? []; + if (tourPlanner.mode.type === 'new-draft') { + return tourPlanner.waypoints.map((point) => [point.lat, point.lng]); + } + if (tourPlanner.mode.type === 'edit-saved' && tourPlanner.waypoints.length) { + return tourPlanner.waypoints.map((point) => [point.lat, point.lng]); + } + return plannerFallbackFocusPoints; + }, [ + wandererBrowser.open, + wandererBrowser.line, + tourPlanner.mode.type, + tourPlanner.readOnlyGpxAnalysis?.routeCoordinates, + tourPlanner.waypoints, + plannerFallbackFocusPoints, + ]); + + return { focusPoints, lastPlanMapViewportRef }; +} diff --git a/client/src/pages/tripPlanner/useTrackLinePick.test.ts b/client/src/pages/tripPlanner/useTrackLinePick.test.ts new file mode 100644 index 000000000..e4ff78429 --- /dev/null +++ b/client/src/pages/tripPlanner/useTrackLinePick.test.ts @@ -0,0 +1,67 @@ +// FE-TP-TRACKPICK-001 to FE-TP-TRACKPICK-004: a click on a recorded paddle track while a marking tool is on marks +// the spot on the line (PAD-03), one too far from it is refused, and otherwise the planner's own click stays. +import { renderHook } from '@testing-library/react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import type { TourPlannerController } from '../../components/Tours/planner/useTourPlanner'; +import { loadTrackMarks, startTrackPick, useTrackMarkToolStore } from '../../store/trackMarkToolStore'; +import { useTrackLinePick } from './useTrackLinePick'; + +const LINE: [number, number][] = [ + [49.2, 12], + [49.2, 12.01], +]; + +function track(mode: 'view-gpx' | 'edit-saved' = 'view-gpx') { + return { + mode: + mode === 'view-gpx' + ? { type: 'view-gpx', placeId: 40, tour: { place_id: 40 } } + : { type: 'edit-saved', placeId: 40 }, + readOnlyGpxAnalysis: { routeCoordinates: LINE }, + } as unknown as Pick; +} + +const passThrough = (handler: ((lat: number, lng: number) => void) | undefined) => handler; + +beforeEach(() => { + loadTrackMarks(40, null, { paddle: null, portage: null }); +}); + +describe('useTrackLinePick', () => { + it('FE-TP-TRACKPICK-001: without a marking tool the planner keeps its own click', () => { + const onAdd = vi.fn(); + const { result } = renderHook(() => useTrackLinePick(track(), true, passThrough, onAdd)); + expect(result.current.pickOnLine).toBe(false); + expect(result.current.onAddOrMark).toBe(onAdd); + }); + + it('FE-TP-TRACKPICK-002: with the lock tool on, a click on the line places the lock there', () => { + startTrackPick('lock'); + const { result } = renderHook(() => useTrackLinePick(track(), true, passThrough, undefined)); + expect(result.current.pickOnLine).toBe(true); + result.current.onAddOrMark?.(49.2, 12.005); + const { marks, dirty } = useTrackMarkToolStore.getState(); + expect(dirty).toBe(true); + expect(marks.locks).toHaveLength(1); + expect(marks.locks[0]?.at_m).toBeGreaterThan(300); + }); + + it('FE-TP-TRACKPICK-003: a click far from the line is refused', () => { + startTrackPick('lock'); + const { result } = renderHook(() => useTrackLinePick(track(), true, passThrough, undefined)); + result.current.onAddOrMark?.(49.25, 12.005); + expect(useTrackMarkToolStore.getState()).toMatchObject({ refused: 'off_line', dirty: false }); + }); + + it('FE-TP-TRACKPICK-004: a viewer, or a tour that is no recorded track, marks nothing', () => { + startTrackPick('lock'); + expect(renderHook(() => useTrackLinePick(track(), false, passThrough, undefined)).result.current).toEqual({ + pickOnLine: false, + onAddOrMark: undefined, + }); + expect( + renderHook(() => useTrackLinePick(track('edit-saved'), true, passThrough, undefined)).result.current.pickOnLine + ).toBe(false); + }); +}); diff --git a/client/src/pages/tripPlanner/useTrackLinePick.ts b/client/src/pages/tripPlanner/useTrackLinePick.ts new file mode 100644 index 000000000..71e465673 --- /dev/null +++ b/client/src/pages/tripPlanner/useTrackLinePick.ts @@ -0,0 +1,31 @@ +import type { TourPlannerController } from '../../components/Tours/planner/useTourPlanner'; +import { pickOnTrack, refuseTrackPick, useTrackMarkToolStore } from '../../store/trackMarkToolStore'; +import { projectOntoTrack, TRACK_PICK_TOLERANCE_M } from '../../utils/paddleTrackMarks'; + +type MapClick = (lat: number, lng: number) => void; + +/** + * PAD-03 on a recorded paddle track: with a marking tool on, a click on the + * line marks the spot on it, one too far from the line is refused. Otherwise + * the planner's own click (adding a waypoint) stays. + */ +export function useTrackLinePick( + tourPlanner: Pick, + canPlaceEdit: boolean, + guard: (handler: MapClick | undefined) => MapClick | undefined, + onAdd: MapClick | undefined +): { pickOnLine: boolean; onAddOrMark: MapClick | undefined } { + const trackPick = useTrackMarkToolStore((state) => state.pick); + const trackLine = tourPlanner.mode.type === 'view-gpx' ? tourPlanner.readOnlyGpxAnalysis?.routeCoordinates : null; + const pickOnLine = canPlaceEdit && trackPick !== null && !!trackLine?.length; + const onTrackClick = guard( + pickOnLine && trackLine + ? (lat, lng) => { + const projected = projectOntoTrack(trackLine, { lat, lng }); + if (!projected || projected.offM > TRACK_PICK_TOLERANCE_M) refuseTrackPick(); + else pickOnTrack(projected.atM); + } + : undefined + ); + return { pickOnLine, onAddOrMark: onAdd ?? onTrackClick }; +} diff --git a/client/src/pages/tripPlanner/useTripPlannerPage.ts b/client/src/pages/tripPlanner/useTripPlannerPage.ts index 7602ec158..62c1b7502 100644 --- a/client/src/pages/tripPlanner/useTripPlannerPage.ts +++ b/client/src/pages/tripPlanner/useTripPlannerPage.ts @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useMemo, useRef, useState } from 'react' +import { useCallback, useEffect, useMemo, useRef } from 'react' import { usePoiExplore, type Bbox } from '../../components/Map/usePoiExplore' import { placeActions } from '../../components/Planner/usePlaceActions' import { tourRouteStyle } from '../../components/Tours/planner/tourRouteStyle' @@ -9,6 +9,7 @@ import { tourPlannedTimes as projectTourPlannedTimes } from '../../components/To import { usePlanMapViewport } from '../../hooks/usePlanMapViewport' import { usePaddleObstacleStore } from '../../store/paddleObstacleStore' import { useMapLocationSearch } from './useMapLocationSearch' +import { usePlannerFocusPoints } from './usePlannerFocusPoints' import { useTripPlanner } from './useTripPlanner' import { useTourWorkspace } from './useTourWorkspace' @@ -31,40 +32,7 @@ export function useTripPlannerPage() { useTourWorkspace(planner, canPlaceEdit, canDayEdit) const tourDetailOpenerRef = useRef<{ placeId: number; element: HTMLElement } | null>(null) - const lastPlanMapViewportRef = useRef(null) - const [plannerViewportSnapshot, setPlannerViewportSnapshot] = useState(lastPlanMapViewportRef.current) - useEffect(() => { - if (activeTab === 'tour-planner') setPlannerViewportSnapshot(lastPlanMapViewportRef.current) - }, [activeTab, tourPlanner.newDraftGeneration]) - - const plannerFallbackFocusPoints = useMemo<[number, number][]>(() => { - const bounds = plannerViewportSnapshot - if (bounds) { - const latitudeSpan = bounds.north - bounds.south - const longitudeSpan = bounds.east - bounds.west - if (latitudeSpan > 0 && latitudeSpan < 160 && longitudeSpan > 0 && longitudeSpan < 340) { - return [[bounds.south, bounds.west], [bounds.north, bounds.east]] - } - } - return places - .filter(place => Number.isFinite(place.lat) && Number.isFinite(place.lng)) - .map(place => [place.lat, place.lng] as [number, number]) - }, [places, plannerViewportSnapshot]) - - const focusPoints = useMemo<[number, number][]>(() => { - if (wandererBrowser.open) return wandererBrowser.line ?? plannerFallbackFocusPoints - if (tourPlanner.mode.type === 'view-gpx') return tourPlanner.readOnlyGpxAnalysis?.routeCoordinates ?? [] - if (tourPlanner.mode.type === 'new-draft') { - return tourPlanner.waypoints.map(point => [point.lat, point.lng]) - } - if (tourPlanner.mode.type === 'edit-saved' && tourPlanner.waypoints.length) { - return tourPlanner.waypoints.map(point => [point.lat, point.lng]) - } - return plannerFallbackFocusPoints - }, [ - wandererBrowser.open, - wandererBrowser.line, - tourPlanner.mode.type, tourPlanner.readOnlyGpxAnalysis?.routeCoordinates, tourPlanner.waypoints, plannerFallbackFocusPoints]) + const { focusPoints, lastPlanMapViewportRef } = usePlannerFocusPoints(activeTab, places, tourPlanner, wandererBrowser) // PAD-06: an obstacle or area the paddle checklist asks for moves the camera as a picked search result does. const paddleFocus = usePaddleObstacleStore(state => state.focus) @@ -122,7 +90,7 @@ export function useTripPlannerPage() { lastPlanMapViewportRef.current = bounds poiViewportChangeRef.current(bounds) reportViewport(bounds) - }, [reportViewport]) + }, [lastPlanMapViewportRef, reportViewport]) // Each tour's planned times keyed by its place, for the day plan's tour rows. const plannedTourDurations = new Map(tours.map(tour => [tour.place_id, { ...projectTourPlannedTimes(tour), tour }] as const))