fix(roadtrip): a stop added by hand opens on the kind being looked for

The button carried nothing and the form hard-wired the kind, so the categories
sitting directly above it had no way through and every manual stop started life
as a fuel stop with ten minutes on it. The click now carries what the corridor
is looking for, dwell included. The picker is multi-select, so the first kind
the panel lists wins rather than a fallback nobody switched on; with nothing
selected the form falls back on its own, because a stop without a kind is a
numbered destination that counts in every total.

This also closes the case it was noticed on: an electric car seeds the panel to
charging, so the picker said Charging over a form that said Fuel.
This commit is contained in:
Maurice
2026-09-18 13:17:59 +02:00
parent e9742e9322
commit 5ef8bf31f4
8 changed files with 119 additions and 13 deletions
@@ -1772,6 +1772,16 @@ describe('PlaceFormModal as a road trip service stop', () => {
expect(screen.getAllByText('Add as a stop').length).toBeGreaterThan(0);
});
it('FE-PLANNER-PLACEFORM-085: it opens on the kind the corridor was looking for, dwell and all', () => {
// The panel seeds itself to charging on an electric car, and a form opening on a
// fuel pump under a picker that says Charging is the fault this carries the answer to.
render(<PlaceFormModal {...defaultProps} serviceStop={serviceStop({ defaultKind: 'charging' })} />);
expect(screen.getByRole('button', { name: 'Charging' })).toHaveAttribute('aria-pressed', 'true');
expect(screen.getByRole('button', { name: /30 min/ })).toHaveAttribute('aria-pressed', 'true');
expect(screen.getByRole('button', { name: 'Fuel' })).toHaveAttribute('aria-pressed', 'false');
});
it('FE-PLANNER-PLACEFORM-076: picking a kind also picks how long that kind takes', async () => {
const onSave = vi.fn().mockResolvedValue({ id: 9 });
render(<PlaceFormModal {...defaultProps} onSave={onSave} serviceStop={serviceStop()} />);
@@ -254,10 +254,15 @@ function usePlaceFormModal(props: PlaceFormModalProps) {
// path exists to avoid. Read out of the closure rather than watched, because the
// mode is fixed for the life of one opening and a rebuilt drive must not reset a
// half-filled form.
// What the corridor panel was looking for when the button was pressed, because
// that is the traveller's own answer to what they are adding; only a panel with
// nothing selected falls back to the constant. The dwell follows the kind, the
// same way picking one by hand moves it.
const kind = serviceStop.defaultKind ?? DEFAULT_SERVICE_KIND
setForm({
...DEFAULT_FORM,
stop_type: DEFAULT_SERVICE_KIND,
duration_minutes: STOP_KIND_BY_KEY[DEFAULT_SERVICE_KIND].defaultMinutes,
stop_type: kind,
duration_minutes: STOP_KIND_BY_KEY[kind].defaultMinutes,
})
} else {
setForm(DEFAULT_FORM)
@@ -441,6 +441,35 @@ describe('RoadtripCorridorPanel', () => {
// nothing is added until a place has been found and saved in there.
fireEvent.click(manual)
expect(onAddManual).toHaveBeenCalledTimes(1)
// The kind, never the MouseEvent: the categories above the button are the answer to
// what is being added, and a form that ignored them opened every manual stop on fuel.
expect(onAddManual).toHaveBeenCalledWith('fuel')
})
it('FE-ROADTRIP-PANEL-035: the manual add opens on what the panel is looking for', () => {
const open = (categories: string[]): unknown => {
const onAddManual = vi.fn()
const { unmount } = wrap(
<RoadtripCorridorPanel
corridor={corridor({ categories })}
routes={routes([day(1, 1)])}
onAddPoi={vi.fn()}
onAddManual={onAddManual}
/>,
)
fireEvent.click(searchRow()[1])
unmount()
return onAddManual.mock.calls[0][0]
}
// An electric car seeds the panel to charging, and this is the case the fault was
// reported on: the panel said Charging and the form opened on Fuel.
expect(open(['charging'])).toBe('charging')
expect(open(['restaurant'])).toBe('restaurant')
// Multi-select: the first the panel LISTS wins, rather than a kind nobody switched on.
expect(open(['sights', 'rest_area'])).toBe('rest_area')
// Nothing selected is no answer, and the form falls back on its own.
expect(open([])).toBeNull()
})
it('FE-ROADTRIP-PANEL-026: with no drive on the trip there is nowhere to add one by hand', () => {
@@ -13,9 +13,10 @@ import CustomSelect from '../shared/CustomSelect'
import RoadtripCategoryPicker from './RoadtripCategoryPicker'
import { serviceColor } from './roadtripModel'
import { alongLabel, offRouteLabel } from './corridorSearchModel'
import { CORRIDOR_CATEGORY_BY_KEY } from './stopKinds'
import { CORRIDOR_CATEGORY_BY_KEY, manualStopKindFor } from './stopKinds'
import { FS } from './typeScale'
import { CORRIDOR_CATEGORY_KEYS, CORRIDOR_SECTION_KM, CORRIDOR_WIDTHS_KM, type RoadtripCorridor } from './useRoadtripCorridor'
import type { RoadtripStopType } from '@trek/shared'
import type { CorridorPoi } from './useCorridorPois'
import type { RoadtripRoutes } from './useRoadtripRoutes'
@@ -38,8 +39,12 @@ interface RoadtripCorridorPanelProps {
/**
* Opens the place form for a stop the search never found. Given only to somebody who
* may add places, which is what keeps the button off a reader's panel.
*
* Carries the kind the panel is looking for, so the form opens on it rather than on a
* constant: the categories above the button are the traveller's own answer to "what am
* I adding", and ignoring them made every manual stop start life as a fuel stop.
*/
onAddManual?: () => void
onAddManual?: (kind: RoadtripStopType | null) => void
}
/**
@@ -476,7 +481,9 @@ export default function RoadtripCorridorPanel({
{onAddPoi && onAddManual && corridor.day ? (
<button
type="button"
onClick={onAddManual}
// Wrapped rather than handed the prop directly: `onClick` would otherwise
// pass the MouseEvent as the kind.
onClick={() => onAddManual(manualStopKindFor(corridor.categories))}
// The full sentence is the accessible name and the tooltip; the face of
// the button carries the one word that fits beside "Search".
aria-label={t('roadtrip.poi.addManual')}
+13 -5
View File
@@ -36,6 +36,13 @@ export interface ServiceStopDay {
/** Everything the form needs to ask where a service stop belongs, and nothing else. */
export interface ServiceStopMode {
/**
* The kind the form opens on, from what the corridor panel is looking for.
*
* Absent or null when the panel had nothing selected, and then `DEFAULT_SERVICE_KIND`
* stands in. Optional so a caller with no panel behind it stays unchanged.
*/
defaultKind?: RoadtripStopType | null
days: ServiceStopDay[]
/** Where a stop goes at the end of the day the panel is on, routed or not. */
appendDay: { dayId: number; dayNumber: number; position: number } | null
@@ -99,12 +106,13 @@ export interface ServiceStopChoice {
}
/**
* The kind the form opens on.
* The kind the form opens on when nothing better is known.
*
* The panel it is reached from is headed by pumps and chargers, and one click is the
* whole distance to any of the others. Opening on nothing would be worse than a guess: a
* stop left without a kind is a numbered destination that counts in every total, which
* is the very thing this path exists to avoid.
* Nothing better means the panel had no category switched on at all; normally the form
* opens on what the corridor is looking for (`manualStopKindFor`), because that is the
* traveller's own answer rather than a guess. Opening on nothing would be worse than
* either: a stop left without a kind is a numbered destination that counts in every
* total, which is the very thing this path exists to avoid.
*/
export const DEFAULT_SERVICE_KIND: RoadtripStopType = 'fuel'
@@ -3,7 +3,7 @@ import { roadtripStopTypeSchema } from '@trek/shared'
import { SERVICE_STOP_TYPES, SERVICE_COLORS } from './roadtripModel'
import {
STOP_KINDS, STOP_KIND_BY_KEY, CORRIDOR_CATEGORIES, CORRIDOR_CATEGORY_BY_KEY, CORRIDOR_CATEGORY_KEYS,
SERVICE_KIND_KEYS, REFUELLING_STOP_TYPES, isOvernightCategory,
SERVICE_KIND_KEYS, REFUELLING_STOP_TYPES, isOvernightCategory, manualStopKindFor,
} from './stopKinds'
/**
@@ -77,4 +77,19 @@ describe('stop kinds', () => {
expect(Number.isInteger(kind.defaultMinutes)).toBe(true)
}
})
it('FE-STOPKIND-008: the manual add opens on what the corridor is looking for', () => {
// A stop added by hand used to open on fuel whatever the panel above it said, which
// on an electric car meant a picker reading Charging over a form reading Fuel.
for (const category of CORRIDOR_CATEGORIES) {
expect(manualStopKindFor([category.key])).toBe(category.stopKind)
}
// Multi-select, so the first the panel LISTS wins: falling back would open on a kind
// that is not among the ones switched on, which is a worse answer than any of them.
const [first, second] = CORRIDOR_CATEGORY_KEYS
expect(manualStopKindFor([second, first])).toBe(CORRIDOR_CATEGORY_BY_KEY[first].stopKind)
// Nothing selected is no answer at all, and the caller decides what to do with that.
expect(manualStopKindFor([])).toBeNull()
expect(manualStopKindFor(['not-a-category'])).toBeNull()
})
})
@@ -86,6 +86,26 @@ export const CORRIDOR_CATEGORY_BY_KEY: Record<string, CorridorCategory> = Object
/** The keys the corridor search offers, in the order the panel shows them. */
export const CORRIDOR_CATEGORY_KEYS = CORRIDOR_CATEGORIES.map(c => c.key)
/**
* The kind a stop added by hand opens on, read off what the corridor is looking for.
*
* The categories are what the traveller has just told the search to find, so they are
* also the best reading of what they are about to add themselves, and on an electric
* car the panel seeds itself to charging, which is precisely the case where a form
* opening on a fuel pump read as a fault.
*
* The picker is multi-select, so the first category the panel LISTS wins. Falling back
* instead would open on a kind that is not among the ones switched on, which is a worse
* answer than any of them. Null only when nothing is selected at all; the caller decides
* what that means, because a form must never open on no kind (see `DEFAULT_SERVICE_KIND`).
*/
export function manualStopKindFor(categories: readonly string[]): RoadtripStopType | null {
for (const category of CORRIDOR_CATEGORIES) {
if (categories.includes(category.key) && category.stopKind) return category.stopKind
}
return null
}
/** Categories whose hit becomes a night rather than a pause. */
export function isOvernightCategory(category: string | null | undefined): boolean {
// Campsite is both: it is a service stop today and stays one, but a night can be
+14 -2
View File
@@ -315,6 +315,14 @@ export function useTripPlanner() {
* form they have always been.
*/
const [serviceStopForm, setServiceStopForm] = useState(false)
/**
* The kind that form opens on, taken from what the corridor panel was looking for.
*
* Beside the flag rather than inside it, because it is written by the same click and
* read by the same memo, and a second piece of state is cheaper to follow than a flag
* that is sometimes a boolean and sometimes an object.
*/
const [serviceStopKind, setServiceStopKind] = useState<RoadtripStopType | null>(null)
/**
* The corridor hit waiting to become a stop, while the small popup is open.
*
@@ -1628,7 +1636,7 @@ export function useTripPlanner() {
* typed-ahead search, which is the whole reason to use it, and where the stop belongs
* is worked out at the save, from what the save carries.
*/
const openManualRoadtripStop = useCallback(() => {
const openManualRoadtripStop = useCallback((kind: RoadtripStopType | null = null) => {
if (!can('place_edit', trip)) return
setEditingPlace(null)
setEditingAssignmentId(null)
@@ -1637,6 +1645,9 @@ export function useTripPlanner() {
// form opens; this one cannot, because nothing has been chosen yet.
setPlaceFormDayId(null)
setPlaceFormPosition(null)
// Set in the same batch as the flag below, so the kind is already there when the
// form's opening effect reads the mode out of its closure.
setServiceStopKind(kind)
setServiceStopForm(true)
setShowPlaceForm(true)
}, [can, trip])
@@ -1653,6 +1664,7 @@ export function useTripPlanner() {
if (!serviceStopForm) return null
const panelDay = roadtripCorridor.day
return {
defaultKind: serviceStopKind,
// A day that has not routed has no order to place anything in.
days: roadtripRoutes.days
.filter(day => day.geometry.length > 1)
@@ -1680,7 +1692,7 @@ export function useTripPlanner() {
: null,
targetFor: manualStopTargetFor,
}
}, [serviceStopForm, roadtripRoutes.days, roadtripCorridor.day, manualStopTargetFor])
}, [serviceStopForm, serviceStopKind, roadtripRoutes.days, roadtripCorridor.day, manualStopTargetFor])
const addRoadtripVia = useCallback(async (lat: number, lng: number) => {
if (!can('day_edit', trip)) return