diff --git a/client/src/components/Planner/DayPlanSidebar.test.tsx b/client/src/components/Planner/DayPlanSidebar.test.tsx index 071f58a92..887e8bef0 100644 --- a/client/src/components/Planner/DayPlanSidebar.test.tsx +++ b/client/src/components/Planner/DayPlanSidebar.test.tsx @@ -48,6 +48,13 @@ vi.mock('../Map/RouteCalculator', () => ({ calculateRoute: vi.fn().mockResolvedValue({ distanceText: '5 km', durationText: '1h', coordinates: [] }), generateGoogleMapsUrl: vi.fn().mockReturnValue('https://maps.google.com/...'), optimizeRoute: vi.fn().mockImplementation((places) => places), + // One leg per waypoint gap; the connector between two stops reads distanceText. + calculateRouteWithLegs: vi.fn().mockImplementation((waypoints) => Promise.resolve({ + distanceText: '2 km', durationText: '10 min', + legs: Array.from({ length: Math.max(0, (waypoints?.length ?? 0) - 1) }, () => ({ + distanceText: '2 km', durationText: '10 min', drivingText: '10 min', walkingText: '25 min', + })), + })), })) // PlaceAvatar needs IntersectionObserver @@ -1784,4 +1791,98 @@ describe('DayPlanSidebar', () => { await user.click(optimizeBtn) await waitFor(() => expect(onReorder).toHaveBeenCalledWith(10, expect.any(Array))) }) + + it('FE-PLANNER-DAYPLAN-101: mobile Route toggle shows inline leg distances without selecting the day (#1374)', async () => { + const user = userEvent.setup() + const onSelectDay = vi.fn() + const onToggleRoute = vi.fn() + const places = [ + buildPlace({ id: 1, name: 'A', lat: 48.85, lng: 2.35 }), + buildPlace({ id: 2, name: 'B', lat: 48.86, lng: 2.36 }), + ] + const day = buildDay({ id: 10, date: '2025-06-01', title: 'Day 1' }) + const assigns = { + '10': [ + buildAssignment({ id: 1, day_id: 10, order_index: 0, place: places[0] }), + buildAssignment({ id: 2, day_id: 10, order_index: 1, place: places[1] }), + ], + } + render() + // Distances are hidden until the user asks for them. + expect(screen.queryByText('2 km')).not.toBeInTheDocument() + await user.click(screen.getByRole('button', { name: 'Route' })) + // The leg distance appears inline… + expect(await screen.findByText('2 km')).toBeInTheDocument() + // …and the day was never selected, so on mobile the sheet stays open. + expect(onSelectDay).not.toHaveBeenCalled() + expect(onToggleRoute).not.toHaveBeenCalled() + }) + + it('FE-PLANNER-DAYPLAN-102: mobile Route toggle hides the distances again on second tap (#1374)', async () => { + const user = userEvent.setup() + const places = [ + buildPlace({ id: 1, name: 'A', lat: 48.85, lng: 2.35 }), + buildPlace({ id: 2, name: 'B', lat: 48.86, lng: 2.36 }), + ] + const day = buildDay({ id: 10, date: '2025-06-01', title: 'Day 1' }) + const assigns = { + '10': [ + buildAssignment({ id: 1, day_id: 10, order_index: 0, place: places[0] }), + buildAssignment({ id: 2, day_id: 10, order_index: 1, place: places[1] }), + ], + } + render() + const routeBtn = screen.getByRole('button', { name: 'Route' }) + await user.click(routeBtn) + expect(await screen.findByText('2 km')).toBeInTheDocument() + await user.click(routeBtn) + await waitFor(() => expect(screen.queryByText('2 km')).not.toBeInTheDocument()) + }) + + it('FE-PLANNER-DAYPLAN-103: two route-toggled days keep separate leg distances despite id overlap (#1374)', async () => { + const user = userEvent.setup() + const { calculateRouteWithLegs } = await import('../Map/RouteCalculator') + // Distance derived from the first waypoint's latitude, so each day yields a + // distinct text. With a flat (non-per-day) leg map, the shared first-place id (5) + // would let the last day overwrite the other — this guards that regression. + vi.mocked(calculateRouteWithLegs as any).mockImplementation((wp: any) => { + const lat = wp?.[0]?.lat ?? 0 + const txt = `${Math.round(lat * 100)} m` + return Promise.resolve({ + distanceText: txt, durationText: '1 min', + legs: Array.from({ length: Math.max(0, (wp?.length ?? 0) - 1) }, () => ({ + distanceText: txt, durationText: '1 min', drivingText: '1 min', walkingText: '1 min', + })), + }) + }) + const dayA = buildDay({ id: 10, date: '2025-06-01', title: 'Day 1' }) + const dayB = buildDay({ id: 11, date: '2025-06-02', title: 'Day 2' }) + // Both days start with an assignment whose id is 5 (the leg is keyed on the first + // place's id) — the collision the per-day nesting must keep apart. + const assigns = { + '10': [ + buildAssignment({ id: 5, day_id: 10, order_index: 0, place: buildPlace({ id: 1, name: 'A1', lat: 10.0, lng: 2.0 }) }), + buildAssignment({ id: 6, day_id: 10, order_index: 1, place: buildPlace({ id: 2, name: 'A2', lat: 10.01, lng: 2.01 }) }), + ], + '11': [ + buildAssignment({ id: 5, day_id: 11, order_index: 0, place: buildPlace({ id: 3, name: 'B1', lat: 20.0, lng: 3.0 }) }), + buildAssignment({ id: 7, day_id: 11, order_index: 1, place: buildPlace({ id: 4, name: 'B2', lat: 20.01, lng: 3.01 }) }), + ], + } + render() + const routeBtns = screen.getAllByRole('button', { name: 'Route' }) + await user.click(routeBtns[0]) // Day 1 + await user.click(routeBtns[1]) // Day 2 + // Each day shows its own distance, not the other's — proves per-day isolation. + expect(await screen.findByText('1000 m')).toBeInTheDocument() + expect(await screen.findByText('2000 m')).toBeInTheDocument() + }) }) diff --git a/client/src/components/Planner/DayPlanSidebar.tsx b/client/src/components/Planner/DayPlanSidebar.tsx index 85ff4ddd7..b1cfe4040 100644 --- a/client/src/components/Planner/DayPlanSidebar.tsx +++ b/client/src/components/Planner/DayPlanSidebar.tsx @@ -155,8 +155,18 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) { const [editTitle, setEditTitle] = useState('') const [isCalculating, setIsCalculating] = useState(false) const [routeInfo, setRouteInfo] = useState(null) - const [routeLegs, setRouteLegs] = useState>({}) - const [hotelLegs, setHotelLegs] = useState<{ top?: { seg: RouteSegment; name: string }; bottom?: { seg: RouteSegment; name: string } }>({}) + // Per-segment legs keyed by day id, then by the start place's assignment id (or the + // transport's reservation id). Nested per day so several Route-toggled mobile days + // can't collide in one flat map — assignment ids and reservation ids come from + // independent sequences and would overwrite each other across days (#1374). + const [routeLegs, setRouteLegs] = useState>>({}) + // Hotel bookend legs keyed by day id. Desktop keys only the selected day; mobile + // keys every day whose Route toggle is on, so each shows its own bookends (#1374). + const [hotelLegs, setHotelLegs] = useState>({}) + // Mobile only: days the user tapped "Route" on. Their leg distances show inline in + // the expanded day, so seeing distances doesn't require selecting the day (which + // closes the mobile sheet) — #1374. + const [expandedRouteDayIds, setExpandedRouteDayIds] = useState>(new Set()) const optimizeFromAccommodation = useSettingsStore(s => s.settings.optimize_from_accommodation) // Recompute the hotel/route legs when the user flips km↔mi so the connector // distances refresh instead of showing stale cached text (#1300). @@ -383,108 +393,138 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) { // eslint-disable-next-line react-hooks/exhaustive-deps }, [days, assignments, dayNotes, reservations, transportPosVersion]) - // Per-segment driving times for the selected day's connectors. Groups located - // places into runs (split at transports), one cached OSRM call per run, keyed by - // the start place's assignment id. Shares RouteCalculator's cache with the map. + // Days whose inline route legs should be computed & shown. Desktop: the selected + // day while the Route toggle is on. Mobile: each expanded day the user tapped + // "Route" on — shown inline so seeing distances between places doesn't require + // selecting the day, which would close the mobile sheet (#1374). + const routeDayIds = useMemo(() => ( + showRouteToolsWhenExpanded + ? days.filter(d => expandedRouteDayIds.has(d.id) && expandedDays.has(d.id)).map(d => d.id) + : (routeShown && selectedDayId ? [selectedDayId] : []) + ), [showRouteToolsWhenExpanded, expandedRouteDayIds, expandedDays, days, routeShown, selectedDayId]) + const routeDayKey = routeDayIds.join(',') + + // Per-segment travel times shown as connectors between a day's located stops. + // Groups located places into runs (split at transports), one cached OSRM call per + // run keyed by the start place's assignment id, plus the hotel bookend legs. Shares + // RouteCalculator's cache with the map. Runs for every day in routeDayIds — one + // selected day on desktop, each Route-toggled day on mobile (#1374). useEffect(() => { if (legsAbortRef.current) legsAbortRef.current.abort() - if (!selectedDayId || !routeShown) { setRouteLegs({}); setHotelLegs({}); return } - const merged = mergedItemsMap[selectedDayId] || [] - const runs: { id: number; lat: number; lng: number }[][] = [] - let cur: { id: number; lat: number; lng: number }[] = [] - // A run is only a real drive when it holds an actual place. Two back-to-back - // transports (e.g. two flights on one day) would otherwise pair the first's - // arrival with the second's departure into a phantom airport→airport leg — the - // flight, not a drive — and surface it as a bogus connector distance (#1394). - let curHasPlace = false - for (const it of merged) { - if (it.type === 'place' && it.data.place?.lat && it.data.place?.lng) { - cur.push({ id: it.data.id, lat: it.data.place.lat, lng: it.data.place.lng }) - curHasPlace = true - } else if (it.type === 'transport') { - const r = it.data - const { from, to } = getTransportRouteEndpoints(r, selectedDayId) - if (from || to) { - // Located transport: route to its departure point, break the run (the - // flight/train itself isn't driven), and let its arrival start the next. - if (from) cur.push({ id: r.id, lat: from.lat, lng: from.lng }) - if (cur.length >= 2 && curHasPlace) runs.push(cur) - cur = [] - curHasPlace = false - if (to) cur.push({ id: r.id, lat: to.lat, lng: to.lng }) - } else if (cur.length > 0) { - // No location: ignore for routing, but attribute the through-leg to the - // booking so its distance/duration shows under it (purely cosmetic). - cur[cur.length - 1] = { ...cur[cur.length - 1], id: r.id } + if (routeDayIds.length === 0) { setRouteLegs({}); setHotelLegs({}); return } + + const hotelName = (a: Accommodation) => (a as any).place_name || (a as any).reservation_title || '' + + // Pure per-day plan: the drive runs (each ≥2 waypoints) and which hotel bookend + // legs to draw. Side-effect free, so the async loop below only does OSRM I/O. + const planDay = (dayId: number) => { + const merged = mergedItemsMap[dayId] || [] + const runs: { id: number; lat: number; lng: number }[][] = [] + let cur: { id: number; lat: number; lng: number }[] = [] + // A run is only a real drive when it holds an actual place. Two back-to-back + // transports (e.g. two flights on one day) would otherwise pair the first's + // arrival with the second's departure into a phantom airport→airport leg — the + // flight, not a drive — and surface it as a bogus connector distance (#1394). + let curHasPlace = false + for (const it of merged) { + if (it.type === 'place' && it.data.place?.lat && it.data.place?.lng) { + cur.push({ id: it.data.id, lat: it.data.place.lat, lng: it.data.place.lng }) + curHasPlace = true + } else if (it.type === 'transport') { + const r = it.data + const { from, to } = getTransportRouteEndpoints(r, dayId) + if (from || to) { + // Located transport: route to its departure point, break the run (the + // flight/train itself isn't driven), and let its arrival start the next. + if (from) cur.push({ id: r.id, lat: from.lat, lng: from.lng }) + if (cur.length >= 2 && curHasPlace) runs.push(cur) + cur = [] + curHasPlace = false + if (to) cur.push({ id: r.id, lat: to.lat, lng: to.lng }) + } else if (cur.length > 0) { + // No location: ignore for routing, but attribute the through-leg to the + // booking so its distance/duration shows under it (purely cosmetic). + cur[cur.length - 1] = { ...cur[cur.length - 1], id: r.id } + } } } - } - if (cur.length >= 2 && curHasPlace) runs.push(cur) + if (cur.length >= 2 && curHasPlace) runs.push(cur) - // Hotel bookend legs: the drive from the day's accommodation to the first located - // waypoint of the day (morning) and from the last one back to it (evening). Only when - // the "optimize from accommodation" setting is on and the day has a hotel. - const day = days.find(d => d.id === selectedDayId) - const bookends = day && optimizeFromAccommodation !== false - ? getDayBookendHotels(day, days, accommodations) - : null - const startHotel = bookends?.morning - const endHotel = bookends?.evening - const hotelName = (a: Accommodation) => (a as any).place_name || (a as any).reservation_title || '' - // Waypoints include transport endpoints (a car return, a taxi/train arrival), so the hotel - // legs connect even when the day starts or ends with a booking rather than a place. Track - // whether each is a place so we can skip a hotel↔transport leg that isn't real: on a day-1 - // arrival the check-in hotel never drove to the departure airport (#1321). - const wayPts: { lat: number; lng: number; isPlace: boolean }[] = [] - for (const it of merged) { - if (it.type === 'place' && it.data.place?.lat && it.data.place?.lng) { - wayPts.push({ lat: it.data.place.lat, lng: it.data.place.lng, isPlace: true }) - } else if (it.type === 'transport') { - const { from, to } = getTransportRouteEndpoints(it.data, selectedDayId) - if (from) wayPts.push({ lat: from.lat, lng: from.lng, isPlace: false }) - if (to) wayPts.push({ lat: to.lat, lng: to.lng, isPlace: false }) + // Hotel bookend legs: the drive from the day's accommodation to the first located + // waypoint of the day (morning) and from the last one back to it (evening). Only when + // the "optimize from accommodation" setting is on and the day has a hotel. + const day = days.find(d => d.id === dayId) + const bookends = day && optimizeFromAccommodation !== false + ? getDayBookendHotels(day, days, accommodations) + : null + const startHotel = bookends?.morning + const endHotel = bookends?.evening + // Waypoints include transport endpoints (a car return, a taxi/train arrival), so the hotel + // legs connect even when the day starts or ends with a booking rather than a place. Track + // whether each is a place so we can skip a hotel↔transport leg that isn't real: on a day-1 + // arrival the check-in hotel never drove to the departure airport (#1321). + const wayPts: { lat: number; lng: number; isPlace: boolean }[] = [] + for (const it of merged) { + if (it.type === 'place' && it.data.place?.lat && it.data.place?.lng) { + wayPts.push({ lat: it.data.place.lat, lng: it.data.place.lng, isPlace: true }) + } else if (it.type === 'transport') { + const { from, to } = getTransportRouteEndpoints(it.data, dayId) + if (from) wayPts.push({ lat: from.lat, lng: from.lng, isPlace: false }) + if (to) wayPts.push({ lat: to.lat, lng: to.lng, isPlace: false }) + } } + const firstWay = wayPts[0] + const lastWay = wayPts[wayPts.length - 1] + const wantTop = !!(startHotel && firstWay && (firstWay.isPlace || bookends?.morningIsSleptHere)) + const wantBottom = !!(endHotel && lastWay && (lastWay.isPlace || bookends?.eveningIsOvernight)) + return { runs, startHotel, endHotel, firstWay, lastWay, wantTop, wantBottom } } - const firstWay = wayPts[0] - const lastWay = wayPts[wayPts.length - 1] - const wantTop = !!(startHotel && firstWay && (firstWay.isPlace || bookends?.morningIsSleptHere)) - const wantBottom = !!(endHotel && lastWay && (lastWay.isPlace || bookends?.eveningIsOvernight)) - - if (runs.length === 0 && !wantTop && !wantBottom) { setRouteLegs({}); setHotelLegs({}); return } const controller = new AbortController() legsAbortRef.current = controller ;(async () => { - const map: Record = {} - for (const run of runs) { - try { - const r = await calculateRouteWithLegs(run.map(p => ({ lat: p.lat, lng: p.lng })), { signal: controller.signal, profile: routeProfile }) - r.legs.forEach((leg, i) => { map[run[i].id] = leg }) - } catch (err) { - if (err instanceof Error && err.name === 'AbortError') return - } - } + const legsByDay: Record> = {} + const hotelByDay: Record = {} - // One extra cached OSRM call per bookend; shares RouteCalculator's cache. + // One cached OSRM call per waypoint pair; shares RouteCalculator's cache. const legBetween = async (a: { lat: number; lng: number }, b: { lat: number; lng: number }): Promise => { try { const r = await calculateRouteWithLegs([a, b], { signal: controller.signal, profile: routeProfile }) return r.legs[0] } catch { return undefined } } - const hotel: { top?: { seg: RouteSegment; name: string }; bottom?: { seg: RouteSegment; name: string } } = {} - if (wantTop) { - const seg = await legBetween({ lat: startHotel!.place_lat as number, lng: startHotel!.place_lng as number }, { lat: firstWay.lat, lng: firstWay.lng }) - if (seg) hotel.top = { seg, name: hotelName(startHotel!) } - } - if (wantBottom) { - const seg = await legBetween({ lat: lastWay.lat, lng: lastWay.lng }, { lat: endHotel!.place_lat as number, lng: endHotel!.place_lng as number }) - if (seg) hotel.bottom = { seg, name: hotelName(endHotel!) } + + for (const dayId of routeDayIds) { + const { runs, startHotel, endHotel, firstWay, lastWay, wantTop, wantBottom } = planDay(dayId) + const dayLegs: Record = {} + for (const run of runs) { + try { + const r = await calculateRouteWithLegs(run.map(p => ({ lat: p.lat, lng: p.lng })), { signal: controller.signal, profile: routeProfile }) + r.legs.forEach((leg, i) => { dayLegs[run[i].id] = leg }) + } catch (err) { + if (err instanceof Error && err.name === 'AbortError') return + } + } + if (Object.keys(dayLegs).length) legsByDay[dayId] = dayLegs + const hotel: { top?: { seg: RouteSegment; name: string }; bottom?: { seg: RouteSegment; name: string } } = {} + if (wantTop) { + const seg = await legBetween({ lat: startHotel!.place_lat as number, lng: startHotel!.place_lng as number }, { lat: firstWay!.lat, lng: firstWay!.lng }) + if (seg) hotel.top = { seg, name: hotelName(startHotel!) } + } + if (wantBottom) { + const seg = await legBetween({ lat: lastWay!.lat, lng: lastWay!.lng }, { lat: endHotel!.place_lat as number, lng: endHotel!.place_lng as number }) + if (seg) hotel.bottom = { seg, name: hotelName(endHotel!) } + } + if (controller.signal.aborted) return + if (hotel.top || hotel.bottom) hotelByDay[dayId] = hotel } - if (!controller.signal.aborted) { setRouteLegs(map); setHotelLegs(hotel) } + if (!controller.signal.aborted) { setRouteLegs(legsByDay); setHotelLegs(hotelByDay) } })() - }, [selectedDayId, routeShown, routeProfile, mergedItemsMap, accommodations, days, optimizeFromAccommodation, distanceUnit]) + // routeDayIds is memoized from the same inputs as routeDayKey below, so keying the + // effect on the string is equivalent while staying stable across unrelated renders. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [routeDayKey, routeProfile, mergedItemsMap, accommodations, days, optimizeFromAccommodation, distanceUnit]) const openAddNote = (dayId, e) => { e?.stopPropagation() @@ -1060,6 +1100,8 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) { totalCost, anyGeoAssignment, anyGeoPlace, + expandedRouteDayIds, + setExpandedRouteDayIds, } } @@ -1219,6 +1261,8 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP totalCost, anyGeoAssignment, anyGeoPlace, + expandedRouteDayIds, + setExpandedRouteDayIds, } = S return (
@@ -1270,6 +1314,10 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP (routeBookends?.evening?.place_lat != null && routeBookends?.evening?.place_lng != null) ) const routeToolsRoutable = da.length >= 2 || (loc != null && hasRouteBookend) + // Is this day's inline route currently on? Mobile toggles it per day (its + // own expandedRouteDayIds entry); desktop uses the global Route toggle on + // the selected day (#1374). + const routeActive = showRouteToolsWhenExpanded ? expandedRouteDayIds.has(day.id) : (routeShown && isSelected) const isDragTarget = dragOverDayId === day.id const merged = mergedItemsMap[day.id] || [] const dayNoteUi = noteUi[day.id] @@ -1511,8 +1559,8 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP handleMergedDrop(day.id, 'note', Number(noteId), lastItem.type, lastItem.data.id, true) }} > - {isSelected && hotelLegs.top && ( - + {hotelLegs[day.id]?.top && ( + )} {merged.length === 0 && !dayNoteUi ? (
)}
- {routeLegs[assignment.id] && } + {routeLegs[day.id]?.[assignment.id] && } ) } @@ -2040,7 +2088,7 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP ) })()}
- {routeLegs[res.id] && } + {routeLegs[day.id]?.[res.id] && } ) } @@ -2148,8 +2196,8 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP ) }) )} - {isSelected && hotelLegs.bottom && ( - + {hotelLegs[day.id]?.bottom && ( + )} {/* Drop-Zone am Listenende — immer vorhanden als Drop-Target */}