From 112017a9e85548ee8bdc5e89b0e51e994d66d77b Mon Sep 17 00:00:00 2001 From: Maurice Date: Tue, 20 Oct 2026 22:44:42 +0200 Subject: [PATCH] fix(tours): clear the stage spread's lint, read the vias after render and test against the reloaded vias The vias the spread keeps on their legs are taken over in a layout effect instead of during render, the route redraw takes the planner's async updateRouteForDay, and the menu items for whole or stages move out of the button component into their own module. Two leftovers of the old day pills in tourParts go. The hook test's reanchor reloads the vias like the real one does, and the detail test finds the button by its label, Add to Day. --- .../src/components/Tours/TourDetailDialog.tsx | 2 +- .../Tours/stages/TourDayAddButton.tsx | 49 +------------------ .../Tours/stages/tourDayChoiceViews.test.tsx | 4 +- .../Tours/stages/tourDayHowItems.ts | 49 +++++++++++++++++++ client/src/components/Tours/tourParts.tsx | 7 +-- .../tripPlanner/useTourDayChoice.test.ts | 31 +++++++++--- .../src/pages/tripPlanner/useTourDayChoice.ts | 35 ++++++------- 7 files changed, 97 insertions(+), 80 deletions(-) create mode 100644 client/src/components/Tours/stages/tourDayHowItems.ts diff --git a/client/src/components/Tours/TourDetailDialog.tsx b/client/src/components/Tours/TourDetailDialog.tsx index 8726299a2..d9b7e6716 100644 --- a/client/src/components/Tours/TourDetailDialog.tsx +++ b/client/src/components/Tours/TourDetailDialog.tsx @@ -57,8 +57,8 @@ import { PaddleWindSection } from './paddle/PaddleWindCard'; import { TourProvenanceBlock } from './provenance/TourProvenanceSection'; import { RunHeadPills } from './run/RunHeadPills'; import { SkiHeadPill } from './ski/skiMarks'; -import { stageKeysOfTour, type TourDayHow } from './stages/stageSpread'; import { TourDayAddButton } from './stages/TourDayAddButton'; +import { stageKeysOfTour, type TourDayHow } from './stages/stageSpread'; import { tourDayRowIcon } from './tourDayRowIcon'; import { tourDaysOf } from './tourDays'; import { heightsUnknown, movingTimeLabel, withoutHeights } from './tourFacts'; diff --git a/client/src/components/Tours/stages/TourDayAddButton.tsx b/client/src/components/Tours/stages/TourDayAddButton.tsx index 82a537bee..7ded23cbe 100644 --- a/client/src/components/Tours/stages/TourDayAddButton.tsx +++ b/client/src/components/Tours/stages/TourDayAddButton.tsx @@ -1,54 +1,9 @@ -import { CalendarDays, CalendarRange, ChevronDown, Plus } from 'lucide-react'; +import { ChevronDown, Plus } from 'lucide-react'; import type { MouseEvent } from 'react'; import { useTranslation } from '../../../i18n'; -import type { Day } from '../../../types'; import { ContextMenu, useContextMenu } from '../../shared/ContextMenu'; import { DialogButton } from '../../shared/DialogShell'; -import { dayMenuLabel } from '../tourDays'; -import type { TourDayAssignment } from '../useTourDayAssignment'; -import { stagesThatFit, tourDayChoices } from './stageSpread'; - -type Translate = ReturnType['t']; -type MenuItems = Parameters['open']>[1]; - -interface TourDayHowInput { - placeId: number; - dayId: number; - days: Day[]; - /** The stages of the multi-day tour. */ - stageCount: number; - assignment: TourDayAssignment; -} - -/** - * MDT-02: the two ways a multi-day tour goes on a day, as menu items: whole, - * or a stage a day from it on, under the day they act on, so the step after - * the day menu still says which day was picked. Whole is greyed out on a day - * that holds the tour already, and says so; the stages say how many fit when - * the trip ends before them. - */ -export function tourDayHowItems(input: TourDayHowInput, t: Translate, locale: string): MenuItems { - const { placeId, dayId, days, stageCount, assignment } = input; - const facts = { - count: stageCount, - fit: stagesThatFit(days, dayId, stageCount), - wholeFree: !assignment.isOnDay(placeId, dayId), - }; - const ways = tourDayChoices(facts, t).map((choice) => { - const disabled = choice.disabled || !assignment.isAvailable(placeId, dayId, choice.how); - return { - label: choice.title, - // Greyed out with no reason reads as broken; the reason goes right under it. - hint: choice.disabled && choice.how === 'whole' ? choice.hint : undefined, - icon: choice.how === 'whole' ? CalendarDays : CalendarRange, - disabled, - onClick: () => void assignment.assign(placeId, dayId, choice.how), - }; - }); - const at = days.findIndex((day) => day.id === dayId); - if (at < 0) return ways; - return [{ label: dayMenuLabel(days[at], at, t, locale), heading: true }, ...ways]; -} +import { tourDayHowItems, type TourDayHowInput } from './tourDayHowItems'; /** * The detail's "Add to day" for the day selected in the plan. A day tour goes diff --git a/client/src/components/Tours/stages/tourDayChoiceViews.test.tsx b/client/src/components/Tours/stages/tourDayChoiceViews.test.tsx index 0ef25d164..03a39102c 100644 --- a/client/src/components/Tours/stages/tourDayChoiceViews.test.tsx +++ b/client/src/components/Tours/stages/tourDayChoiceViews.test.tsx @@ -68,13 +68,13 @@ describe('the detail button and the dialog', () => { it('FE-TOUR-STAGE-SPREAD-019: the detail adds a day tour at once and offers a multi-day tour both ways', async () => { const props = { placeId: 81, dayId: 4, days: DAYS, assignment }; const { unmount } = render(); - await userEvent.click(screen.getByRole('button', { name: 'Add to day' })); + await userEvent.click(screen.getByRole('button', { name: 'Add to Day' })); expect(assignment.assign).toHaveBeenCalledWith(81, 4); unmount(); vi.mocked(assignment.isOnDay).mockReturnValue(true); render(); - await userEvent.click(screen.getByRole('button', { name: 'Add to day' })); + await userEvent.click(screen.getByRole('button', { name: 'Add to Day' })); const whole = screen.getByRole('button', { name: /^Whole tour on this day/ }); expect(whole).toBeDisabled(); expect(whole).toHaveTextContent('This day already holds the tour.'); diff --git a/client/src/components/Tours/stages/tourDayHowItems.ts b/client/src/components/Tours/stages/tourDayHowItems.ts new file mode 100644 index 000000000..21460733a --- /dev/null +++ b/client/src/components/Tours/stages/tourDayHowItems.ts @@ -0,0 +1,49 @@ +import { CalendarDays, CalendarRange } from 'lucide-react'; +import type { useTranslation } from '../../../i18n'; +import type { Day } from '../../../types'; +import type { useContextMenu } from '../../shared/ContextMenu'; +import { dayMenuLabel } from '../tourDays'; +import type { TourDayAssignment } from '../useTourDayAssignment'; +import { stagesThatFit, tourDayChoices } from './stageSpread'; + +type Translate = ReturnType['t']; +type MenuItems = Parameters['open']>[1]; + +export interface TourDayHowInput { + placeId: number; + dayId: number; + days: Day[]; + /** The stages of the multi-day tour. */ + stageCount: number; + assignment: TourDayAssignment; +} + +/** + * MDT-02: the two ways a multi-day tour goes on a day, as menu items: whole, + * or a stage a day from it on, under the day they act on, so the step after + * the day menu still says which day was picked. Whole is greyed out on a day + * that holds the tour already, and says so; the stages say how many fit when + * the trip ends before them. + */ +export function tourDayHowItems(input: TourDayHowInput, t: Translate, locale: string): MenuItems { + const { placeId, dayId, days, stageCount, assignment } = input; + const facts = { + count: stageCount, + fit: stagesThatFit(days, dayId, stageCount), + wholeFree: !assignment.isOnDay(placeId, dayId), + }; + const ways = tourDayChoices(facts, t).map((choice) => { + const disabled = choice.disabled || !assignment.isAvailable(placeId, dayId, choice.how); + return { + label: choice.title, + // Greyed out with no reason reads as broken; the reason goes right under it. + hint: choice.disabled && choice.how === 'whole' ? choice.hint : undefined, + icon: choice.how === 'whole' ? CalendarDays : CalendarRange, + disabled, + onClick: () => void assignment.assign(placeId, dayId, choice.how), + }; + }); + const at = days.findIndex((day) => day.id === dayId); + if (at < 0) return ways; + return [{ label: dayMenuLabel(days[at], at, t, locale), heading: true }, ...ways]; +} diff --git a/client/src/components/Tours/tourParts.tsx b/client/src/components/Tours/tourParts.tsx index e3749bb48..a3e66575d 100644 --- a/client/src/components/Tours/tourParts.tsx +++ b/client/src/components/Tours/tourParts.tsx @@ -9,7 +9,7 @@ import { ContextMenu, useContextMenu } from '../shared/ContextMenu' import { DialogButton, fs } from '../shared/DialogShell' import { Tooltip } from '../shared/Tooltip' import { BOX, Eyebrow } from '../Planner/bookings/bookingParts' -import { SoftPill, WhiteBadge } from '../Planner/planParts' +import { WhiteBadge } from '../Planner/planParts' import { hikeSourceBadgeLabel, tourSource } from './tourPresentation' import { type TourDay, dayMenuLabel } from './tourDays' import { formatDate } from '../../utils/formatters' @@ -18,7 +18,7 @@ import type { TourDayAssignment } from './useTourDayAssignment' import { useRailFold } from './railFold' import { tourDayRowIcon } from './tourDayRowIcon' import { SkiCornerMark } from './ski/skiMarks' -import { tourDayHowItems } from './stages/TourDayAddButton' +import { tourDayHowItems } from './stages/tourDayHowItems' /** * A tour's tile in a list: its T-scale grade in place of the mountain, named in @@ -87,9 +87,6 @@ export function TourListCard({ icon, ) } -/** How many days a list row names before the rest fold into one "+n" pill. */ -const SHOWN_TOUR_DAYS = 3 - /** The days a tour is on as one white badge of a plan row: "Days 2, 4", each day named in full in the tooltip. */ export function TourDaysBadge({ tourDays }: { tourDays: readonly TourDay[] }) { const { t, locale } = useTranslation() diff --git a/client/src/pages/tripPlanner/useTourDayChoice.test.ts b/client/src/pages/tripPlanner/useTourDayChoice.test.ts index 59dea846a..a0664084e 100644 --- a/client/src/pages/tripPlanner/useTourDayChoice.test.ts +++ b/client/src/pages/tripPlanner/useTourDayChoice.test.ts @@ -4,7 +4,7 @@ // their legs. import { act, renderHook } from '@testing-library/react'; import type { TourListItem } from '@trek/shared'; -import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { beforeEach, describe, expect, it, vi, type Mock } from 'vitest'; import { buildAssignment, buildDay, buildPlace } from '../../../tests/helpers/factories'; import { resetAllStores, seedStore } from '../../../tests/helpers/store'; import { useTripStore } from '../../store/tripStore'; @@ -22,7 +22,7 @@ const TOURS = [ ] as unknown as TourListItem[]; const t = (key: string, params?: Record) => (params ? `${key} ${JSON.stringify(params)}` : key); -let writes: Record>; +let writes: Record; let options: Options; function seed(assignments: AssignmentsMap) { @@ -66,15 +66,23 @@ function writesThatLand() { const setDay = (dayId: number, items: Assignment[]) => useTripStore.setState((state) => ({ assignments: { ...state.assignments, [String(dayId)]: items } })); writes.removeAssignment.mockImplementation(async (_trip: number, dayId: number, id: number) => { - setDay(dayId, dayOf(dayId).filter((item) => item.id !== id)); + setDay( + dayId, + dayOf(dayId).filter((item) => item.id !== id) + ); }); writes.assignPlaceToDay.mockImplementation( async (_trip: number, dayId: number, placeId: number, position: number | null, key?: string) => { - const rows = dayOf(dayId).slice().sort((a, b) => a.order_index - b.order_index); + const rows = dayOf(dayId) + .slice() + .sort((a, b) => a.order_index - b.order_index); const added = buildAssignment({ id: 900 + dayId, day_id: dayId, place: buildPlace({ id: placeId }) }); added.tour_stage_key = key ?? null; rows.splice(position ?? rows.length, 0, added); - setDay(dayId, rows.map((item, index) => ({ ...item, order_index: index }))); + setDay( + dayId, + rows.map((item, index) => ({ ...item, order_index: index })) + ); return added; } ); @@ -218,12 +226,23 @@ describe('useTourDayChoice', () => { seed({ '1': [], '2': [stop(50, 0, 100), whole, stop(51, 2, 101), stop(52, 3, 102)], '3': [] }); writesThatLand(); // One via on the leg into the tour, one on the leg after it (stop 51 to stop 52). - options.roadtripVias.byDay = { + const road = options.roadtripVias; + road.byDay = { 2: [ { id: 701, day_id: 2, after_order_index: 0, lat: 1, lng: 1 }, { id: 700, day_id: 2, after_order_index: 2, lat: 2, lng: 2 }, ] as never, }; + // The real one writes the new anchors and loads the day's vias again, so the undo reads them from there. + vi.mocked(road.reanchor).mockImplementation(async (dayId, plan) => { + const moved = new Map(plan.vias.map((via) => [via.id, via.after_order_index] as const)); + road.byDay = { + ...road.byDay, + [dayId]: (road.byDay[dayId] ?? []) + .filter((via) => !plan.remove.includes(via.id)) + .map((via) => ({ ...via, after_order_index: moved.get(via.id) ?? via.after_order_index })), + }; + }); const { result } = render(); await act(async () => { expect(await result.current.handleAssignToDay(9, 1, undefined, 'stages')).toBe(true); diff --git a/client/src/pages/tripPlanner/useTourDayChoice.ts b/client/src/pages/tripPlanner/useTourDayChoice.ts index 928204e1c..0be9e07ed 100644 --- a/client/src/pages/tripPlanner/useTourDayChoice.ts +++ b/client/src/pages/tripPlanner/useTourDayChoice.ts @@ -1,5 +1,12 @@ import type { TourListItem } from '@trek/shared'; -import { useCallback, useRef, useState } from 'react'; +import { useCallback, useLayoutEffect, useRef, useState } from 'react'; +import { + isEmptyReanchoring, + reanchorAfterInsert, + reanchorAfterRemove, + type Reanchoring, +} from '../../components/Roadtrip/roadtripModel'; +import type { RoadtripVias } from '../../components/Roadtrip/useRoadtripVias'; import { planStageSpread, stageKeysOfTour, @@ -9,13 +16,6 @@ import { type TourDayHow, type TourDayQuestion, } from '../../components/Tours/stages/stageSpread'; -import { - isEmptyReanchoring, - reanchorAfterInsert, - reanchorAfterRemove, - type Reanchoring, -} from '../../components/Roadtrip/roadtripModel'; -import type { RoadtripVias } from '../../components/Roadtrip/useRoadtripVias'; import { isTourOnDay } from '../../components/Tours/useTourDayAssignment'; import { useTripStore } from '../../store/tripStore'; import type { Day, Place } from '../../types'; @@ -39,7 +39,7 @@ interface TourDayChoiceOptions { pushUndo: (label: string, undoFn: () => Promise | void, dayIds?: number[], placeIds?: number[]) => void; /** The plan's one way of putting a place on a day, which puts a tour there whole. */ assignWhole: (placeId: number, dayId?: number, position?: number) => Promise; - updateRouteForDay: (dayId: number) => void; + updateRouteForDay: (dayId: number) => Promise | void; onToursChanged: () => Promise | void; /** The road trip's vias, which every write that shifts the stops of a day carries along. */ roadtripVias: Pick; @@ -123,12 +123,7 @@ type Undo = (road: ViaKeeper) => Promise; * middle of one carries the day's vias along (`viaKeeper`). The undo runs * later, and is handed the keeper of its own run. */ -async function runStep( - tripId: number, - placeId: number, - step: SpreadStep, - road: ViaKeeper -): Promise { +async function runStep(tripId: number, placeId: number, step: SpreadStep, road: ViaKeeper): Promise { const store = useTripStore.getState(); if (step.kind === 'move') { await road.takeOff(step.fromDayId, step.assignmentId, () => @@ -193,12 +188,14 @@ function spreadMessage(plan: StageSpread, days: readonly Day[], t: Translate): s */ export function useTourDayChoice(options: TourDayChoiceOptions) { const { tripId, days, tours, selectedDayId, t, toast } = options; - const { pushUndo, assignWhole, updateRouteForDay, onToursChanged } = options; + const { pushUndo, assignWhole, updateRouteForDay, onToursChanged, roadtripVias, viaLiesBefore } = options; const [tourDayQuestion, setTourDayQuestion] = useState(null); const answer = useRef<((how: TourDayHow | null) => void) | null>(null); // The vias as the plan has them now: an undo runs long after the spread that pushed it. - const viaFeed = useRef(options); - viaFeed.current = { roadtripVias: options.roadtripVias, viaLiesBefore: options.viaLiesBefore }; + const viaFeed = useRef({ roadtripVias, viaLiesBefore }); + useLayoutEffect(() => { + viaFeed.current = { roadtripVias, viaLiesBefore }; + }, [roadtripVias, viaLiesBefore]); const answerTourDay = useCallback((how: TourDayHow | null) => { const resolve = answer.current; @@ -247,7 +244,7 @@ export function useTourDayChoice(options: TourDayChoiceOptions) { const touched = [...new Set(plan.steps.flatMap(daysOf))]; const redraw = () => { const selected = useTripStore.getState().selectedDayId; - if (selected != null && touched.includes(selected)) updateRouteForDay(selected); + if (selected != null && touched.includes(selected)) void updateRouteForDay(selected); }; redraw(); await onToursChanged();