diff --git a/client/src/components/Planner/LocationSelect.test.tsx b/client/src/components/Planner/LocationSelect.test.tsx index c83ca1204..22ec2eb15 100644 --- a/client/src/components/Planner/LocationSelect.test.tsx +++ b/client/src/components/Planner/LocationSelect.test.tsx @@ -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)) // 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(initial); - return { setValue(l); onPick?.(l); }} />; + return { 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(); @@ -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(); + 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(); + 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(); + 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(); + 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(); + 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' })); + }); }); diff --git a/client/src/components/Planner/LocationSelect.tsx b/client/src/components/Planner/LocationSelect.tsx index c0985b26c..4fc08c450 100644 --- a/client/src/components/Planner/LocationSelect.tsx +++ b/client/src/components/Planner/LocationSelect.tsx @@ -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) => { - 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 )} - {open && (loading || results.length > 0) && ( + {open && ((loading && !showPicks) || rows.length > 0) && (
- {loading && results.length === 0 && ( + {loading && !showPicks && rows.length === 0 && (
{t('common.loading')}
)} - {results.map((r, i) => ( + {rows.map((r, i) => (
diff --git a/client/src/components/Planner/locationPicks.test.ts b/client/src/components/Planner/locationPicks.test.ts new file mode 100644 index 000000000..5fb864ddd --- /dev/null +++ b/client/src/components/Planner/locationPicks.test.ts @@ -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(); + }); +}); diff --git a/client/src/components/Planner/locationPicks.ts b/client/src/components/Planner/locationPicks.ts new file mode 100644 index 000000000..0070a2057 --- /dev/null +++ b/client/src/components/Planner/locationPicks.ts @@ -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() + 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 +} diff --git a/client/src/mobile/screens/trip/sheets/MTransportFormSheet.tsx b/client/src/mobile/screens/trip/sheets/MTransportFormSheet.tsx index 8644d3244..9dd067cf5 100644 --- a/client/src/mobile/screens/trip/sheets/MTransportFormSheet.tsx +++ b/client/src/mobile/screens/trip/sheets/MTransportFormSheet.tsx @@ -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
{roleLabel}
- updateWp({ location: l || null })} /> + updateWp({ location: l || null })} places={locationPicks} />
{!isFirst && !isLast && (
diff --git a/client/tests/unit/mobile/trip/MTransportFormSheet.test.tsx b/client/tests/unit/mobile/trip/MTransportFormSheet.test.tsx index af6a9002b..80bdb9e1d 100644 --- a/client/tests/unit/mobile/trip/MTransportFormSheet.test.tsx +++ b/client/tests/unit/mobile/trip/MTransportFormSheet.test.tsx @@ -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 }[] }) => ( 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') + }) })