fix(planner): trip places in every manual location field, never at 0,0

The trip's places went into the field's search results as they were. A place
without a pin was picked as 0,0, because Number(null) is a finite 0, and a
highlight left over from a longer search could point past the shorter list
and throw on Enter. The phone's form, the train stations and the car stops
were not offered the places at all.

The places are now prepared once per form (named, with a real coordinate,
each once) and handed to every location field of the manual tab, on the
desktop and on the phone. The field shows them while it is empty or holds
fewer than three characters, narrowed by what was typed, and derives that
list rather than writing it into the search results, so a late search answer
cannot replace it and a place already picked does not reopen it.
This commit is contained in:
Maurice
2026-09-23 13:41:31 +02:00
parent cc5957dc6a
commit 3a67a8e279
8 changed files with 234 additions and 24 deletions
@@ -1,4 +1,4 @@
// FE-PLANNER-LOCSEL-001 to FE-PLANNER-LOCSEL-019
// FE-PLANNER-LOCSEL-001 to FE-PLANNER-LOCSEL-024
import { useState } from 'react';
import { delay, http, HttpResponse } from 'msw';
import userEvent from '@testing-library/user-event';
@@ -29,11 +29,15 @@ function searchRoute(places: SearchHit[] | (() => Response | Promise<Response>))
// Controlled host — `value` drives the input text and mutes the search for the
// already-picked name, exactly like ReservationModal wires it up.
function Host({ initial = null, onPick }: { initial?: LocationPoint | null; onPick?: (l: LocationPoint | null) => void }) {
function Host({ initial = null, onPick, places }: { initial?: LocationPoint | null; onPick?: (l: LocationPoint | null) => void; places?: LocationPoint[] }) {
const [value, setValue] = useState<LocationPoint | null>(initial);
return <LocationSelect value={value} onChange={(l) => { setValue(l); onPick?.(l); }} />;
return <LocationSelect value={value} onChange={(l) => { setValue(l); onPick?.(l); }} places={places} />;
}
/** The trip's own places, as the transport forms hand them over (#2468). */
const HOTEL = { name: 'Bach Suites Saigon', lat: 10.78, lng: 106.7, address: 'District 1' };
const EXPO = { name: 'Textile Exhibition', lat: 10.8, lng: 106.66, address: null };
describe('LocationSelect', () => {
it('FE-PLANNER-LOCSEL-001: falls back to the translated placeholder', () => {
render(<LocationSelect value={null} onChange={vi.fn()} />);
@@ -263,4 +267,77 @@ describe('LocationSelect', () => {
await waitFor(() => expect(screen.queryByText('Gare du Nord')).not.toBeInTheDocument());
});
it('FE-PLANNER-LOCSEL-020: an empty field offers the trip places, and picking one needs no search', async () => {
const user = userEvent.setup();
let calls = 0;
server.use(searchRoute(() => { calls++; return HttpResponse.json({ places: [GARE] }); }));
const onPick = vi.fn();
render(<Host places={[HOTEL, EXPO]} onPick={onPick} />);
await user.click(screen.getByRole('textbox'));
expect(screen.getByText('Bach Suites Saigon')).toBeInTheDocument();
expect(screen.getByText('Textile Exhibition')).toBeInTheDocument();
await user.click(screen.getByText('Textile Exhibition'));
expect(onPick).toHaveBeenLastCalledWith({ name: 'Textile Exhibition', lat: 10.8, lng: 106.66, address: null });
await settle();
expect(calls).toBe(0);
});
it('FE-PLANNER-LOCSEL-021: one or two typed characters narrow the trip places by name', async () => {
const user = userEvent.setup();
render(<Host places={[HOTEL, EXPO]} />);
await user.type(screen.getByRole('textbox'), 'te');
expect(screen.getByText('Textile Exhibition')).toBeInTheDocument();
// "Bach Suites" has a "te" inside "Suites", so it stays; a name without it goes.
expect(screen.getByText('Bach Suites Saigon')).toBeInTheDocument();
await user.type(screen.getByRole('textbox'), '{backspace}{backspace}ex');
expect(screen.getByText('Textile Exhibition')).toBeInTheDocument();
expect(screen.queryByText('Bach Suites Saigon')).not.toBeInTheDocument();
});
it('FE-PLANNER-LOCSEL-022: from the third character on the map search answers instead of the trip places', async () => {
const user = userEvent.setup();
server.use(searchRoute([GARE]));
render(<Host places={[HOTEL, EXPO]} />);
await user.type(screen.getByRole('textbox'), 'Gare');
expect(await screen.findByText('Gare du Nord')).toBeInTheDocument();
expect(screen.queryByText('Bach Suites Saigon')).not.toBeInTheDocument();
});
it('FE-PLANNER-LOCSEL-023: a place already picked does not reopen the trip places when the field is focused', async () => {
const user = userEvent.setup();
render(<Host places={[HOTEL, EXPO]} initial={{ name: 'Ab', lat: 1, lng: 2 }} />);
await user.click(screen.getByRole('textbox'));
expect(screen.queryByText('Bach Suites Saigon')).not.toBeInTheDocument();
});
it('FE-PLANNER-LOCSEL-024: shortening the query after moving down the search results picks nothing stale', async () => {
const user = userEvent.setup();
server.use(searchRoute([GARE, LYON, { ...GARE, name: 'Gare de l Est', osm_id: 'n3' }]));
const onPick = vi.fn();
render(<Host places={[HOTEL, EXPO]} onPick={onPick} />);
const input = screen.getByRole('textbox');
await user.type(input, 'Gare');
await screen.findByText('Gare de Lyon');
await user.keyboard('{ArrowDown}{ArrowDown}{ArrowDown}');
// Back under three characters: the trip places are shown and nothing is highlighted,
// so Enter does nothing rather than reaching for the third row of the old list.
await user.type(input, '{backspace}{backspace}{backspace}');
await user.keyboard('{Enter}');
expect(onPick).not.toHaveBeenCalledWith(expect.objectContaining({ lat: expect.any(Number) }));
await user.keyboard('{ArrowDown}{Enter}');
expect(onPick).toHaveBeenLastCalledWith(expect.objectContaining({ name: 'Bach Suites Saigon' }));
});
});
@@ -16,7 +16,12 @@ interface Props {
onChange: (loc: LocationPoint | null) => void
placeholder?: string
style?: React.CSSProperties
places?: { name: string; lat: number; lng: number }[]
/**
* The trip's own places, offered while the field is empty or holds fewer than three
* characters, the way the transit search offers them (#2468). Typing narrows them by
* name; from the third character on the map search answers instead.
*/
places?: LocationPoint[]
}
export default function LocationSelect({ value, onChange, placeholder, style, places }: Props) {
@@ -47,7 +52,7 @@ export default function LocationSelect({ value, onChange, placeholder, style, pl
if (debounceRef.current) clearTimeout(debounceRef.current)
const trimmed = query.trim()
if (trimmed.length < 3 || (value && trimmed === value.name)) {
setResults(trimmed.length < 3 && places ? places : [])
setResults([])
return
}
debounceRef.current = setTimeout(async () => {
@@ -82,11 +87,20 @@ export default function LocationSelect({ value, onChange, placeholder, style, pl
setResults([])
}
// The trip's places stand in for the search below three characters. Derived rather
// than written into `results`, so a search answer that lands after the field was
// shortened cannot replace them, and a place already picked does not reopen them.
const typed = query.trim()
const showPicks = !value && typed.length < 3 && !!places?.length
const rows = showPicks
? (places ?? []).filter(p => !typed || p.name.toLowerCase().includes(typed.toLowerCase()))
: results
const onKey = (e: React.KeyboardEvent<HTMLInputElement>) => {
if (!open || results.length === 0) return
if (e.key === 'ArrowDown') { e.preventDefault(); setHighlight(h => Math.min(h + 1, results.length - 1)) }
if (!open || rows.length === 0) return
if (e.key === 'ArrowDown') { e.preventDefault(); setHighlight(h => Math.min(h + 1, rows.length - 1)) }
else if (e.key === 'ArrowUp') { e.preventDefault(); setHighlight(h => Math.max(h - 1, 0)) }
else if (e.key === 'Enter' && highlight >= 0) { e.preventDefault(); pick(results[highlight]) }
else if (e.key === 'Enter' && rows[highlight]) { e.preventDefault(); pick(rows[highlight]) }
else if (e.key === 'Escape') setOpen(false)
}
@@ -98,7 +112,7 @@ export default function LocationSelect({ value, onChange, placeholder, style, pl
type="text"
value={query}
placeholder={placeholder ?? t('reservations.searchLocation')}
onChange={(e) => { setQuery(e.target.value); setOpen(true); if (value) onChange(null) }}
onChange={(e) => { setQuery(e.target.value); setOpen(true); setHighlight(-1); if (value) onChange(null) }}
onFocus={() => setOpen(true)}
onKeyDown={onKey}
className="bg-transparent text-content"
@@ -111,14 +125,14 @@ export default function LocationSelect({ value, onChange, placeholder, style, pl
)}
</div>
{open && (loading || results.length > 0) && (
{open && ((loading && !showPicks) || rows.length > 0) && (
<div className="bg-surface-card" style={{ position: 'absolute', top: 'calc(100% + 4px)', left: 0, right: 0, border: '1px solid var(--border-primary)', borderRadius: 10, boxShadow: '0 8px 24px rgba(0,0,0,0.18)', maxHeight: 260, overflowY: 'auto', zIndex: 1000 }}>
{loading && results.length === 0 && (
{loading && !showPicks && rows.length === 0 && (
<div className="text-content-faint" style={{ padding: 10, fontSize: 'calc(12px * var(--fs-scale-body, 1))' }}>{t('common.loading')}</div>
)}
{results.map((r, i) => (
{rows.map((r, i) => (
<button
key={`${r.osm_id || r.google_place_id || i}`}
key={showPicks ? `pick:${r.name}:${r.lat}:${r.lng}` : `${r.osm_id || r.google_place_id || i}`}
type="button"
onClick={() => pick(r)}
onMouseEnter={() => setHighlight(i)}
@@ -41,8 +41,13 @@ vi.mock('./AirportSelect', () => ({
}));
vi.mock('./LocationSelect', () => ({
default: ({ onChange }: { onChange: (l: any) => void }) => (
<input data-testid="location-select" type="text" onChange={e => onChange({ name: e.target.value, lat: 0, lng: 0, address: null })} />
default: ({ onChange, places }: { onChange: (l: any) => void; places?: { name: string }[] }) => (
<input
data-testid="location-select"
data-picks={(places ?? []).map(p => p.name).join('|')}
type="text"
onChange={e => onChange({ name: e.target.value, lat: 0, lng: 0, address: null })}
/>
),
}));
@@ -1438,4 +1443,29 @@ describe('TransportModal', () => {
expect(payload.confirmation_number).toBe('BOOK1');
});
});
it('FE-PLANNER-TRANSMODAL-071: the manual From and To fields offer the trip places that have a location, each once (#2468)', async () => {
const places = [
buildPlace({ id: 1, name: 'Louvre', lat: 48.86, lng: 2.33 }),
buildPlace({ id: 2, name: 'No pin yet', lat: null, lng: null }),
buildPlace({ id: 3, name: 'Louvre', lat: 48.86, lng: 2.33 }),
];
render(<TransportModal {...defaultProps} places={places} />);
await userEvent.click(screen.getByRole('button', { name: /^Bus$/i }));
const fields = screen.getAllByTestId('location-select');
expect(fields).toHaveLength(2);
for (const field of fields) expect(field).toHaveAttribute('data-picks', 'Louvre');
});
it('FE-PLANNER-TRANSMODAL-072: every train station field offers the trip places too', async () => {
const places = [buildPlace({ id: 1, name: 'Berlin Hbf', lat: 52.52, lng: 13.37 })];
render(<TransportModal {...defaultProps} places={places} />);
await userEvent.click(screen.getByRole('button', { name: /^Train$/i }));
await userEvent.click(screen.getByRole('button', { name: /Add stop/i }));
const stations = screen.getAllByTestId('location-select');
expect(stations).toHaveLength(3);
for (const station of stations) expect(station).toHaveAttribute('data-picks', 'Berlin Hbf');
});
});
@@ -1,4 +1,4 @@
import { useState, useEffect, useRef } from 'react'
import { useState, useEffect, useRef, useMemo } from 'react'
import { useParams } from 'react-router'
import { Plane, Train, Car, Ship, Bus, Sailboat, Bike, CarTaxiFront, Route, TramFront, Paperclip, FileText, X, ExternalLink, Link2, Plus, Trash2, ChevronUp, ChevronDown } from 'lucide-react'
import Modal from '../shared/Modal'
@@ -7,6 +7,7 @@ import { BookingCodeInput } from '../shared/BookingCode'
import CustomTimePicker from '../shared/CustomTimePicker'
import AirportSelect, { type Airport } from './AirportSelect'
import LocationSelect, { type LocationPoint } from './LocationSelect'
import { toLocationPicks } from './locationPicks'
import { useTranslation } from '../../i18n'
import { useToast } from '../shared/Toast'
import { useTripStore } from '../../store/tripStore'
@@ -195,6 +196,8 @@ interface TransportModalProps {
export function TransportModal({ isOpen, onClose, onSave, reservation, days, selectedDayId, files = [], onFileUpload, onFileDelete, onOpenExpense, prefill = null, places = [], assignments = {}, accommodations = [], initialAutomated = false, transitPrefill = null, tripHasDates = true, tripMembers = [] }: TransportModalProps) {
const { t, locale } = useTranslation()
const toast = useToast()
// The trip's places, offered by every location field of the manual tab (#2468).
const locationPicks = useMemo(() => toLocationPicks(places), [places])
const isBudgetEnabled = useAddonStore(s => s.isEnabled('budget'))
const budgetItems = useTripStore(s => s.budgetItems)
const deleteBudgetItem = useTripStore(s => s.deleteBudgetItem)
@@ -937,7 +940,7 @@ export function TransportModal({ isOpen, onClose, onSave, reservation, days, sel
<div style={{ display: 'flex', alignItems: 'center', gap: 8 }}>
<span className="text-content-faint" style={{ fontSize: 'calc(10px * var(--fs-scale-caption, 1))', fontWeight: 700, textTransform: 'uppercase', letterSpacing: '0.03em', flexShrink: 0 }}>{roleLabel}</span>
<div style={{ flex: 1, minWidth: 0 }}>
<LocationSelect value={wp.location} onChange={l => updateWp({ location: l || null })} />
<LocationSelect value={wp.location} onChange={l => updateWp({ location: l || null })} places={locationPicks} />
</div>
{!isFirst && !isLast && (
<button type="button" onClick={() => setTrainWaypoints(prev => prev.filter((_, j) => j !== i))} aria-label={t('common.delete')} className="text-content-faint" style={{ background: 'none', border: 'none', cursor: 'pointer', display: 'flex', padding: 4, flexShrink: 0 }}>
@@ -1009,11 +1012,11 @@ export function TransportModal({ isOpen, onClose, onSave, reservation, days, sel
<div className="grid grid-cols-1 sm:grid-cols-2 gap-3">
<div>
<label className={labelClass}>{t('reservations.meta.from')}</label>
<LocationSelect value={fromPick.location || null} onChange={l => setFromPick({ location: l || undefined })} places={places} />
<LocationSelect value={fromPick.location || null} onChange={l => setFromPick({ location: l || undefined })} places={locationPicks} />
</div>
<div>
<label className={labelClass}>{t('reservations.meta.to')}</label>
<LocationSelect value={toPick.location || null} onChange={l => setToPick({ location: l || undefined })} places={places} />
<LocationSelect value={toPick.location || null} onChange={l => setToPick({ location: l || undefined })} places={locationPicks} />
</div>
</div>
@@ -1057,6 +1060,7 @@ export function TransportModal({ isOpen, onClose, onSave, reservation, days, sel
<LocationSelect
value={stop.location}
onChange={l => setCarStops(prev => prev.map((s, j) => (j === i ? { ...s, location: l || null } : s)))}
places={locationPicks}
/>
</div>
<div style={{ width: 110, flexShrink: 0 }}>
@@ -0,0 +1,32 @@
// FE-PLANNER-LOCPICKS-001 to FE-PLANNER-LOCPICKS-003
import { toLocationPicks } from './locationPicks';
describe('toLocationPicks', () => {
it('FE-PLANNER-LOCPICKS-001: a trip place becomes a pick with its name, coordinates and address', () => {
expect(toLocationPicks([{ name: ' Bach Suites Saigon ', lat: '10.78', lng: 106.7, address: 'District 1' }])).toEqual([
{ name: 'Bach Suites Saigon', lat: 10.78, lng: 106.7, address: 'District 1' },
]);
});
it('FE-PLANNER-LOCPICKS-002: a place without a name or a real coordinate is not offered, never as 0,0', () => {
expect(toLocationPicks([
{ name: 'No coordinates', lat: null, lng: null },
{ name: 'Half a coordinate', lat: 10, lng: null },
{ name: 'Blank coordinate', lat: '', lng: '' },
{ name: 'Not a number', lat: 'north', lng: 106.7 },
{ name: ' ', lat: 10, lng: 106 },
{ name: null, lat: 10, lng: 106 },
])).toEqual([]);
expect(toLocationPicks(undefined)).toEqual([]);
});
it('FE-PLANNER-LOCPICKS-003: the same place added twice is offered once, a namesake elsewhere stays', () => {
const picks = toLocationPicks([
{ name: 'Hotel', lat: 10, lng: 106 },
{ name: 'Hotel', lat: 10, lng: 106 },
{ name: 'Hotel', lat: 11, lng: 106 },
]);
expect(picks.map(p => `${p.name}@${p.lat}`)).toEqual(['Hotel@10', 'Hotel@11']);
expect(picks[0].address).toBeNull();
});
});
@@ -0,0 +1,33 @@
import type { LocationPoint } from './LocationSelect'
/** A trip place as far as a location field can offer it. */
interface PlaceLike {
name?: string | null
lat?: number | string | null
lng?: number | string | null
address?: string | null
}
/**
* The trip's places as ready-made picks for a location field.
*
* Only a place with a name and a real coordinate is offered. A place without one would
* be picked as 0,0 (`Number(null)` is a finite 0), which puts the endpoint of a booking
* in the Gulf of Guinea. The same place added twice is offered once.
*/
export function toLocationPicks(places: readonly PlaceLike[] | null | undefined): LocationPoint[] {
const picks: LocationPoint[] = []
const seen = new Set<string>()
for (const place of places ?? []) {
const name = place.name?.trim()
if (!name || place.lat == null || place.lng == null || place.lat === '' || place.lng === '') continue
const lat = Number(place.lat)
const lng = Number(place.lng)
if (!Number.isFinite(lat) || !Number.isFinite(lng)) continue
const key = `${name}:${lat}:${lng}`
if (seen.has(key)) continue
seen.add(key)
picks.push({ name, lat, lng, address: place.address ?? null })
}
return picks
}
@@ -1,4 +1,4 @@
import { useEffect, useRef, useState } from 'react'
import { useEffect, useMemo, useRef, useState } from 'react'
import { Bike, Bus, Car, CarTaxiFront, Check, ChevronDown, ChevronUp, Plane, Plus, Route, Sailboat, Ship, Train, TrainFront, TramFront, Trash2, X } from 'lucide-react'
import MSheet from '../../../components/MSheet'
import { useAddonStore } from '../../../../store/addonStore'
@@ -11,6 +11,7 @@ import CustomTimePicker from '../../../../components/shared/CustomTimePicker'
import { BookingCodeInput } from '../../../../components/shared/BookingCode'
import AirportSelect, { type Airport } from '../../../../components/Planner/AirportSelect'
import LocationSelect, { type LocationPoint } from '../../../../components/Planner/LocationSelect'
import { toLocationPicks } from '../../../../components/Planner/locationPicks'
import TransitSearchPanel from '../../../../components/Planner/TransitSearchPanel'
import { Eyebrow, FIELD_AREA_CLS, FIELD_CLS, FormSheetFooter, FormSheetHeader } from './PlSheetChrome'
import PlFileAttach from './PlFileAttach'
@@ -158,6 +159,8 @@ export default function MTransportFormSheet({ planner, onOpenExpense }: MTranspo
const isBudgetEnabled = useAddonStore(s => s.isEnabled('budget'))
const tripHasDates = Boolean(trip?.start_date && trip?.end_date)
// The trip's places, offered by every location field of the manual tab (#2468).
const locationPicks = useMemo(() => toLocationPicks(places), [places])
const [form, setForm] = useState({ ...EMPTY })
const [automated, setAutomated] = useState(false)
@@ -850,7 +853,7 @@ export default function MTransportFormSheet({ planner, onOpenExpense }: MTranspo
<div className="mb-[8px] flex items-center gap-2">
<span className="flex-none font-geist text-[0.625rem] font-bold uppercase tracking-[.09em] text-m-faint">{roleLabel}</span>
<div className="min-w-0 flex-1">
<LocationSelect value={wp.location} onChange={l => updateWp({ location: l || null })} />
<LocationSelect value={wp.location} onChange={l => updateWp({ location: l || null })} places={locationPicks} />
</div>
{!isFirst && !isLast && (
<button type="button" onClick={() => setTrainWaypoints(prev => prev.filter((_, j) => j !== i))} aria-label={t('common.delete')} className="flex-none text-m-faint">
@@ -928,9 +931,9 @@ export default function MTransportFormSheet({ planner, onOpenExpense }: MTranspo
<>
{/* From / To endpoints (non-flight / non-train) */}
<Eyebrow className="mb-[5px] mt-3 uppercase">{t('reservations.meta.from')}</Eyebrow>
<LocationSelect value={fromPick.location || null} onChange={l => setFromPick({ location: l || undefined })} />
<LocationSelect value={fromPick.location || null} onChange={l => setFromPick({ location: l || undefined })} places={locationPicks} />
<Eyebrow className="mb-[5px] mt-3 uppercase">{t('reservations.meta.to')}</Eyebrow>
<LocationSelect value={toPick.location || null} onChange={l => setToPick({ location: l || undefined })} />
<LocationSelect value={toPick.location || null} onChange={l => setToPick({ location: l || undefined })} places={locationPicks} />
{/* Stops along the drive — cars only (#1797). The rental frame above stays the
pick-up and return; these are the places in between, in order. */}
@@ -969,6 +972,7 @@ export default function MTransportFormSheet({ planner, onOpenExpense }: MTranspo
<LocationSelect
value={stop.location}
onChange={l => setCarStops(prev => prev.map((s, j) => (j === i ? { ...s, location: l || null } : s)))}
places={locationPicks}
/>
</div>
<div className="w-[92px] shrink-0">
@@ -65,9 +65,10 @@ vi.mock('../../../../src/components/Planner/AirportSelect', () => ({
}))
vi.mock('../../../../src/components/Planner/LocationSelect', () => ({
default: ({ value, onChange }: { value: { name: string } | null; onChange: (l: unknown) => void }) => (
default: ({ value, onChange, places }: { value: { name: string } | null; onChange: (l: unknown) => void; places?: { name: string }[] }) => (
<input
aria-label="location-select"
data-picks={(places ?? []).map(p => p.name).join('|')}
value={value?.name ?? ''}
onChange={e => onChange(LOCATIONS[e.target.value] ?? null)}
/>
@@ -1021,4 +1022,19 @@ describe('MTransportFormSheet', () => {
expect(payload.confirmation_number).toBe('BOOK1')
})
})
it('FE-MOB-TRFRM-053: the manual From and To fields offer the trip places that have a location, each once (#2468)', () => {
renderSheet(makePlanner({
places: [
{ id: 1, name: 'Fushimi Inari', lat: 34.97, lng: 135.77 },
{ id: 2, name: 'No pin yet', lat: null, lng: null },
{ id: 3, name: 'Fushimi Inari', lat: 34.97, lng: 135.77 },
],
}))
fireEvent.click(screen.getByRole('button', { name: 'reservations.type.bus' }))
const fields = locationInputs()
expect(fields).toHaveLength(2)
for (const field of fields) expect(field).toHaveAttribute('data-picks', 'Fushimi Inari')
})
})