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]); +}