diff --git a/client/src/components/Roadtrip/RoadtripLimitsCard.test.tsx b/client/src/components/Roadtrip/RoadtripLimitsCard.test.tsx index f9eecae35..39f61bf9e 100644 --- a/client/src/components/Roadtrip/RoadtripLimitsCard.test.tsx +++ b/client/src/components/Roadtrip/RoadtripLimitsCard.test.tsx @@ -278,7 +278,7 @@ describe('RoadtripLimitsCard', () => { expect(onSave).toHaveBeenCalledWith('roadtrip_connect_days', false) }) - it('FE-ROADTRIP-LIMITS-018: starting and ending each day at the stay is off until it is asked for', async () => { + it('FE-ROADTRIP-LIMITS-018: starting and ending each day at the stay is off until it is asked for', () => { const onSave = vi.fn() open(onSave) @@ -288,11 +288,16 @@ describe('RoadtripLimitsCard', () => { fireEvent.click(toggle) expect(onSave).toHaveBeenCalledWith('roadtrip_hotel_bookends', true) expect(onSave).toHaveBeenCalledTimes(1) - // What it does is said on the label, where the panel has no room for it. - fireEvent.mouseEnter(screen.getByText('Start and end each day at your stay')) - expect(await screen.findByRole('tooltip')).toHaveTextContent( - 'After a booked night the day starts where you slept, and before one it ends there.', + // What it does is said under it for everybody to read, and the switch is described by + // that sentence, so a screen reader hears it with the switch rather than never. + const hint = screen.getByText( + 'After a booked night the day starts at that stay, and before one it ends at the stay booked for that night.', ) + expect(toggle).toHaveAttribute('aria-describedby', hint.id) + expect(hint.id).not.toBe('') + expect(screen.queryByRole('tooltip')).toBeNull() + // A switch that needs no sentence is described by none. + expect(screen.getByRole('button', { name: 'Connect the days' })).not.toHaveAttribute('aria-describedby') }) it('FE-ROADTRIP-LIMITS-019: switched on, it reads on and reports the way back off', () => { diff --git a/client/src/components/Roadtrip/RoadtripLimitsCard.tsx b/client/src/components/Roadtrip/RoadtripLimitsCard.tsx index 8839dc477..71221fad9 100644 --- a/client/src/components/Roadtrip/RoadtripLimitsCard.tsx +++ b/client/src/components/Roadtrip/RoadtripLimitsCard.tsx @@ -1,5 +1,5 @@ import { useRoadtripSettings } from '../../hooks/useRoadtripSettings' -import React, { useState } from 'react' +import React, { useId, useState } from 'react' import { Clock, Fuel, CalendarClock, SlidersHorizontal, ChevronRight, Coins, Signpost, Ship, Check, Car, Zap, BatteryCharging, BatteryFull, BatteryWarning, Gauge, Route, Sparkles, Link2, Palette, @@ -18,7 +18,6 @@ import { FS } from './typeScale' import DayWindowFields from './DayWindowFields' import SettingsHint from './SettingsHint' import { dayWindow } from './dayWindow' -import { Tooltip } from '../shared/Tooltip' import { BOOKEND_ICON } from './nightBookend' import { useHotelBookends } from './useHotelBookends' @@ -166,7 +165,9 @@ function LimitRow({ icon: Icon, label, suffix, value, placeholder, step, derived * pointed the instance at their own OSRM has no second engine, and a switch that flips * and changes nothing is worse than one that says why it cannot. * - * A `hint` is the sentence a switch needs and its panel has no room for, on the label. + * A `hint` is the sentence a switch needs to say what flipping it changes. It stands under + * the label, in the caption the panels close on, and the switch is described by it: behind a + * hover it was a sentence nobody found, and a screen reader never heard. */ function AvoidRow({ icon: Icon, label, hint, on, disabled, onToggle }: { icon: typeof Coins @@ -176,14 +177,17 @@ function AvoidRow({ icon: Icon, label, hint, on, disabled, onToggle }: { disabled: boolean onToggle: () => void }): React.ReactElement { - const text = {label} + const hintId = useId() return (
- {hint ? {text} : text} + + {label} + {hint ? {hint} : null} + {disabled ? {'—'} - : } + : }
) } diff --git a/client/src/components/Roadtrip/RoadtripSidebar.test.tsx b/client/src/components/Roadtrip/RoadtripSidebar.test.tsx index 2cc8e309c..57604e71e 100644 --- a/client/src/components/Roadtrip/RoadtripSidebar.test.tsx +++ b/client/src/components/Roadtrip/RoadtripSidebar.test.tsx @@ -1545,4 +1545,61 @@ describe('RoadtripSidebar with a booked night at the edge of the day', () => { const hotelRow = screen.getByText('Back to Hotel Alpenblick').closest('li')! expect(within(hotelRow).getByText('+30 min')).toBeInTheDocument() }) + + /** Whether a row's disc has the rail running into it from above. */ + const lineAbove = (text: string): boolean => { + const rail = screen.getByText(text).closest('button')!.firstElementChild! + return rail.firstElementChild!.className.includes('w-[1.5px]') + } + + it('FE-ROADTRIP-SIDEBAR-071: the hotel row the morning marker went into starts the rail, with no line running in from nowhere', () => { + const resume = stop({ + assignmentId: -2_000_000_003, + name: 'Continue journey', + lat: 47.2, + lng: 11.4, + automaticNight: { phase: 'start', fromDayNumber: 1 }, + }) + const stops = [resume, bookend('morning', { checkingOut: true }, { ownerIndex: 0 }), stop({ assignmentId: 1, name: 'Lookout', ownerIndex: 0 })] + wrap() + expect(lineAbove('Check-out · Hotel Alpenblick')).toBe(false) + // Below it the rail runs on as always. + expect(lineAbove('Lookout')).toBe(true) + }) + + /** A transfer day: out of one stay and into the next, with nothing stored in between. */ + const transfer = () => day({ + dayId: 2, + dayNumber: 2, + stops: [ + bookend('morning', { checkingOut: true }, { ownerIndex: 0 }), + bookend('evening', { accommodationId: 6, checkingIn: true }, { ownerIndex: 0, name: 'Wallinga', lat: 48, lng: 12 }), + ], + legs: [leg()], + }) + + it('FE-ROADTRIP-SIDEBAR-072: a day that only drives from one stay to the next says its drive and no count of stops', () => { + wrap() + expect(screen.getByText('Check-in · Wallinga')).toBeInTheDocument() + const header = screen.getByText('Day 2').closest('header')! + expect(within(header).getByText(/100 km/)).toBeInTheDocument() + expect(within(header).queryByText(/stops?$/)).toBeNull() + expect(screen.queryByText('0 stops')).toBeNull() + }) + + it('FE-ROADTRIP-SIDEBAR-073: a stop dragged onto such a day lands as its first stop, dropped on either hotel', () => { + const onMoveStopToDay = vi.fn() + const { container } = wrap( + , + ) + const hotelRow = screen.getByText('Check-in · Wallinga').closest('li')! + const rows = container.querySelectorAll('li[draggable="true"]') + fireEvent.dragStart(rows[1], { dataTransfer: { effectAllowed: '', setData: vi.fn() } }) + fireEvent.dragOver(hotelRow) + expect(hotelRow.className).toContain('ring-accent') + fireEvent.drop(hotelRow) + + // From day 1, its second stop (assignment 2), onto day 2 as its first. + expect(onMoveStopToDay).toHaveBeenCalledWith(1, 2, 2, 0) + }) }) diff --git a/client/src/components/Roadtrip/RoadtripSidebar.tsx b/client/src/components/Roadtrip/RoadtripSidebar.tsx index 55f623461..dbfc7401a 100644 --- a/client/src/components/Roadtrip/RoadtripSidebar.tsx +++ b/client/src/components/Roadtrip/RoadtripSidebar.tsx @@ -24,7 +24,7 @@ import StopKindPicker from './StopKindPicker' import StopFillPicker from './StopFillPicker' import { useVehicleRange } from './useVehicleRange' import { MAX_TRIP_DAYS, type RoadtripStopType } from '@trek/shared' -import { isCarrierMode, undatedRides, type CarrierTerminal } from '@trek/shared/roadtrip' +import { isCarrierMode, isStoredStop, undatedRides, type CarrierTerminal } from '@trek/shared/roadtrip' import type { QuietDay, RoadtripDay, RoadtripRoutes, RoadtripStop } from './useRoadtripRoutes' import type { SpillMark } from './nightSpill' import { ARRIVING_DRIVE, openOn, type LegAlternatives, type RailDrive } from './useRouteAlternatives' @@ -1611,6 +1611,24 @@ function DaySection({ day, selectedAssignmentId, onSelectStop, onOpenBooking, ca // The running number a stop wears, with the service stops passed over — so a day with a // charger halfway through still counts one, two, three the way its map pins do. let counted = 0 + // A day of nothing but its hotels, the drive from one stay to the next. The hotels make it + // a day, so it is no quiet row a stop can be dropped on, and it has no stored stop to drop + // onto either: its hotel rows take the drop instead, as the day's first stop. + const dropOnHotels: React.LiHTMLAttributes = onMoveStopToDay && from && !day.stops.some(isStoredStop) + ? { + onDragOver: e => { + e.preventDefault() + if (dropAt?.dayId !== day.dayId || dropAt.index !== -1) setDropAt({ dayId: day.dayId, index: -1 }) + }, + onDrop: e => { + e.preventDefault() + setFrom(null) + setDropAt(null) + if (from.dayId !== day.dayId) onMoveStopToDay(from.dayId, from.assignmentId, day.dayId, 0) + }, + className: `rounded-lg ${dropAt?.dayId === day.dayId && dropAt.index === -1 ? 'ring-2 ring-inset ring-accent' : ''}`, + } + : {} /** * One row of the chain: the stop, the drive leaving it, whatever hangs off that drive. @@ -1672,14 +1690,15 @@ function DaySection({ day, selectedAssignmentId, onSelectStop, onOpenBooking, ca if (booking !== null && onOpenBooking) open = () => onOpenBooking(booking) else if (onSelectStop) open = () => onSelectStop(stop.placeId) return ( -
  • +
  • {i < last && (!day.stops[i + 1].automaticNight || day.legs[i]?.distance !== 0) ? : null} @@ -1935,7 +1954,9 @@ function DaySection({ day, selectedAssignmentId, onSelectStop, onOpenBooking, ca ) : null} - {day.legs.length > 0 ? ( + {/* Nor does a day whose drive runs from one hotel to the next with no stop of its + own: "0 stops" beside the drive reads as the stops having gone missing. */} + {day.legs.length > 0 && destinationCount(day) > 0 ? ( {t('roadtrip.day.stopCount', { count: destinationCount(day) })} diff --git a/client/src/components/Roadtrip/nightBookend.test.ts b/client/src/components/Roadtrip/nightBookend.test.ts index 7017cd8e3..d4eb78c96 100644 --- a/client/src/components/Roadtrip/nightBookend.test.ts +++ b/client/src/components/Roadtrip/nightBookend.test.ts @@ -1,9 +1,9 @@ import { describe, expect, it } from 'vitest' import { BedDouble } from 'lucide-react' -import { BOOKEND_ICON, bookendBooking, bookendMeta, bookendTitle } from './nightBookend' +import { BOOKEND_ICON, bookendBooking, bookendMeta, bookendTitle, staysAtTheirPlaces } from './nightBookend' import type { BookendReading } from './roadtripRowModel' -// FE-BOOKEND-001 to FE-BOOKEND-005 +// FE-BOOKEND-001 to FE-BOOKEND-006 const reading = (over: Partial = {}): BookendReading => ({ phase: 'morning', @@ -50,3 +50,29 @@ describe('a booked night as the rail and the phone say it', () => { expect(BOOKEND_ICON).toBe(BedDouble) }) }) + +describe('the hotel a booked night stands for', () => { + it('FE-BOOKEND-006: is read off the trip place as it is now, and the stay row stands in only where there is none', () => { + const moved = { id: 1, place_id: 10, place_name: 'Old name', place_lat: 47.2, place_lng: 11.4 } + const unknown = { id: 2, place_id: 20, place_name: 'Elsewhere', place_lat: 48.1, place_lng: 11.6 } + const placeless = { id: 3, place_id: null, place_name: null, place_lat: null, place_lng: null } + const places = [ + { id: 10, name: 'Hotel Alpenblick', lat: 47.3, lng: 11.5 }, + { id: 30, name: 'Unrelated', lat: 1, lng: 2 }, + ] + + const read = staysAtTheirPlaces([moved, unknown, placeless], places) + expect(read[0]).toEqual({ id: 1, place_id: 10, place_name: 'Hotel Alpenblick', place_lat: 47.3, place_lng: 11.5 }) + expect(read[1]).toBe(unknown) + expect(read[2]).toBe(placeless) + + // A place that lost its pin takes the bookend with it, as the server's join does. + expect(staysAtTheirPlaces([moved], [{ id: 10, name: 'Hotel Alpenblick', lat: null, lng: null }])[0]) + .toMatchObject({ place_lat: null, place_lng: null }) + + // Nothing to change hands the list back as it came, so nothing downstream recomputes. + const current = [{ ...moved, place_name: 'Hotel Alpenblick', place_lat: 47.3, place_lng: 11.5 }] + expect(staysAtTheirPlaces(current, places)).toBe(current) + expect(staysAtTheirPlaces(current, [])).toBe(current) + }) +}) diff --git a/client/src/components/Roadtrip/nightBookend.ts b/client/src/components/Roadtrip/nightBookend.ts index b08e2a4a8..37cc2c604 100644 --- a/client/src/components/Roadtrip/nightBookend.ts +++ b/client/src/components/Roadtrip/nightBookend.ts @@ -10,7 +10,7 @@ import type { BookendReading } from './roadtripRowModel' * One module for both shells, the way `carrierRide` is for a ride: the two would otherwise * carry the same sentence and the same decision each, and the duplication budget does not * stretch to that. What the row IS comes from the row model (`bookendReading`); this only - * says it. + * says it, and reads the hotel it stands for off the trip's places (`staysAtTheirPlaces`). */ /** The bed the day plan's stay chips wear, so the hotel looks like itself here too. */ @@ -29,6 +29,49 @@ export function bookendMeta(reading: BookendReading, t: TranslationFn, is12h: bo return reading.until ? t('roadtrip.stay.until', { time: formatClockTime(reading.until, is12h) }) : null } +/** The fields of a trip place the hotel of a stay is read from. */ +export interface HotelPlace { + id: number + name: string + lat?: number | null + lng?: number | null +} + +/** The fields of a stay row that name its hotel and where it stands. */ +export interface StayAtPlace { + place_id?: number | null + place_name?: string | null + place_lat?: number | null + place_lng?: number | null +} + +/** + * The stays with their hotel where the trip's places have it now. + * + * A stay row carries its place's name and position as they were when the stays were last + * fetched, and the planner fetches them again on a stay's own edit only. A pin moved on the + * hotel reaches its visits at once (`mergeAssignmentPlace`) and the stay row not at all, so a + * bookend seated from the row stood at the old spot beside its own stop at the new one: the + * day drove from the one to the other, and the server, which joins the place afresh, planned + * another drive. The place wins; the row stands in only for a place the list does not hold. + * Rows that need nothing come back as they were, and so does the list when none does. + */ +export function staysAtTheirPlaces(stays: readonly S[], places: readonly HotelPlace[]): readonly S[] { + if (!stays.length || !places.length) return stays + const byId = new Map(places.map(place => [place.id, place])) + let changed = false + const out = stays.map(stay => { + const place = stay.place_id == null ? undefined : byId.get(stay.place_id) + if (!place) return stay + const lat = place.lat ?? null + const lng = place.lng ?? null + if (stay.place_lat === lat && stay.place_lng === lng && stay.place_name === place.name) return stay + changed = true + return { ...stay, place_lat: lat, place_lng: lng, place_name: place.name } + }) + return changed ? out : stays +} + /** * The booking a tap on the row opens, or null when it opens the stay's place instead. * diff --git a/client/src/components/Roadtrip/useRoadtripRoutes.test.ts b/client/src/components/Roadtrip/useRoadtripRoutes.test.ts index 24268ca02..9a4768684 100644 --- a/client/src/components/Roadtrip/useRoadtripRoutes.test.ts +++ b/client/src/components/Roadtrip/useRoadtripRoutes.test.ts @@ -25,6 +25,7 @@ vi.mock('../Map/RouteCalculator', async importOriginal => ({ import { useRoadtripRoutes } from './useRoadtripRoutes' import { DEFAULT_SETTINGS, useSettingsStore } from '../../store/settingsStore' +import { useTripStore } from '../../store/tripStore' import { lineMetres } from './corridor' const HAMBURG: [number, number] = [53.5511, 9.9937] @@ -1165,7 +1166,7 @@ describe('a leg asked for again on demand', () => { }) /** - * FE-ROADTRIP-ROUTES-052..059: a booked night at both ends of the days around it. + * FE-ROADTRIP-ROUTES-052..061: a booked night at both ends of the days around it. * * The first cases here that hand the hook the trip's stays. What is pinned is what goes to * the router: the drive from the hotel slept in and to tonight's is part of the day's own @@ -1209,7 +1210,10 @@ describe('a booked night at both ends of its days', () => { calculateRouteWithLegs.mockImplementation(async (points: { lat: number; lng: number }[]) => hourly(points)) switchTo(true) }) - afterEach(() => act(() => useSettingsStore.setState({ settings: { ...DEFAULT_SETTINGS } }))) + afterEach(() => act(() => { + useSettingsStore.setState({ settings: { ...DEFAULT_SETTINGS } }) + useTripStore.setState({ places: [] }) + })) /** A check-in with two places, then a transfer day that is only the next check-in. */ const cam = () => ({ @@ -1356,4 +1360,48 @@ describe('a booked night at both ends of its days', () => { await waitFor(() => expect(result.current.days[1].stops[0].bookend?.reservationId).toBe(90)) expect(calculateRouteWithLegs).toHaveBeenCalledTimes(calls) }) + + it('FE-ROADTRIP-ROUTES-060: a hotel whose pin was moved stands where its place is now, not where the stay row last saw it', async () => { + // The stay rows are fetched on a stay's own edit only, so after the pin moved they still + // carry the old spot while the visit is already at the new one. Seated from the row, + // the check-in day drove from the hotel's stop to its old spot and every morning set + // off from there, while the server, which joins the place afresh, planned neither. + const MOVED: [number, number] = [-33.8, 150.2] + const days = [day(1, 1), day(2, 2), day(3, 3)] + const assignments = { + ...map(1, [{ id: 1, at: MOVED }]), + ...map(2, [{ id: 2, at: LOOKOUT }]), + } as AssignmentsMap + const stays = [stayOf(1, 1, 1, 3, GETAWAY)] + act(() => { + useTripStore.setState({ places: [{ id: 10, trip_id: 7, name: 'Getaway Motel', lat: MOVED[0], lng: MOVED[1] }] as never }) + }) + const { result } = renderHook(() => useRoadtripRoutes(7, days, assignments, 'driving', {}, [], stays)) + await waitFor(() => expect(result.current.loading).toBe(false)) + + // The check-in day is its hotel alone, with no drive to a second copy of it. + expect(result.current.days.map(d => shape(d.stops))).toEqual([ + ['Stop 1'], + ['morning:1', 'Stop 2', 'evening:1'], + ]) + expect(result.current.days[1].stops[0]).toMatchObject({ lat: MOVED[0], lng: MOVED[1], name: 'Getaway Motel' }) + expect(asked()).toEqual([[MOVED[0], LOOKOUT[0], MOVED[0]]]) + }) + + it('FE-ROADTRIP-ROUTES-061: the drive out of the morning hotel goes the way the first place is reached from it', async () => { + // The mode the day plan draws that leg in, and the one the server routes it in. + const { days, stays } = simeon() + const assignments = { + ...map(1, [{ id: 10, at: HOTEL }, { id: 11, at: P[0] }, { id: 12, at: P[1] }]), + ...map(2, [{ id: 13, at: P[2], incoming: 'walking' }, { id: 14, at: P[3] }]), + ...map(3, [{ id: 15, at: P[4] }, { id: 16, at: P[5] }]), + } as AssignmentsMap + const { result } = renderHook(() => useRoadtripRoutes(7, days, assignments, 'driving', {}, [], stays)) + await waitFor(() => expect(result.current.loading).toBe(false)) + + const runs = calculateRouteWithLegs.mock.calls.map(c => [c[1].profile, c[0].map((p: { lat: number }) => p.lat)]) + expect(runs).toContainEqual(['walking', [HOTEL[0], P[2][0]]]) + expect(runs).toContainEqual(['driving', [P[2][0], P[3][0], HOTEL[0]]]) + expect(runs).toContainEqual(['driving', [HOTEL[0], P[4][0], P[5][0], HOTEL[0]]]) + }) }) diff --git a/client/src/components/Roadtrip/useRoadtripRoutes.ts b/client/src/components/Roadtrip/useRoadtripRoutes.ts index de00aeb8f..fe1ff1527 100644 --- a/client/src/components/Roadtrip/useRoadtripRoutes.ts +++ b/client/src/components/Roadtrip/useRoadtripRoutes.ts @@ -6,12 +6,14 @@ import { resolveLegMode } from '../Planner/legMode' import { splitIntoRuns, parseAvoid, type DriveLimits } from './roadtripModel' import { spillChains } from './nightSpill' import { useSettingsStore } from '../../store/settingsStore' +import { useTripStore } from '../../store/tripStore' import { useVehicleRange } from './useVehicleRange' import type { Assignment, AssignmentsMap, Accommodation, Day, Reservation, RouteAvoidClass, SnappedWaypoint } from '../../types' import type { RoadtripVia, RoadtripDayBoundary } from '@trek/shared' import { dayWindow } from './dayWindow' import { useTranslation } from '../../i18n/TranslationContext' import { stayStartingOn } from '../../utils/dayMerge' +import { staysAtTheirPlaces } from './nightBookend' export type { RoadtripStop, RoadtripDay, RoadtripRoutes, QuietDay, AccessSpur, RoutedLeg, SnappedPoint } from '@trek/shared/roadtrip' @@ -333,11 +335,15 @@ export function useRoadtripRoutes( * Off until the trip switches it on, so a trip drives exactly as it did before. */ const bookendsOn = useRoadtripSettings(hotelBookendsOn, tripId) - // The nights as the rule reads them, with the booking behind each. Read only while the - // switch is on, so a new booking on a trip without it touches nothing downstream. + // Where each hotel stands now. The stay rows keep the position their place had when they + // were fetched, while the visits follow the place at once, and the server joins it afresh. + const tripPlaces = useTripStore(s => s.places) + // The nights as the rule reads them, with the booking behind each and the hotel where its + // place is. Read only while the switch is on, so a new booking or a moved pin on a trip + // without it touches nothing downstream. const stays = useMemo( - () => (bookendsOn ? bookendStaysOf(accommodations, reservations) : []), - [bookendsOn, accommodations, reservations], + () => (bookendsOn ? bookendStaysOf(staysAtTheirPlaces(accommodations, tripPlaces), reservations) : []), + [bookendsOn, accommodations, tripPlaces, reservations], ) // The stored days with the night before and the night after seated at their edges, by // the rule the server's calculate_roadtrip runs too. Before a day is asked whether it diff --git a/client/src/components/Settings/ToggleSwitch.tsx b/client/src/components/Settings/ToggleSwitch.tsx index 1c339580a..5d1e7a6bb 100644 --- a/client/src/components/Settings/ToggleSwitch.tsx +++ b/client/src/components/Settings/ToggleSwitch.tsx @@ -1,8 +1,12 @@ import React from 'react' -export default function ToggleSwitch({ on, onToggle, label }: { on: boolean; onToggle: () => void; label?: string }) { +/** + * `describedBy` names the element that explains the switch, for a switch whose label alone + * does not say what flipping it changes. + */ +export default function ToggleSwitch({ on, onToggle, label, describedBy }: { on: boolean; onToggle: () => void; label?: string; describedBy?: string }) { return ( -