mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
@
fix(import): stop reading tickets as lodging, and stop scroll escaping a dropdown Two v4.0.0 reports. The file importer classified nearly every transport document as a hotel. Its keyword table tested lodging second, on a group that included "check-in" and "check-out" — words printed on almost every ferry, train and bus ticket there is — so a ferry ticket came back a hotel, and the review then handed the user the booking form, whose six chips contain no transport type at all. That is where "everything ends up as other" comes from: it was the only honest pick left. Transport is tested first now, the lodging pattern asks for a word that names lodging, and the rental pattern asks for a rental rather than for a pick-up time that ferries and shuttles also print. Two more in the same function. The flight path forces its type on everything it returns, and it claimed any document with two letters followed by digits — which is what a German rail number looks like, so an IC ticket was imported as a flight without the keyword table ever running. It now stands down when the document already names another kind of transport. And a type neither form can express opened the booking form regardless; it now follows the tab the import was started from, while a type either form knows still wins over the tab, since one PDF routinely holds a flight and a hotel. An unreadable type reaching the transport form arrives as transport_other rather than as a flight: a wrong flight looks right enough to be saved unnoticed. The other report: a dropdown inside a sheet scrolled the page instead of its own list. The panel is portaled to document.body and positioned fixed, so its scroll chain runs to the viewport rather than to the sheet it looks like it belongs to, and nothing stopped it. Three portaled lists carry overscroll-behavior: contain now — the select, the navigation menu (which reaches the phone through the place sheet, not only the desktop inspector) and the phone day picker. @
This commit is contained in:
@@ -23,7 +23,7 @@ export function MobileDayPickerSheet(S: SidebarState) {
|
||||
<div className="text-content" style={{ fontSize: 'calc(15px * var(--fs-scale-subtitle, 1))', fontWeight: 700 }}>{dayPickerPlace.name}</div>
|
||||
{dayPickerPlace.address && <div className="text-content-faint" style={{ fontSize: 'calc(12px * var(--fs-scale-body, 1))', marginTop: 2 }}>{dayPickerPlace.address}</div>}
|
||||
</div>
|
||||
<div style={{ overflowY: 'auto', padding: '8px 12px' }}>
|
||||
<div style={{ overflowY: 'auto', overscrollBehavior: 'contain', padding: '8px 12px' }}>
|
||||
{/* View details */}
|
||||
<button type="button"
|
||||
onClick={() => { onPlaceClick(dayPickerPlace.id); setDayPickerPlace(null); setMobileShowDays(false) }}
|
||||
|
||||
@@ -531,6 +531,18 @@ describe('TransportModal', () => {
|
||||
{ id: 2, reservation_id: 1, role: 'to', sequence: 1, name: 'New York (JFK)', code: 'JFK', lat: 40.64, lng: -73.78, timezone: 'America/New_York', local_date: toDate, local_time: '13:00' },
|
||||
]);
|
||||
|
||||
// #2076 — an import whose type could not be read used to arrive here as a flight.
|
||||
// A wrong flight looks right enough to be saved without a second look; an
|
||||
// explicit "other" asks to be corrected.
|
||||
it('FE-PLANNER-TRANSMODAL-062: an unrecognised prefill type lands on transport_other, not flight', async () => {
|
||||
const onSave = vi.fn().mockResolvedValue(undefined);
|
||||
const prefill = { title: 'Airport transfer', type: 'shuttle-voucher', status: 'pending' } as any;
|
||||
render(<TransportModal {...defaultProps} prefill={prefill} onSave={onSave} />);
|
||||
await userEvent.click(screen.getByRole('button', { name: /^Add$/i }));
|
||||
await waitFor(() => expect(onSave).toHaveBeenCalled());
|
||||
expect(onSave.mock.calls[0][0].type).toBe('transport_other');
|
||||
});
|
||||
|
||||
it('FE-PLANNER-TRANSMODAL-030: an import prefill resolves each waypoint day from its endpoint local_date (#1684)', async () => {
|
||||
const onSave = vi.fn().mockResolvedValue(undefined);
|
||||
// A parsed import carries local_date per endpoint but no day_id at all.
|
||||
|
||||
@@ -225,9 +225,12 @@ export function TransportModal({ isOpen, onClose, onSave, reservation, days, sel
|
||||
const eps = src.endpoints || []
|
||||
const from = eps.find(e => e.role === 'from')
|
||||
const to = eps.find(e => e.role === 'to')
|
||||
// 'transport_other', not 'flight': an import whose type could not be read has
|
||||
// to arrive as something the user corrects, and a wrong flight looks right
|
||||
// enough to be saved unnoticed (#2076).
|
||||
const type = (TRANSPORT_TYPES as readonly string[]).includes(src.type)
|
||||
? src.type as TransportType
|
||||
: 'flight'
|
||||
: 'transport_other'
|
||||
setForm({
|
||||
title: src.title || '',
|
||||
type,
|
||||
|
||||
@@ -44,6 +44,17 @@ export function parsedItemToDraft(item: BookingImportPreviewItem): BookingReview
|
||||
|
||||
/** Transport types route to the TransportModal; everything else to the ReservationModal. */
|
||||
const TRANSPORT_TYPES = new Set(['flight', 'train', 'bus', 'car', 'taxi', 'bicycle', 'cruise', 'ferry', 'transit', 'transport_other'])
|
||||
/** The types the booking form can express — its own chip list. */
|
||||
const BOOKING_TYPES = new Set(['hotel', 'restaurant', 'event', 'tour', 'activity', 'parking', 'other'])
|
||||
|
||||
export function isTransportItem(item: BookingImportPreviewItem): boolean {
|
||||
return TRANSPORT_TYPES.has(item.type)
|
||||
}
|
||||
|
||||
/**
|
||||
* Neither form can express this type, so neither is obviously right. The tab the
|
||||
* user started the import from breaks the tie (#2076).
|
||||
*/
|
||||
export function isUnplaceableItem(item: BookingImportPreviewItem): boolean {
|
||||
return !TRANSPORT_TYPES.has(item.type) && !BOOKING_TYPES.has(item.type)
|
||||
}
|
||||
|
||||
@@ -80,6 +80,30 @@ describe('CustomSelect', () => {
|
||||
expect(screen.queryByText('Cherry')).toBeNull();
|
||||
});
|
||||
|
||||
// #2078 — the panel is portaled to document.body and positioned fixed, so its
|
||||
// scroll chain runs to the viewport, not to the sheet it visually sits in. On a
|
||||
// phone a flick past either end of the list moved the page instead.
|
||||
it('FE-COMP-SELECT-009: the option list keeps its scroll to itself', async () => {
|
||||
const user = userEvent.setup();
|
||||
render(<CustomSelect value="" onChange={onChange} options={OPTIONS} />);
|
||||
await user.click(screen.getByRole('button'));
|
||||
|
||||
const list = screen.getByText('Apple').closest('div[style*="overflow"]') as HTMLElement;
|
||||
expect(list.style.overscrollBehavior).toBe('contain');
|
||||
});
|
||||
|
||||
it('FE-COMP-SELECT-010: it is still a bounded scroller, not a contained page', async () => {
|
||||
const user = userEvent.setup();
|
||||
render(<CustomSelect value="" onChange={onChange} options={OPTIONS} />);
|
||||
await user.click(screen.getByRole('button'));
|
||||
|
||||
// Guards the other half: containment without a height cap would just make the
|
||||
// panel grow off screen.
|
||||
const list = screen.getByText('Apple').closest('div[style*="overflow"]') as HTMLElement;
|
||||
expect(list.style.overflowY).toBe('auto');
|
||||
expect(Number.parseInt(list.style.maxHeight, 10)).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
it('FE-COMP-SELECT-008: disabled state prevents the dropdown from opening', async () => {
|
||||
const user = userEvent.setup();
|
||||
render(<CustomSelect value="" onChange={onChange} options={OPTIONS} disabled={true} placeholder="Pick" />);
|
||||
|
||||
@@ -171,6 +171,11 @@ export default function CustomSelect({
|
||||
<div style={{
|
||||
maxHeight: Math.min(220, Math.max(96, (anchored?.maxHeight ?? 220) - (searchable ? 38 : 0))),
|
||||
overflowY: 'auto',
|
||||
// The panel is portaled to document.body and positioned fixed, so its
|
||||
// scroll chain runs to the viewport rather than to the sheet it looks
|
||||
// like it belongs to. On a phone that meant a flick past either end of
|
||||
// the list moved the page instead (#2078).
|
||||
overscrollBehavior: 'contain',
|
||||
padding: '4px',
|
||||
}}>
|
||||
{filtered.length === 0 ? (
|
||||
|
||||
@@ -82,6 +82,10 @@ export function NavigationMenu({ targets, anchor, onClose, title }: NavigationMe
|
||||
minWidth: 186,
|
||||
maxHeight: `calc(100vh - ${EDGE * 2}px)`,
|
||||
overflowY: 'auto',
|
||||
// Portaled and fixed like CustomSelect's list, so it chains to the viewport
|
||||
// unless told otherwise (#2078). This one reaches the phone through
|
||||
// MPlaceSheet, not only the desktop inspector.
|
||||
overscrollBehavior: 'contain',
|
||||
padding: 5,
|
||||
background: 'var(--bg-card)',
|
||||
backdropFilter: 'blur(24px) saturate(180%)',
|
||||
|
||||
@@ -417,7 +417,7 @@ export default function MTripShell({
|
||||
}}
|
||||
/>
|
||||
{planner.bookingImportAvailable && (
|
||||
<MIconBtn ariaLabel={t('reservations.import.title')} onClick={() => planner.setShowBookingImport(true)} size={40} className="text-m-muted backdrop-blur-[24px] backdrop-saturate-[1.7]">
|
||||
<MIconBtn ariaLabel={t('reservations.import.title')} onClick={() => { planner.setBookingImportKind('transports'); planner.setShowBookingImport(true) }} size={40} className="text-m-muted backdrop-blur-[24px] backdrop-saturate-[1.7]">
|
||||
<Download size={15} strokeWidth={2} />
|
||||
</MIconBtn>
|
||||
)}
|
||||
@@ -437,7 +437,7 @@ export default function MTripShell({
|
||||
onClick={() => { planner.setEditingReservation(null); planner.setShowReservationModal(true) }}
|
||||
/>
|
||||
{planner.bookingImportAvailable && (
|
||||
<MIconBtn ariaLabel={t('reservations.import.title')} onClick={() => planner.setShowBookingImport(true)} size={40} className="text-m-muted backdrop-blur-[24px] backdrop-saturate-[1.7]">
|
||||
<MIconBtn ariaLabel={t('reservations.import.title')} onClick={() => { planner.setBookingImportKind('bookings'); planner.setShowBookingImport(true) }} size={40} className="text-m-muted backdrop-blur-[24px] backdrop-saturate-[1.7]">
|
||||
<Download size={15} strokeWidth={2} />
|
||||
</MIconBtn>
|
||||
)}
|
||||
|
||||
@@ -252,7 +252,7 @@ function TripPlannerPageDesktop(): React.ReactElement | null {
|
||||
prefillCoords, setPrefillCoords, editingAssignmentId, setEditingAssignmentId,
|
||||
showTripForm, setShowTripForm, showMembersModal, setShowMembersModal,
|
||||
showReservationModal, setShowReservationModal, editingReservation, setEditingReservation,
|
||||
showBookingImport, setShowBookingImport, bookingImportAvailable,
|
||||
showBookingImport, setShowBookingImport, setBookingImportKind, bookingImportAvailable,
|
||||
airTrailAvailable, showAirTrailImport, setShowAirTrailImport,
|
||||
bookingForAssignmentId, setBookingForAssignmentId,
|
||||
showTransportModal, setShowTransportModal, editingTransport, setEditingTransport,
|
||||
@@ -732,7 +732,7 @@ function TripPlannerPageDesktop(): React.ReactElement | null {
|
||||
assignments={assignments}
|
||||
files={files}
|
||||
onAdd={() => { setEditingTransport(null); setTransitPrefill(null); setTransportModalAutomated(false); setShowTransportModal(true) }}
|
||||
onImport={() => setShowBookingImport(true)}
|
||||
onImport={() => { setBookingImportKind('transports'); setShowBookingImport(true) }}
|
||||
bookingImportAvailable={bookingImportAvailable}
|
||||
onAirTrailImport={() => setShowAirTrailImport(true)}
|
||||
airTrailAvailable={airTrailAvailable}
|
||||
@@ -758,7 +758,7 @@ function TripPlannerPageDesktop(): React.ReactElement | null {
|
||||
assignments={assignments}
|
||||
files={files}
|
||||
onAdd={() => { setEditingReservation(null); setShowReservationModal(true) }}
|
||||
onImport={() => setShowBookingImport(true)}
|
||||
onImport={() => { setBookingImportKind('bookings'); setShowBookingImport(true) }}
|
||||
bookingImportAvailable={bookingImportAvailable}
|
||||
onEdit={(r) => { setEditingReservation(r); setShowReservationModal(true) }}
|
||||
onDelete={handleDeleteReservation}
|
||||
|
||||
@@ -193,6 +193,8 @@ function baseState(): HookState {
|
||||
setEditingReservation: vi.fn(),
|
||||
showBookingImport: false,
|
||||
setShowBookingImport: vi.fn(),
|
||||
bookingImportKind: 'bookings' as const,
|
||||
setBookingImportKind: vi.fn(),
|
||||
bookingImportAvailable: true,
|
||||
airTrailAvailable: true,
|
||||
showAirTrailImport: false,
|
||||
|
||||
@@ -1634,6 +1634,45 @@ describe('useTripPlanner — booking import review', () => {
|
||||
expect(result.current.reservationPrefill?._sourceFiles).toEqual([file])
|
||||
})
|
||||
|
||||
// #2076 — an item whose type neither form can express used to land in the booking
|
||||
// form, which offers six chips and not one transport among them, so the only
|
||||
// honest pick left was 'other'. The tab the import started from breaks the tie.
|
||||
it('FE-TP-HOOK-112: an unreadable item opens the transport form when the import began there', async () => {
|
||||
seedTrip()
|
||||
const { result } = await renderPlanner()
|
||||
const odd = { type: 'shuttle-voucher', title: 'Airport transfer' }
|
||||
|
||||
act(() => { result.current.setBookingImportKind('transports') })
|
||||
act(() => { result.current.startImportReview([odd] as never) })
|
||||
|
||||
expect(result.current.showTransportModal).toBe(true)
|
||||
expect(result.current.showReservationModal).toBe(false)
|
||||
})
|
||||
|
||||
it('FE-TP-HOOK-113: the same item opens the booking form when the import began there', async () => {
|
||||
seedTrip()
|
||||
const { result } = await renderPlanner()
|
||||
const odd = { type: 'shuttle-voucher', title: 'Airport transfer' }
|
||||
|
||||
act(() => { result.current.setBookingImportKind('bookings') })
|
||||
act(() => { result.current.startImportReview([odd] as never) })
|
||||
|
||||
expect(result.current.showReservationModal).toBe(true)
|
||||
expect(result.current.showTransportModal).toBe(false)
|
||||
})
|
||||
|
||||
// A recognised type always wins over the tab: one PDF routinely holds both.
|
||||
it('FE-TP-HOOK-114: a hotel imported from the transports tab still opens the booking form', async () => {
|
||||
seedTrip()
|
||||
const { result } = await renderPlanner()
|
||||
|
||||
act(() => { result.current.setBookingImportKind('transports') })
|
||||
act(() => { result.current.startImportReview([hotelItem] as never) })
|
||||
|
||||
expect(result.current.showReservationModal).toBe(true)
|
||||
expect(result.current.showTransportModal).toBe(false)
|
||||
})
|
||||
|
||||
it('FE-TP-HOOK-090: advancing moves on to the transport item and then finishes the session', async () => {
|
||||
seedTrip()
|
||||
vi.mocked(accommodationsApi.list).mockResolvedValue({ accommodations: [{ id: 2 }] })
|
||||
|
||||
@@ -9,7 +9,7 @@ import { Map, Ticket, PackageCheck, Wallet, FolderOpen, Users, Train } from 'luc
|
||||
import { resolvePluginIcon } from '../../components/shared/PluginIcon'
|
||||
import { useTranslation, translateApiError } from '../../i18n'
|
||||
import { addonsApi, accommodationsApi, authApi, tripsApi, assignmentsApi, healthApi, airtrailApi, mapsApi, placesApi } from '../../api/client'
|
||||
import { parsedItemToDraft, isTransportItem, type BookingReviewDraft } from '../../components/Planner/parsedItemToDraft'
|
||||
import { parsedItemToDraft, isTransportItem, isUnplaceableItem, type BookingReviewDraft } from '../../components/Planner/parsedItemToDraft'
|
||||
import type { BookingImportPreviewItem } from '@trek/shared'
|
||||
import { accommodationRepo } from '../../repo/accommodationRepo'
|
||||
import { offlineDb, getImportFiles, deleteImportFiles } from '../../db/offlineDb'
|
||||
@@ -228,6 +228,8 @@ export function useTripPlanner() {
|
||||
const [showReservationModal, setShowReservationModal] = useState<boolean>(false)
|
||||
const [editingReservation, setEditingReservation] = useState<Reservation | null>(null)
|
||||
const [showBookingImport, setShowBookingImport] = useState<boolean>(false)
|
||||
// Which tab opened the importer. Only ever a tie-breaker — see openImportItem.
|
||||
const [bookingImportKind, setBookingImportKind] = useState<'transports' | 'bookings'>('bookings')
|
||||
const [bookingImportAvailable, setBookingImportAvailable] = useState<boolean>(false)
|
||||
const { available: airTrailAvailable } = useAirtrailConnection()
|
||||
const [showAirTrailImport, setShowAirTrailImport] = useState<boolean>(false)
|
||||
@@ -919,13 +921,19 @@ export function useTripPlanner() {
|
||||
}
|
||||
|
||||
// Open the right edit modal for a parsed item, pre-filled, in create mode.
|
||||
//
|
||||
// A type neither form can express belongs to whichever tab the user started from.
|
||||
// Handing an unreadable transport document to the booking form is what left them
|
||||
// with six chips, none of them a transport, and 'other' as the only honest pick
|
||||
// (#2076). A type either form DOES know always wins over the tab — one PDF
|
||||
// routinely holds a flight and a hotel.
|
||||
const openImportItem = (item: BookingImportPreviewItem) => {
|
||||
const draft = parsedItemToDraft(item)
|
||||
// Attach the file this item was parsed from so it lands in the booking's Files on save.
|
||||
const srcName = item.source?.fileName
|
||||
const srcFile = srcName ? importSourceFilesRef.current.find(f => f.name === srcName) : undefined
|
||||
if (srcFile) draft._sourceFiles = [srcFile]
|
||||
if (isTransportItem(item)) {
|
||||
if (isTransportItem(item) || (isUnplaceableItem(item) && bookingImportKind === 'transports')) {
|
||||
setShowReservationModal(false); setEditingReservation(null); setReservationPrefill(null)
|
||||
setEditingTransport(null); setTransportModalDayId(null)
|
||||
setTransportPrefill(draft); setShowTransportModal(true)
|
||||
@@ -1041,7 +1049,7 @@ export function useTripPlanner() {
|
||||
placeFormDayId, setPlaceFormDayId, reservationModalDayId, setReservationModalDayId,
|
||||
showTripForm, setShowTripForm, showMembersModal, setShowMembersModal,
|
||||
showReservationModal, setShowReservationModal, editingReservation, setEditingReservation,
|
||||
showBookingImport, setShowBookingImport, bookingImportAvailable,
|
||||
showBookingImport, setShowBookingImport, bookingImportKind, setBookingImportKind, bookingImportAvailable,
|
||||
airTrailAvailable, showAirTrailImport, setShowAirTrailImport,
|
||||
bookingForAssignmentId, setBookingForAssignmentId,
|
||||
showTransportModal, setShowTransportModal, editingTransport, setEditingTransport,
|
||||
|
||||
@@ -133,6 +133,8 @@ export function buildPlanner(overrides: Partial<TripPlanner> = {}): TripPlanner
|
||||
setEditingReservation: vi.fn(),
|
||||
showBookingImport: false,
|
||||
setShowBookingImport: vi.fn(),
|
||||
bookingImportKind: 'bookings' as const,
|
||||
setBookingImportKind: vi.fn(),
|
||||
bookingImportAvailable: true,
|
||||
airTrailAvailable: false,
|
||||
showAirTrailImport: false,
|
||||
|
||||
Reference in New Issue
Block a user