From 4f68029b2c2bb7d750c9b4d68f83ba30bd6aaeb8 Mon Sep 17 00:00:00 2001 From: Maurice Date: Tue, 20 Oct 2026 13:26:18 +0200 Subject: [PATCH] fix(tours): keep one card in the map corner and offer only ways that change a section While the other ways of a hike's section were compared, a click on a via ferrata or a place on the map opened its card in the same corner, stacked over the comparison. The corner now holds one card: whichever opened last stays and the other closes, before paint, so the two never show together. The router's first answer between the two ends is its own way under the hike's rules, which the tour already routes the section by. Offered as a choice, taking it only planned the leg again, usually to the same line, and the card closed without "Way taken" or an undo step. It is no longer offered, so every way taken is held by shaping points, changes the route and can be taken back in one step. --- .../Tours/planner/LegVariantsCard.test.tsx | 1 - .../Tours/planner/legVariantPaint.test.ts | 1 - .../Tours/planner/legVariantRouting.test.ts | 5 +- .../Tours/planner/legVariants.test.ts | 17 ++++--- .../components/Tours/planner/legVariants.ts | 17 ++++--- .../Tours/planner/useLegVariants.test.tsx | 30 +++++++----- .../Tours/planner/useLegVariants.ts | 25 +++++----- .../Tours/planner/useTourPlanner.ts | 2 +- .../tripPlanner/TourPlannerMapStage.test.tsx | 49 +++++++++++++++++++ .../pages/tripPlanner/TourPlannerMapStage.tsx | 10 +++- .../src/pages/tripPlanner/useOneCornerCard.ts | 45 +++++++++++++++++ 11 files changed, 154 insertions(+), 48 deletions(-) create mode 100644 client/src/pages/tripPlanner/useOneCornerCard.ts diff --git a/client/src/components/Tours/planner/LegVariantsCard.test.tsx b/client/src/components/Tours/planner/LegVariantsCard.test.tsx index c056884de..80eb7ab59 100644 --- a/client/src/components/Tours/planner/LegVariantsCard.test.tsx +++ b/client/src/components/Tours/planner/LegVariantsCard.test.tsx @@ -20,7 +20,6 @@ const POINTS = [ const way = (key: LegVariant['key'], overrides: Partial = {}): LegVariant => ({ key, current: key === 'current', - direct: false, coordinates: [ [47, 11], [47, 11.02], diff --git a/client/src/components/Tours/planner/legVariantPaint.test.ts b/client/src/components/Tours/planner/legVariantPaint.test.ts index 7c3d25bad..bd3a0e2f3 100644 --- a/client/src/components/Tours/planner/legVariantPaint.test.ts +++ b/client/src/components/Tours/planner/legVariantPaint.test.ts @@ -8,7 +8,6 @@ import type { LegVariant } from './legVariants'; const variant = (key: LegVariant['key'], lat: number): LegVariant => ({ key, current: key === 'current', - direct: false, coordinates: [ [lat, 11], [lat, 11.02], diff --git a/client/src/components/Tours/planner/legVariantRouting.test.ts b/client/src/components/Tours/planner/legVariantRouting.test.ts index a9cd22e8a..b81202206 100644 --- a/client/src/components/Tours/planner/legVariantRouting.test.ts +++ b/client/src/components/Tours/planner/legVariantRouting.test.ts @@ -25,7 +25,6 @@ const LEG: ComparedLeg = { const CURRENT: LegVariant = { key: 'current', current: true, - direct: false, coordinates: [ [47, 11], [47.001, 11.01], @@ -102,13 +101,13 @@ describe('askOtherWays', () => { const report = vi.fn(); valhallaAlternates.mockImplementationOnce(async () => { controller.abort(); - return [leg(NORTH)]; + return [leg(CURRENT.coordinates), leg(NORTH)]; }); await askOtherWays(LEG, CURRENT, controller.signal, report); expect(report).not.toHaveBeenCalled(); const later = new AbortController(); - valhallaAlternates.mockResolvedValueOnce([leg(NORTH)]); + valhallaAlternates.mockResolvedValueOnce([leg(CURRENT.coordinates), leg(NORTH)]); enrichTourElevations.mockImplementationOnce(async () => { later.abort(); return null; diff --git a/client/src/components/Tours/planner/legVariants.test.ts b/client/src/components/Tours/planner/legVariants.test.ts index 0fb86d285..e8369d6fb 100644 --- a/client/src/components/Tours/planner/legVariants.test.ts +++ b/client/src/components/Tours/planner/legVariants.test.ts @@ -99,7 +99,6 @@ describe('the figures of a way', () => { expect(current).toMatchObject({ key: 'current', current: true, - direct: false, heights: 'known', gainM: 60, lossM: 20, @@ -145,19 +144,23 @@ describe('the ways offered', () => { offer(west), ]); expect(variants.map((variant) => variant.key)).toEqual(['current', 'a', 'b', 'c']); - expect(variants[1]).toMatchObject({ coordinates: north, current: false, direct: false, heights: 'pending' }); + expect(variants[1]).toMatchObject({ coordinates: north, current: false, heights: 'pending' }); expect(variants.slice(1).every((variant) => variant.gainM === null && variant.minutes === null)).toBe(true); }); - it("FE-TOUR-LEG-VARIANTS-007: the router's own way is marked direct, so it is held without a shaping point", () => { - const [, first, second] = offeredVariants(current, [offer(north), offer(south)]); - expect(first.direct).toBe(true); - expect(second.direct).toBe(false); + it("FE-TOUR-LEG-VARIANTS-007: the router's own way is the one the tour routes already, so it is never offered", () => { + // Drawn apart from the route, it is still no choice: taking it could not change the section. + const variants = offeredVariants(current, [offer(north), offer(south)]); + expect(variants.map((variant) => [variant.key, variant.coordinates])).toEqual([ + ['current', current.coordinates], + ['a', south], + ]); + expect(offeredVariants(current, [offer(north)])).toEqual([current]); expect(offeredVariants(current, [])).toEqual([current]); }); it('FE-TOUR-LEG-VARIANTS-008: heights fill a way in, and the difference to the way taken now is what is known of both', () => { - const [, pending] = offeredVariants(current, [offer(north)]); + const [, pending] = offeredVariants(current, [offer(current.coordinates), offer(north)]); const known = withVariantHeights(pending, [ [47.0, 11.0, 1000], [47.03, 11.01, 1300], diff --git a/client/src/components/Tours/planner/legVariants.ts b/client/src/components/Tours/planner/legVariants.ts index 302e09e12..c68e7d12c 100644 --- a/client/src/components/Tours/planner/legVariants.ts +++ b/client/src/components/Tours/planner/legVariants.ts @@ -28,8 +28,6 @@ export interface LegVariant { key: LegVariantKey; /** The way the section takes now, cut from the drawn route. */ current: boolean; - /** The router's own way between the two points: held without a shaping point. */ - direct: boolean; coordinates: [number, number][]; /** Measured along the line, as the tour's own distance is. */ distanceM: number; @@ -135,7 +133,6 @@ export function currentVariantOf( return { key: 'current', current: true, - direct: false, coordinates: line.map(([lat, lng]): [number, number] => [lat, lng]), ...figuresOf(withHeights ?? line), }; @@ -146,22 +143,26 @@ const roadOf = (coordinates: [number, number][]) => ({ coordinates, distance: 0, /** * The way the section takes now first, then each other way the router * offered that is not the same way again, lettered in the order the router - * gave them and at most three. Their heights are still to be asked for. + * gave them and at most three. The router's first answer is never one of + * them: it is its own way between the two points under the same rules, the + * one the tour already routes the section by, so choosing it could change + * nothing even where it is drawn a little apart from the route. Their heights + * are still to be asked for. */ export function offeredVariants(current: LegVariant, offers: readonly ValhallaLeg[]): LegVariant[] { const variants: LegVariant[] = [current]; - offers.forEach((offer, index) => { + for (const offer of offers.slice(1)) { const key = OTHER_WAY_KEYS[variants.length - 1]; - if (!key || variants.some((known) => sameRoad(roadOf(known.coordinates), roadOf(offer.coordinates)))) return; + if (!key) break; + if (variants.some((known) => sameRoad(roadOf(known.coordinates), roadOf(offer.coordinates)))) continue; variants.push({ key, current: false, - direct: index === 0, coordinates: offer.coordinates, ...figuresOf(offer.coordinates), heights: 'pending', }); - }); + } return variants; } diff --git a/client/src/components/Tours/planner/useLegVariants.test.tsx b/client/src/components/Tours/planner/useLegVariants.test.tsx index 3bc39774d..3aef4e1fe 100644 --- a/client/src/components/Tours/planner/useLegVariants.test.tsx +++ b/client/src/components/Tours/planner/useLegVariants.test.tsx @@ -30,7 +30,6 @@ const ROUTE: [number, number][] = [ const OTHER: LegVariant = { key: 'a', current: false, - direct: false, coordinates: [ [47.0, 11.0], [47.03, 11.01], @@ -56,7 +55,6 @@ function options(overrides: Partial = {}): LegVariantsOption maxHikingDifficulty: 2, hikingPreferences: null, insertShapingPoints: vi.fn(() => ['s1']), - recomputeLeg: vi.fn(), undo: vi.fn(), mayEdit: () => true, ...overrides, @@ -144,7 +142,9 @@ describe('useLegVariants', () => { await act(async () => view.result.current.apply()); const [{ offer, current, route }] = pinAlternative.mock.calls[0]; - expect(offer).toMatchObject({ coordinates: OTHER.coordinates, direct: false }); + // Never the router's own way: the first pin goes where the offer leaves the way taken now. + expect(offer).toMatchObject({ coordinates: OTHER.coordinates }); + expect(offer.direct).toBeFalsy(); expect(current.coordinates).toEqual(ROUTE.slice(0, 3)); await route([{ lat: 1, lng: 2 }], new AbortController().signal); expect(routeSection).toHaveBeenCalledWith( @@ -182,20 +182,26 @@ describe('useLegVariants', () => { expect(view.result.current.comparison?.notice).toBe('failed'); }); - it("FE-TOUR-USE-LEG-VARIANTS-007: the router's own way needs no pin: the leg is only planned again", async () => { - answers = [ - { phase: 'choose', variants: [{ ...OTHER, key: 'current', current: true }, { ...OTHER, direct: true }] }, - ]; - pinAlternative.mockResolvedValue({ held: true, pins: [], fellBack: false, last: {} }); - const recomputeLeg = vi.fn(); + it('FE-TOUR-USE-LEG-VARIANTS-007: a way taken always changes the route, says so and is taken back in one undo step', async () => { + answers = [{ phase: 'choose', variants: [{ ...OTHER, key: 'current', current: true }, OTHER] }]; + pinAlternative.mockResolvedValue({ held: true, pins: [{ lat: 47.03, lng: 11.01 }], fellBack: false, last: {} }); + const undo = vi.fn(); const insertShapingPoints = vi.fn(() => ['s1']); - const view = setup(options({ recomputeLeg, insertShapingPoints })); + const view = setup(options({ insertShapingPoints, undo })); await compared(view); act(() => view.result.current.select('a')); await act(async () => view.result.current.apply()); - expect(recomputeLeg).toHaveBeenCalledWith('a'); - expect(insertShapingPoints).not.toHaveBeenCalled(); + expect(insertShapingPoints).toHaveBeenCalledTimes(1); expect(view.result.current.comparison).toBeNull(); + expect(view.result.current.appliedHint.shown).toBe(false); + + // The write lands as the shaping point in the leg: "way taken" is up, and its undo is the editor's one step. + const S: TourPlannerWaypoint = { id: 's1', lat: 47.03, lng: 11.01, role: 'via', shaping: true }; + view.rerender(options({ insertShapingPoints, undo, waypoints: [A, S, B, C] })); + expect(view.result.current.appliedHint.shown).toBe(true); + act(() => view.result.current.appliedHint.undo()); + expect(undo).toHaveBeenCalledTimes(1); + expect(view.result.current.appliedHint.shown).toBe(false); }); it('FE-TOUR-USE-LEG-VARIANTS-008: a choice checked while the section changed is not written; a refused write says stale too', async () => { diff --git a/client/src/components/Tours/planner/useLegVariants.ts b/client/src/components/Tours/planner/useLegVariants.ts index 68814fb0e..1f9183ef8 100644 --- a/client/src/components/Tours/planner/useLegVariants.ts +++ b/client/src/components/Tours/planner/useLegVariants.ts @@ -52,28 +52,23 @@ export interface LegVariantsOptions { beforeId: string, spots: ReadonlyArray<{ lat: number; lng: number }> ) => string[] | null; - /** Plans the leg after a point afresh, for the router's own way, which needs no shaping point. */ - recomputeLeg: (id: string) => void; undo: () => void; /** Whether the route may change now: the permission held and no save under way. */ mayEdit: () => boolean; } /** - * A checked choice written into the tour: held by its pins as one step of the - * history, or, for the router's own way, the leg planned again. `stale` when - * the section is another one by now, so nothing lands where it was not asked. + * A checked choice written into the tour, held by its pins as one step of the + * history. Every way offered leaves the router's own (see `offeredVariants`), + * so a held one always has a pin. `stale` when the section is another one by + * now, so nothing lands where it was not asked. */ function writeChoice( leg: ComparedLeg, pins: ReadonlyArray<{ lat: number; lng: number }>, { options, costing }: { options: LegVariantsOptions; costing: CostingOptions } -): 'written' | 'replanned' | 'stale' { +): 'written' | 'stale' { if (comparedLegOf(options.waypoints, leg.fromId, options.tourType, costing)?.key !== leg.key) return 'stale'; - if (pins.length === 0) { - options.recomputeLeg(leg.fromId); - return 'replanned'; - } return options.insertShapingPoints(leg.fromId, leg.toId, pins) ? 'written' : 'stale'; } @@ -195,7 +190,6 @@ export function useLegVariants(options: LegVariantsOptions) { distance: chosen.distanceM, duration: 0, divergence: null, - direct: chosen.direct, }, current: { coordinates: request.current.coordinates, distance: request.current.distanceM, duration: 0 }, route: (pins, routeSignal) => routeSection(request.leg, pins, routeSignal), @@ -204,9 +198,12 @@ export function useLegVariants(options: LegVariantsOptions) { if (signal.aborted) return; const before = latest.current.options.waypoints; const outcome = pinned.held ? writeChoice(request.leg, pinned.pins, latest.current) : 'notHeld'; - if (outcome === 'written') noteApplied(before); - if (outcome === 'written' || outcome === 'replanned') close(); - else settle(outcome); + if (outcome !== 'written') { + settle(outcome); + return; + } + noteApplied(before); + close(); } catch { if (!signal.aborted) settle('failed'); } diff --git a/client/src/components/Tours/planner/useTourPlanner.ts b/client/src/components/Tours/planner/useTourPlanner.ts index a79d2d365..e71fca4a0 100644 --- a/client/src/components/Tours/planner/useTourPlanner.ts +++ b/client/src/components/Tours/planner/useTourPlanner.ts @@ -803,7 +803,7 @@ export function useTourPlanner({ tripId, active = true, onSaved, canEdit: editPe // RTE-02: other ways for one routed section of a hike, compared and taken on purpose. const legVariants = useLegVariants({ waypoints, route, enrichedGeometry, routeCurrent, tourType, maxHikingDifficulty, hikingPreferences, undo, mayEdit: mayEditRoute, - insertShapingPoints: shaping.insertShapingPoints, recomputeLeg: shaping.recomputeLeg, + insertShapingPoints: shaping.insertShapingPoints, }) const readOnlyGpxAnalysis = useMemo( diff --git a/client/src/pages/tripPlanner/TourPlannerMapStage.test.tsx b/client/src/pages/tripPlanner/TourPlannerMapStage.test.tsx index 39b6f97a8..ce8bbd8df 100644 --- a/client/src/pages/tripPlanner/TourPlannerMapStage.test.tsx +++ b/client/src/pages/tripPlanner/TourPlannerMapStage.test.tsx @@ -612,6 +612,55 @@ describe('TourPlannerMapStage', () => { expect(appliedHint.hold).toHaveBeenCalledWith(true); }); + describe('one card in the corner (RTE-02)', () => { + const waypoints = [ + { id: 'w1', lat: 46, lng: 7 }, + { id: 'w2', lat: 46.2, lng: 7.2 }, + ]; + const comparison = { + leg: { fromId: 'w1', toId: 'w2', from: { lat: 46, lng: 7 }, to: { lat: 46.2, lng: 7.2 }, costing: {}, key: 'k' }, + phase: 'loading', + variants: [], + selected: null, + proving: false, + notice: null, + }; + const comparingPlanner = (legVariants: ReturnType) => + planner('new-draft', { waypoints, route: null, canEdit: true, legVariants: { ...legVariants, comparison } }); + + /** Picks the via ferrata on the flat map, as a click on its line does. */ + function pickFerrata() { + act(() => stage.flatMap.props?.ferrataLayer?.onToggle()); + const group = stage.flatMap.props?.ferrataLayer?.groups[0]; + if (!group) throw new Error('no group'); + act(() => stage.flatMap.props?.ferrataLayer?.onSelect(group, FERRATA, { lat: 46.52, lng: 11.8 })); + } + + it('FE-PAGE-TOURSTAGE-021: a via ferrata picked while a section is compared closes the comparison, so one card stands there', () => { + const legVariants = idleLegVariants(); + const view = renderStage(comparingPlanner(legVariants)); + expect(screen.getByTestId('leg-variants-overlay')).toBeInTheDocument(); + pickFerrata(); + expect(legVariants.close).toHaveBeenCalledTimes(1); + + view.rerender(); + expect(screen.getByTestId('ferrata-detail-overlay')).toBeInTheDocument(); + expect(screen.queryByTestId('leg-variants-overlay')).not.toBeInTheDocument(); + }); + + it('FE-PAGE-TOURSTAGE-022: a comparison opened over a via ferrata card closes that card instead', () => { + const legVariants = idleLegVariants(); + const view = renderStage(planner('new-draft', { waypoints, legVariants })); + pickFerrata(); + expect(screen.getByTestId('ferrata-detail-overlay')).toBeInTheDocument(); + + view.rerender(); + expect(screen.queryByTestId('ferrata-detail-overlay')).not.toBeInTheDocument(); + expect(screen.getByTestId('leg-variants-overlay')).toBeInTheDocument(); + expect(legVariants.close).not.toHaveBeenCalled(); + }); + }); + describe('map layers per tour type (P2B-06/07, PAD-04, PAD-10)', () => { const flat = () => stage.flatMap.props as MapViewProps; diff --git a/client/src/pages/tripPlanner/TourPlannerMapStage.tsx b/client/src/pages/tripPlanner/TourPlannerMapStage.tsx index a0d943702..7a957fdbb 100644 --- a/client/src/pages/tripPlanner/TourPlannerMapStage.tsx +++ b/client/src/pages/tripPlanner/TourPlannerMapStage.tsx @@ -23,6 +23,7 @@ import { useSettingsStore } from '../../store/settingsStore'; import { publishTourFerrataCrossings } from '../../store/tourFerrataCrossingStore'; import { publishTourMapViewport } from '../../store/tourMapViewportStore'; import { useFerrataAdopt } from './useFerrataAdopt'; +import { useOneCornerCard } from './useOneCornerCard'; import { useTourMapPoints } from './useTourMapPoints'; import { useTourStageLayers } from './useTourStageLayers'; import { useTrackLinePick } from './useTrackLinePick'; @@ -169,6 +170,13 @@ export function TourPlannerMapStage({ const poiMarkers = useTourPoiMarkers(); // RTE-02: while other ways of a section are compared, a click on the line sets nothing on it. const comparing = tourPlanner.legVariants.comparison !== null; + // The comparison's card shares the corner with the others: the one opened last stays. + useOneCornerCard({ + comparing, + closeComparison: tourPlanner.legVariants.close, + ferrataOpen: !!ferrataSelected, + closeFerrata: ferrata.close, + }); const shapeAt = useShapingClick(tourPlanner, { editable: !!waypoints.onMove, picking: pickOnLine || comparing }); // What each panel covers of the map, a collapsed one its rail. const leftInset = leftHidden ? 56 : leftPanelPx; @@ -282,7 +290,7 @@ export function TourPlannerMapStage({ insetEnd={cardInsetEnd} {...ferrataCardPlacement(terrain3d, bottomRowTop, terrainLegend)} /> - {/* RTE-02: the ways of one section compared, in the same corner as the other cards. */} + {/* RTE-02: the ways of one section compared, in the same corner as the other cards and never over one. */} void; + ferrataOpen: boolean; + closeFerrata: () => void; +}): void { + const paddleOpen = usePaddleObstacleStore((state) => state.selectedRef !== null); + const poiOpen = useSelectedPoi() !== null; + const selectPoi = useTourPoiStore((state) => state.select); + const otherOpen = ferrataOpen || paddleOpen || poiOpen; + const was = useRef({ comparing, otherOpen }); + + useLayoutEffect(() => { + const before = was.current; + was.current = { comparing, otherOpen }; + if (!comparing || !otherOpen) return; + // A card picked on the map while comparing takes the corner; a comparison opened over a card does. + if (!before.otherOpen) { + closeComparison(); + return; + } + closeFerrata(); + selectPaddleObstacle(null); + selectPoi(null); + }, [closeComparison, closeFerrata, comparing, otherOpen, selectPoi]); +}