From ea9de65550eff51e7fe386e0bce5bca27115add7 Mon Sep 17 00:00:00 2001 From: Maurice Date: Fri, 4 Sep 2026 22:29:47 +0200 Subject: [PATCH] feat(places): make the kind of stop editable, and enforce it on the way in MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three holes, one seam. Leaving the corridor popup by "more details" dropped the kind and the dwell it had just worked out, so a fuel stop silently became a numbered destination that counted in every total, in the badge and in the PDF. Once set, the kind could never be changed back through the web UI at all. And the REST route did not validate the field: both request schemas are open objects, so stop_type: 'banana' was written straight to the column, where isServiceStopType read it as false and the stop counted as a destination again. The form now carries the field, so the popup's answer survives the trip through it, an ordinary place can become a service stop, and a wrong kind can be taken off. Editing keeps stop_type and deliberately leaves duration_minutes out: how long a stay takes belongs to the rail's own dialog, and sending it from here would overwrite what was set there. The schemas gain the one field rather than closing: roughly 190 call sites post whatever their form holds, and a closed object would reject all of them at once. The doc comment claiming the list was already enforced is corrected — it was describing an intention. The two driving findings move into the stop's badge row in warning colours, beside the stay and the walk from the road. All three say what this stop costs, and one row reads as one answer rather than as notes stacked underneath. --- .../Planner/PlaceFormModal.helpers.ts | 10 ++++ .../src/components/Planner/PlaceFormModal.tsx | 51 +++++++++++++++- .../components/Roadtrip/RoadtripSidebar.tsx | 59 ++++++++++--------- .../components/Roadtrip/RoadtripStopPopup.tsx | 5 +- .../src/pages/tripPlanner/useTripPlanner.ts | 17 +++++- shared/src/i18n/ar/roadtrip.ts | 2 + shared/src/i18n/br/roadtrip.ts | 2 + shared/src/i18n/ca/roadtrip.ts | 2 + shared/src/i18n/cs/roadtrip.ts | 2 + shared/src/i18n/de/roadtrip.ts | 2 + shared/src/i18n/en/roadtrip.ts | 2 + shared/src/i18n/es/roadtrip.ts | 2 + shared/src/i18n/fr/roadtrip.ts | 2 + shared/src/i18n/gr/roadtrip.ts | 2 + shared/src/i18n/hu/roadtrip.ts | 2 + shared/src/i18n/id/roadtrip.ts | 2 + shared/src/i18n/it/roadtrip.ts | 2 + shared/src/i18n/ja/roadtrip.ts | 2 + shared/src/i18n/ko/roadtrip.ts | 2 + shared/src/i18n/nl/roadtrip.ts | 2 + shared/src/i18n/pl/roadtrip.ts | 2 + shared/src/i18n/ru/roadtrip.ts | 2 + shared/src/i18n/sv/roadtrip.ts | 2 + shared/src/i18n/tr/roadtrip.ts | 2 + shared/src/i18n/uk/roadtrip.ts | 2 + shared/src/i18n/vi/roadtrip.ts | 2 + shared/src/i18n/zh-TW/roadtrip.ts | 2 + shared/src/i18n/zh/roadtrip.ts | 2 + shared/src/place/place.schema.spec.ts | 25 ++++++++ shared/src/place/place.schema.ts | 32 ++++++++-- 30 files changed, 205 insertions(+), 40 deletions(-) diff --git a/client/src/components/Planner/PlaceFormModal.helpers.ts b/client/src/components/Planner/PlaceFormModal.helpers.ts index dea1f9907..a3ea5c490 100644 --- a/client/src/components/Planner/PlaceFormModal.helpers.ts +++ b/client/src/components/Planner/PlaceFormModal.helpers.ts @@ -1,3 +1,5 @@ +import type { RoadtripStopType } from '@trek/shared' + export interface PlaceFormData { name: string description: string @@ -24,6 +26,14 @@ export interface PlaceFormData { // submit payload when unchanged, and useTripPlanner strips it off the place // update and PUTs it per assignment instead. assignment_notes?: string + // What kind of stop on a drive this is, if any (#1797). Optional and absent from + // DEFAULT_FORM for the same reason as image_url: a missing key writes nothing, and a + // place created outside a road trip has no opinion about fuel stops. + stop_type?: RoadtripStopType | null + // Offered only while creating, and only alongside a stop_type: the corridor popup + // suggests how long that kind of pause usually takes, and the full form should not + // throw the suggestion away on the way through. Editing a stay is the rail's job. + duration_minutes?: number } export function isGoogleMapsUrl(input: string): boolean { diff --git a/client/src/components/Planner/PlaceFormModal.tsx b/client/src/components/Planner/PlaceFormModal.tsx index a83fc3c90..469734c49 100644 --- a/client/src/components/Planner/PlaceFormModal.tsx +++ b/client/src/components/Planner/PlaceFormModal.tsx @@ -1,14 +1,16 @@ import { useState, useEffect, useRef, useMemo, useCallback } from 'react' import Modal from '../shared/Modal' +import type { RoadtripStopType } from '@trek/shared' +import { STOP_KINDS } from '../Roadtrip/stopKinds' import CustomSelect from '../shared/CustomSelect' import NoteFormatToolbar from '../shared/NoteFormatToolbar' import { mapsApi } from '../../api/client' import { recordPlacePick } from '../../api/placeShadow' import { useAuthStore } from '../../store/authStore' +import { useAddonStore } from '../../store/addonStore' import { useCanDo } from '../../store/permissionsStore' import { useTripStore } from '../../store/tripStore' import { useSettingsStore } from '../../store/settingsStore' -import { useAddonStore } from '../../store/addonStore' import CollectionPicker from '../Collections/CollectionPicker' import PlaceDetailsColumn, { type PlaceDetailsSelection } from './PlaceDetailsColumn' import { useToast } from '../shared/Toast' @@ -38,7 +40,7 @@ interface PlaceFormModalProps { onClose: () => void onSave: (data: PlaceSubmitData, files?: File[]) => Promise<{ id: number } | void> | void place: Place | null - prefillCoords?: { lat: number; lng: number; name?: string; address?: string; website?: string; phone?: string; osm_id?: string } | null + prefillCoords?: { lat: number; lng: number; name?: string; address?: string; website?: string; phone?: string; osm_id?: string; stop_type?: RoadtripStopType | null; duration_minutes?: number } | null tripId: number categories: Category[] onCategoryCreated: (category: { name: string; color?: string; icon?: string }) => Promise | undefined @@ -88,6 +90,9 @@ function usePlaceFormModal(props: PlaceFormModalProps) { onCategoryCreated, assignmentId, dayAssignments = [], isMobile = false, onOpenExpense, } = props + // Hidden while the addon is off, because the kinds only mean anything to the road trip + // rail: on an instance without it they would be six labels that change nothing. + const roadtripEnabled = useAddonStore(s => s.isEnabled('roadtrip')) const [form, setForm] = useState(DEFAULT_FORM) const [mapsSearch, setMapsSearch] = useState('') const [mapsResults, setMapsResults] = useState([]) @@ -153,6 +158,11 @@ function usePlaceFormModal(props: PlaceFormModalProps) { notes: place.notes || '', transport_mode: place.transport_mode || 'walking', website: place.website || '', + // Carried through every edit. Without it, opening a fuel stop to fix a typo + // submits an empty kind and turns it back into a numbered destination. + // duration_minutes deliberately stays out: how long a stay takes belongs to the + // rail's own dialog, and sending it from here would overwrite what was set there. + stop_type: place.stop_type ?? null, // The day-specific note rides only with an assignment in context (#2163); // otherwise the key stays absent so submit never sends a notes write. ...(assignment ? { assignment_notes: assignment.notes || '' } : {}), @@ -167,6 +177,8 @@ function usePlaceFormModal(props: PlaceFormModalProps) { website: prefillCoords.website || '', phone: prefillCoords.phone || '', osm_id: prefillCoords.osm_id, + stop_type: prefillCoords.stop_type ?? null, + duration_minutes: prefillCoords.duration_minutes, }) } else { setForm(DEFAULT_FORM) @@ -500,6 +512,15 @@ function usePlaceFormModal(props: PlaceFormModalProps) { lat: form.lat ? Number.parseFloat(form.lat) : null, lng: form.lng ? Number.parseFloat(form.lng) : null, category_id: form.category_id || null, + // An explicit null is how a stop stops being a fuel stop; the service reads it + // that way rather than as "leave alone", which is what a missing key means. + stop_type: form.stop_type || null, + // Only on the way in, and only with a kind: it is the popup's suggestion for how + // long that kind of pause takes. On an edit it is left out entirely, because the + // stay belongs to the rail's dialog and sending it here would overwrite it. + ...(!place && form.stop_type && form.duration_minutes + ? { duration_minutes: form.duration_minutes } + : {}), _pendingFiles: pendingFiles.length > 0 ? pendingFiles : undefined, } // #2163: the per-assignment note only travels when an assignment is in @@ -548,6 +569,7 @@ function usePlaceFormModal(props: PlaceFormModalProps) { dayAssignments, isMobile, collectionsEnabled, + roadtripEnabled, form, setForm, mapsSearch, @@ -620,6 +642,7 @@ export default function PlaceFormModal(props: PlaceFormModalProps) { dayAssignments, isMobile, collectionsEnabled, + roadtripEnabled, form, setForm, mapsSearch, @@ -891,6 +914,30 @@ export default function PlaceFormModal(props: PlaceFormModalProps) { + {/* What kind of stop this is on a drive. + Under the category rather than beside it, because the two answer different + questions: a category is the traveller's own label, editable and shared across + the whole instance, while this is a fact about the place. Refuelling is not a + taste. Shown always, not only in road trip mode: a fuel stop added on a Tuesday + is still a fuel stop when the mode is switched on later, and this is the only + place in the web UI where the kind can be taken off again. */} + {roadtripEnabled ? ( +
+ + handleChange('stop_type', String(value) as RoadtripStopType | '')} + placeholder={t('roadtrip.stop.none')} + options={[ + { value: '', label: t('roadtrip.stop.none') }, + ...STOP_KINDS.map(k => ({ value: k.key, label: t(k.labelKey) })), + ]} + size="sm" + /> +

{t('roadtrip.stop.kindHelp')}

+
+ ) : null} + {/* Category */}
diff --git a/client/src/components/Roadtrip/RoadtripSidebar.tsx b/client/src/components/Roadtrip/RoadtripSidebar.tsx index 9b62228ce..a8e3865bd 100644 --- a/client/src/components/Roadtrip/RoadtripSidebar.tsx +++ b/client/src/components/Roadtrip/RoadtripSidebar.tsx @@ -1,7 +1,7 @@ import React from 'react' import { MapPin, CarFront, Footprints, Bike, Zap, AlertTriangle, Moon, - ParkingSquare, Shuffle, + ParkingSquare, Shuffle, Fuel, Clock, type LucideIcon, } from 'lucide-react' import { useTranslation } from '../../i18n/TranslationContext' @@ -454,15 +454,19 @@ function RouteViaStop({ via }: { via: RouteVia }): React.ReactElement { } /** - * A finding about the drive rather than about a stop: too long at the wheel in one go, or - * the tank running out before anywhere to fill it. + * A finding about the drive leaving this stop: too long at the wheel in one go, or the + * tank running out before anywhere to fill it. * - * Sits in the chain between the two stops the drive runs between, because that is where - * it happens. Neither finding names a time of day: with no stop pinned to a clock the - * cascade produces no times at all, so both are durations and distances, which exist as - * soon as the leg has routed. + * Wears the same two-part shell as the stay and the walk, in warning colours, and sits in + * the same row: all of them say what this stop costs, and one row of badges reads as one + * answer rather than as notes stacked under each other. The index is the stop the leg + * LEAVES, so the badge means "the drive from here". + * + * Neither finding names a time of day. With no stop pinned to a clock the cascade + * produces no times at all, so both are durations and distances, which exist as soon as + * the leg has routed. */ -function DriveFinding({ warning }: { warning: ScheduleWarning }): React.ReactElement | null { +function DriveFindingBadge({ warning }: { warning: ScheduleWarning }): React.ReactElement | null { const { t } = useTranslation() const distanceUnit = useSettingsStore(s => s.settings.distance_unit) const text = warning.code === 'leg' @@ -471,21 +475,24 @@ function DriveFinding({ warning }: { warning: ScheduleWarning }): React.ReactEle ? t('roadtrip.limit.range', { distance: formatDistance(warning.sinceKm ?? 0, distanceUnit) }) : null if (!text) return null + const Icon = warning.code === 'range' ? Fuel : Clock return ( -
- - + + + - - - - {text} - + + {warning.code === 'range' + ? formatDistance(warning.sinceKm ?? 0, distanceUnit) + : `+${formatDurationShort((warning.overMinutes ?? 0) * 60)}`} -
+ ) } @@ -556,12 +563,14 @@ function Arrival({ entry }: { entry: ScheduleEntry }): React.ReactElement { ) } -function Stop({ stop, number, entry, late, selected, continues, onSelect, onMove, canMove, onEditStay }: { +function Stop({ stop, number, entry, late, driveFindings, selected, continues, onSelect, onMove, canMove, onEditStay }: { stop: RoadtripStop /** Position within the day — the same count the map badges its markers with. */ number: number entry: ScheduleEntry | undefined late: ScheduleWarning | undefined + /** Findings about the drive LEAVING this stop, when the limits are set and it goes over. */ + driveFindings?: ScheduleWarning[] selected: boolean /** Whether the chain goes on below, so the marker keeps hold of the line. */ continues: boolean @@ -629,6 +638,7 @@ function Stop({ stop, number, entry, late, selected, continues, onSelect, onMove {spurWorthLabelling(stop.offRoadMeters) ? : null} + {(driveFindings ?? []).map(w => )} {lateText ? ( w.code === 'late')} + driveFindings={day.driveWarnings.filter(w => w.index === i)} selected={selectedAssignmentId === stop.assignmentId} continues={i < last} onSelect={onSelectStop ? () => onSelectStop(stop.placeId, stop.assignmentId) : undefined} @@ -799,12 +810,6 @@ function DaySection({ day, selectedAssignmentId, onSelectStop, onReorderStop, on alternativesOpen={openAlternatives?.dayId === day.dayId && openAlternatives.index === i} /> ) : null} - {/* Between the two stops, which is where "somewhere on this drive" actually - is. The range finding is the only one with its remedy right beside it: - fuel and charging are the corridor search's first two categories. */} - {i < last ? day.driveWarnings.filter(w => w.index === i).map(w => ( - - )) : null} {/* After the band, because a plugin halt happens on the drive it describes rather than before setting off. */} {i < last ? (day.legVias[i] ?? []).map((via, vi) => ( diff --git a/client/src/components/Roadtrip/RoadtripStopPopup.tsx b/client/src/components/Roadtrip/RoadtripStopPopup.tsx index cf80ebd27..92c96ace3 100644 --- a/client/src/components/Roadtrip/RoadtripStopPopup.tsx +++ b/client/src/components/Roadtrip/RoadtripStopPopup.tsx @@ -39,7 +39,8 @@ interface RoadtripStopPopupProps { onClose: () => void onSave: (input: { stopType: RoadtripStopType | null; dwellMinutes: number }) => Promise | void /** Opens the full place form instead, carrying what has been filled in so far. */ - onMoreDetails: () => void + /** Hands over what has been picked here, so the full form opens on the same answer. */ + onMoreDetails: (stop: { stopType: RoadtripStopType | null; dwellMinutes: number }) => void } /** @@ -161,7 +162,7 @@ export default function RoadtripStopPopup({