diff --git a/client/src/mobile/screens/trip/roadtrip/MRoadtripRows.tsx b/client/src/mobile/screens/trip/roadtrip/MRoadtripRows.tsx index 54d5b41f5..dd1bc50cf 100644 --- a/client/src/mobile/screens/trip/roadtrip/MRoadtripRows.tsx +++ b/client/src/mobile/screens/trip/roadtrip/MRoadtripRows.tsx @@ -31,28 +31,47 @@ export interface RowChrome { unit: DistanceUnit } -/** 30px disc: a number for a destination, the kind's colour for a service stop. */ -function Disc({ row }: { row: StopRow }) { +/** + * 30px disc: a number for a destination, the kind's colour for a service stop. + * + * With `onPickKind` it is also the control that switches between the two, which is what + * the desktop rail does with its own disc. The disc is what changes, so it is its own + * preview: tap the 3, pick the pump, and the 3 becomes an orange disc while everything + * below it renumbers. The tap is stopped here rather than allowed to reach the row, which + * opens the stop, and the 34px column the row gives this disc is the whole target. + */ +function Disc({ row, t, onPickKind }: { row: StopRow; t: TranslationFn; onPickKind?: () => void }) { const kind = row.stop.stopType ? STOP_KIND_BY_KEY[row.stop.stopType] : undefined - if (row.service && kind) { - const Icon = kind.Icon - return ( + const face = row.service && kind + ? ( - ) - } + : ( + // Deliberately not font-geist: `.m-root .font-geist` caps the tier at Medium, + // which would quietly undo the bold on a number that has to read at 12px. + + {row.number} + + ) + + if (!onPickKind) return face return ( - // Deliberately not font-geist: `.m-root .font-geist` caps the tier at Medium, - // which would quietly undo the bold on a number that has to read at 12px. - - {row.number} - + ) } @@ -90,10 +109,12 @@ function warningMark(warning: ScheduleWarning, chrome: RowChrome): ReactNode { * why it is a div with role rather than a button: the desktop needs controls nested * inside its row, and a button inside a button is not markup. */ -export function RtStopRow({ row, chrome, onOpen }: { +export function RtStopRow({ row, chrome, onOpen, onPickKind }: { row: StopRow chrome: RowChrome onOpen: () => void + /** Absent for a traveller who may not edit places: the disc is then not a control. */ + onPickKind?: () => void }) { const { t, unit } = chrome const marks: ReactNode[] = [] @@ -119,7 +140,7 @@ export function RtStopRow({ row, chrome, onOpen }: { onKeyDown={e => { if (e.target === e.currentTarget && (e.key === 'Enter' || e.key === ' ')) { e.preventDefault(); onOpen() } }} className="grid cursor-pointer items-center gap-x-[10px] py-1" style={{ gridTemplateColumns: '34px 1fr auto' }} > - + {row.stop.name} @@ -142,6 +163,30 @@ const LEG_ICONS: Record = { cycling: Bike, } +/** + * What a leg pill says: how far and how long, or that it is not routed yet. + * + * A segment carries both a pre-formatted text and the raw metres and seconds, and the two + * do not always arrive together: a segment that reaches the chain before its routing round + * has landed has the numbers at zero and the texts empty. Reading only the texts printed + * the template with nothing in its slots, so a leg showed as a bare " in " (the + * separator of `roadtrip.leg.driveText` with both values missing) — the screenshot is a + * hotel followed by a pill saying nothing. + * + * So the text is used when there is one and the numbers are formatted when there is not, + * which is what the desktop rail does with the same segment. With neither, the leg says + * it is pending, the honest answer and the one the missing-segment case already gave. + */ +function legText(seg: RouteSegment | undefined, { t, unit }: RowChrome): string { + if (!seg) return t('roadtrip.leg.pending') + const metres = Number.isFinite(seg.distance) ? seg.distance : 0 + const seconds = Number.isFinite(seg.duration) ? seg.duration : 0 + const distance = seg.distanceText || (metres > 0 ? formatDistance(metres / 1000, unit) : '') + const time = seg.durationText || seg.drivingText || (seconds > 0 ? formatDurationShort(seconds) : '') + if (!distance || !time) return t('roadtrip.leg.pending') + return t('roadtrip.leg.driveText', { distance, time }) +} + /** * The drive between two stops. 12px rather than the desktop's 10: a four-hour leg * deserves the space it takes. @@ -171,9 +216,7 @@ export function RtLegRow({ seg, mode, chrome, onAlternatives, alternativesOpen = }) { const { t } = chrome const Icon = mode && LEG_ICONS[mode] ? LEG_ICONS[mode] : mode?.startsWith('plugin:') ? Zap : CarFront - const text = seg - ? t('roadtrip.leg.driveText', { distance: seg.distanceText, time: seg.durationText ?? seg.drivingText }) - : t('roadtrip.leg.pending') + const text = legText(seg, chrome) return (
{/* flex-col, not flex: in a row the dashes would stretch sideways and read as diff --git a/client/src/mobile/screens/trip/roadtrip/MRoadtripTab.tsx b/client/src/mobile/screens/trip/roadtrip/MRoadtripTab.tsx index 1e9854a0d..c3314e1e7 100644 --- a/client/src/mobile/screens/trip/roadtrip/MRoadtripTab.tsx +++ b/client/src/mobile/screens/trip/roadtrip/MRoadtripTab.tsx @@ -62,6 +62,10 @@ export default function MRoadtripTab({ planner, shell }: MTripTabPanelProps) { const stage = rt.stage + // The disc is only a control for somebody who may change a place. Read here rather than + // inside the row so the whole chain asks once, the way `rt` and `chrome` are handled. + const canEditPlaces = planner.can('place_edit', planner.trip) + const openStop = (row: StopRow) => { shell.openSheet('rtstop', { dayId: row.stop.ownerDayId, assignmentId: row.stop.assignmentId }) } @@ -207,7 +211,23 @@ export default function MRoadtripTab({ planner, shell }: MTripTabPanelProps) {
{rt.rows.map((row, i) => { - if (row.kind === 'stop') return openStop(row)} /> + if (row.kind === 'stop') { + return ( + openStop(row)} + onPickKind={canEditPlaces + ? () => shell.openSheet('rtkind', { + placeId: row.stop.placeId, + stopType: row.stop.stopType ?? null, + name: row.stop.name, + }) + : undefined} + /> + ) + } if (row.kind === 'leg') { // Only where the desk rail offers it too (legReroutable), and with the // card's day id, the one the desk passes: the planner finds the day each diff --git a/client/src/mobile/screens/trip/roadtrip/MRtKindSheet.tsx b/client/src/mobile/screens/trip/roadtrip/MRtKindSheet.tsx new file mode 100644 index 000000000..44b0d389f --- /dev/null +++ b/client/src/mobile/screens/trip/roadtrip/MRtKindSheet.tsx @@ -0,0 +1,149 @@ +import { useRef, useState } from 'react' +import { MapPin } from 'lucide-react' +import MSheet from '../../../components/MSheet' +import type { MTripSheetsProps } from '../MTripShell' +import { useTranslation } from '../../../../i18n' +import { STOP_KINDS } from '../../../../components/Roadtrip/stopKinds' +import { FormSheetHeader } from '../sheets/PlSheetChrome' +import type { RoadtripStopType } from '@trek/shared' + +/** What the chain hands over when a stop's disc is tapped. */ +interface RtKindSheetPayload { + placeId?: number + /** What the stop is now: a service kind, or null for a destination. */ + stopType?: string | null + name?: string +} + +/** + * Turning a place on the drive into a pause, and back, on the phone ('rtkind', payload + * { placeId, stopType, name }). + * + * Opened from the stop's own disc in the chain, which is where the desktop opens it from + * and for the same reason: the disc is exactly what changes. A destination wears its + * running number, a service stop wears its kind's icon in that kind's colour, so the + * control is its own preview. Tap the 3, pick the pump, and the 3 becomes an orange disc + * while everything below it renumbers. + * + * A sheet rather than the desktop's popover under the disc: a popover is aimed at an + * anchor with a mouse, and the same palette on a phone would land under the thumb that + * opened it. The kinds stay a palette of discs rather than a list of words, as they are + * on the desktop. At this size the colour and the shape are the label, and seven rows of + * text for seven icons would be a menu where a palette does. Each disc still carries its + * name for a screen reader. + * + * It writes through `planner.setRoadtripStopKind`, the very call the desktop rail makes: + * one field on one place, with the chain redrawing off the store as soon as it lands. + */ +export default function MRtKindSheet({ planner, shell }: MTripSheetsProps) { + const { t } = useTranslation() + const open = shell.sheet?.id === 'rtkind' + const payload = (shell.sheet?.payload ?? {}) as RtKindSheetPayload + const canEdit = planner.can('place_edit', planner.trip) + + // Hold the last payload so the name and the current kind survive the exit animation + // instead of blinking out while the sheet slides away. + const heldRef = useRef(null) + if (open && typeof payload.placeId === 'number') heldRef.current = payload + const stop = open && typeof payload.placeId === 'number' ? payload : heldRef.current + + const [saving, setSaving] = useState(false) + const current = stop?.stopType ?? null + + /** + * Writes the kind and closes. + * + * Closing rather than reopening the stop sheet: the answer to "what kind is this" is + * the disc in the chain, which is already on screen behind this sheet and redraws the + * moment the store lands. `setRoadtripStopKind` reports its own failures and resolves + * either way, so what the guard buys is a sheet that does not sit open pretending to + * save. + */ + const pick = async (kind: RoadtripStopType | null) => { + const placeId = stop?.placeId + if (!canEdit || saving || placeId == null) return + setSaving(true) + try { + await planner.setRoadtripStopKind(placeId, kind) + shell.closeSheet() + } catch { + // Swallowed rather than toasted again: `setRoadtripStopKind` catches its own + // failures and shows the message itself, so a second one here would be the same + // news twice. What this catch is for is the sheet, which stays open on a write that + // never landed instead of closing as if it had. + } finally { + setSaving(false) + } + } + + return ( + + {stop && ( + <> + + +
+ {stop.name && ( +
{stop.name}
+ )} + + {/* Wrapped rather than scrolled sideways: seven discs do not fit one phone row, + and a row that scrolls hides the kinds past its edge behind a gesture. */} +
+ {STOP_KINDS.map(({ key, labelKey, Icon, color }) => { + const on = current === key + return ( + + ) + })} +
+ + {/* Only once there is something to undo. On a destination it would be a button + saying "leave everything as it is". */} + {current && ( + + )} +
+ + )} + + ) +} diff --git a/client/src/mobile/screens/trip/sheets/MTripSheets.tsx b/client/src/mobile/screens/trip/sheets/MTripSheets.tsx index a7b74a63e..6c304c84c 100644 --- a/client/src/mobile/screens/trip/sheets/MTripSheets.tsx +++ b/client/src/mobile/screens/trip/sheets/MTripSheets.tsx @@ -25,6 +25,7 @@ import MExportSheet from './MExportSheet' import MMehrSheet from './MMehrSheet' import MRtStopSheet from '../roadtrip/MRtStopSheet' import MRtStaySheet from '../roadtrip/MRtStaySheet' +import MRtKindSheet from '../roadtrip/MRtKindSheet' import MRtInfoSheet from '../roadtrip/MRtInfoSheet' import MRtCorridorSheet from '../roadtrip/MRtCorridorSheet' import MRtDraftSheet from '../roadtrip/MRtDraftSheet' @@ -72,6 +73,7 @@ export default function MTripSheets({ planner, shell }: MTripSheetsProps) { sheet hangs off planner.stopDraft the way the place editor hangs off its flag. */} + {/* Before the draft sheet, not after: both sit at the same z, so the one mounted later paints on top, and taking a hit onto the trip opens the draft OVER the diff --git a/client/tests/unit/mobile/trip/MRoadtripRows.test.tsx b/client/tests/unit/mobile/trip/MRoadtripRows.test.tsx index 1d17d098a..c5972cc3b 100644 --- a/client/tests/unit/mobile/trip/MRoadtripRows.test.tsx +++ b/client/tests/unit/mobile/trip/MRoadtripRows.test.tsx @@ -9,7 +9,7 @@ import type { TranslationFn } from '../../../../src/types' import type { RefuelSearch } from '../../../../src/components/Roadtrip/useRefuelSearch' import type { RefuelCandidate } from '../../../../src/components/Roadtrip/refuelSuggestion' -// FE-MOB-RTROW-001 to FE-MOB-RTROW-040 +// FE-MOB-RTROW-001 to FE-MOB-RTROW-046 // Same echo strategy as tests/helpers/mobileTrip: assertions stay on keys, not copy. const t: TranslationFn = (key, params) => @@ -168,6 +168,52 @@ describe('RtStopRow', () => { expect(screen.getByText('35 min')).toBeInTheDocument() }) + // The disc as the control that switches a stop between a destination and a pause, the + // way the desktop rail's own disc does. + it('FE-MOB-RTROW-041: the disc is a button that asks for the kind, and the tap stops there', () => { + const onOpen = vi.fn() + const onPickKind = vi.fn() + render() + + const disc = screen.getByRole('button', { name: 'roadtrip.stop.makeService' }) + expect(disc).toHaveTextContent('2') + + fireEvent.click(disc) + + expect(onPickKind).toHaveBeenCalledTimes(1) + // The row opens the stop; a tap meant for the disc must not do both. + expect(onOpen).not.toHaveBeenCalled() + }) + + it('FE-MOB-RTROW-042: the disc of a service stop asks the same question, named for what it already is', () => { + const onPickKind = vi.fn() + render( + , + ) + + const disc = screen.getByRole('button', { name: 'roadtrip.stop.kind' }) + // Still the kind's own symbol: the control is its own preview of what it changes. + expect(disc.querySelector('.lucide-fuel')).not.toBeNull() + + fireEvent.click(disc) + expect(onPickKind).toHaveBeenCalledTimes(1) + }) + + it('FE-MOB-RTROW-043: without the handler the disc is not a button at all', () => { + // A traveller who may not edit places gets no handler, and a disabled-looking control + // for something they cannot do is worse than no control. + render() + + expect(screen.queryByRole('button', { name: 'roadtrip.stop.makeService' })).toBeNull() + expect(screen.queryByRole('button', { name: 'roadtrip.stop.kind' })).toBeNull() + expect(screen.getByText('2')).toBeInTheDocument() + }) + it('FE-MOB-RTROW-013: writes an overnight warning as a word, since it has no figure', () => { const warning: ScheduleWarning = { index: 1, code: 'overnight' } const { container } = render() @@ -204,6 +250,49 @@ describe('RtLegRow', () => { expect(screen.getByText('roadtrip.leg.pending')).toBeInTheDocument() }) + it('FE-MOB-RTROW-044: a segment with no texts yet is read off its numbers, the way the desktop rail reads it', () => { + // A segment can reach the chain before its routing round has landed: metres and + // seconds are there, the pre-formatted texts are not. + render( + , + ) + + expect(screen.getByText('roadtrip.leg.driveText:210 km,2 h 40 min')).toBeInTheDocument() + }) + + it('FE-MOB-RTROW-045: a segment with neither texts nor numbers says pending, never a bare separator', () => { + // This printed the template with both slots empty, so the pill read " in " and said + // nothing at all. A leg that is not routed says so. + render( + , + ) + + expect(screen.getByText('roadtrip.leg.pending')).toBeInTheDocument() + expect(screen.queryByText(/driveText/)).toBeNull() + }) + + it('FE-MOB-RTROW-046: half a segment is pending too, rather than a figure with a hole beside it', () => { + // A distance with no time would read "210 km in", which is worse than saying nothing: + // it looks like a sentence that was cut off. + render( + , + ) + + expect(screen.getByText('roadtrip.leg.pending')).toBeInTheDocument() + }) + it('FE-MOB-RTROW-017: swaps the icon for a walked leg and for a plugin mode', () => { const walk = render() expect(walk.container.querySelector('.lucide-footprints')).not.toBeNull() diff --git a/client/tests/unit/mobile/trip/MRoadtripTab.test.tsx b/client/tests/unit/mobile/trip/MRoadtripTab.test.tsx index 7417d35ea..e5119f827 100644 --- a/client/tests/unit/mobile/trip/MRoadtripTab.test.tsx +++ b/client/tests/unit/mobile/trip/MRoadtripTab.test.tsx @@ -230,6 +230,28 @@ describe('MRoadtripTab', () => { expect(shell.openSheet).toHaveBeenCalledWith('rtstop', { dayId: 2, assignmentId: 503 }) }) + it('FE-MOB-RTTAB-051: the disc asks which kind of stop it is, without opening the stop', () => { + const { shell } = renderTab() + + // Kyoto Station is a destination, so its disc offers to make it a stop on the way. + fireEvent.click(screen.getAllByRole('button', { name: 'roadtrip.stop.makeService' })[1]) + + expect(shell.openSheet).toHaveBeenCalledWith('rtkind', { + placeId: 103, + stopType: null, + name: 'Kyoto Station', + }) + // The row's own tap opens the stop; the disc's must not do both. + expect(shell.openSheet).toHaveBeenCalledTimes(1) + }) + + it('FE-MOB-RTTAB-052: a traveller who may not edit places gets a disc that is not a control', () => { + renderTab(planner({ can: () => false })) + + expect(screen.queryByRole('button', { name: 'roadtrip.stop.makeService' })).toBeNull() + expect(screen.queryByRole('button', { name: 'roadtrip.stop.kind' })).toBeNull() + }) + it('FE-MOB-RTTAB-006: writes the day warning out with the sentence the desktop hides in a tooltip', () => { const over = stage({ dayWarning: { code: 'dayDriving', minutes: 620, limitMinutes: 540 } }) renderTab(planner({ roadtripRoutes: routes({ days: [over] }) })) diff --git a/client/tests/unit/mobile/trip/MRtKindSheet.test.tsx b/client/tests/unit/mobile/trip/MRtKindSheet.test.tsx new file mode 100644 index 000000000..b76a87e41 --- /dev/null +++ b/client/tests/unit/mobile/trip/MRtKindSheet.test.tsx @@ -0,0 +1,122 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import MRtKindSheet from '../../../../src/mobile/screens/trip/roadtrip/MRtKindSheet' +import type { MTripShellApi, TripPlanner } from '../../../../src/mobile/screens/trip/MTripShell' +import { buildPlanner, buildShell } from '../../../helpers/mobileTrip' +import { resetAllStores } from '../../../helpers/store' +import { fireEvent, render, screen, waitFor } from '../../../helpers/render' + +// FE-MOB-RTKIND-001 to FE-MOB-RTKIND-008 +// +// The sheet renders inside the real TranslationProvider, so the copy is asserted in +// English. It writes through the very call the desktop rail makes, so what these hold is +// the phone's half: which stop, which kind, and that a second tap undoes it. + +function makePlanner(overrides: Record = {}) { + return buildPlanner({ + tripId: 4, + setRoadtripStopKind: vi.fn(async () => undefined), + ...overrides, + } as unknown as Partial) +} + +function makeShell(overrides: Record = {}) { + return buildShell({ + sheet: { id: 'rtkind', payload: { placeId: 202, stopType: null, name: 'Bremen Marktplatz' } }, + ...overrides, + } as unknown as Partial) +} + +function renderSheet(plannerOverrides: Record = {}, shellOverrides: Record = {}) { + const planner = makePlanner(plannerOverrides) + const shell = makeShell(shellOverrides) + render() + return { planner, shell } +} + +describe('MRtKindSheet', () => { + beforeEach(() => { + resetAllStores() + }) + + it('FE-MOB-RTKIND-001: stays closed while another sheet id is active', () => { + renderSheet({}, { sheet: { id: 'rtstop', payload: { placeId: 202 } } }) + expect(screen.queryByRole('dialog')).not.toBeInTheDocument() + }) + + it('FE-MOB-RTKIND-002: opens on the stop it was asked about, with every kind offered', () => { + renderSheet() + + expect(screen.getByRole('dialog', { name: 'Kind of stop' })).toBeInTheDocument() + expect(screen.getByText('Bremen Marktplatz')).toBeInTheDocument() + // The seven the desktop palette offers, each named for a screen reader although the + // disc itself is the label on screen. + for (const name of ['Accommodation', 'Fuel', 'Charging', 'Rest area', 'Campsite', 'Food', 'Sights']) { + expect(screen.getByRole('button', { name, pressed: false })).toBeInTheDocument() + } + }) + + it('FE-MOB-RTKIND-003: picking a kind writes it to the place and closes', async () => { + const { planner, shell } = renderSheet() + + fireEvent.click(screen.getByRole('button', { name: 'Fuel' })) + + await waitFor(() => expect(planner.setRoadtripStopKind).toHaveBeenCalledWith(202, 'fuel')) + await waitFor(() => expect(shell.closeSheet).toHaveBeenCalledTimes(1)) + }) + + it('FE-MOB-RTKIND-004: the kind a stop already is shows as chosen, and tapping it again takes it back', async () => { + const { planner } = renderSheet({}, { + sheet: { id: 'rtkind', payload: { placeId: 202, stopType: 'fuel', name: 'Bremen Marktplatz' } }, + }) + + expect(screen.getByRole('button', { name: 'Fuel', pressed: true })).toBeInTheDocument() + + fireEvent.click(screen.getByRole('button', { name: 'Fuel' })) + + // Null is what turns a pause back into a destination, and it is the same write. + await waitFor(() => expect(planner.setRoadtripStopKind).toHaveBeenCalledWith(202, null)) + }) + + it('FE-MOB-RTKIND-005: the way back is offered only once there is something to undo', async () => { + renderSheet() + // On a destination it would be a button saying "leave everything as it is". + expect(screen.queryByRole('button', { name: 'Back to a destination' })).toBeNull() + + const { planner } = renderSheet({}, { + sheet: { id: 'rtkind', payload: { placeId: 202, stopType: 'rest_area', name: 'Bremen Marktplatz' } }, + }) + + fireEvent.click(screen.getAllByRole('button', { name: 'Back to a destination' })[0]) + + await waitFor(() => expect(planner.setRoadtripStopKind).toHaveBeenCalledWith(202, null)) + }) + + it('FE-MOB-RTKIND-006: a traveller who may not edit places can open it but change nothing', () => { + const { planner } = renderSheet({ can: () => false }) + + for (const name of ['Fuel', 'Accommodation']) { + expect(screen.getByRole('button', { name })).toBeDisabled() + } + + fireEvent.click(screen.getByRole('button', { name: 'Fuel' })) + expect(planner.setRoadtripStopKind).not.toHaveBeenCalled() + }) + + it('FE-MOB-RTKIND-007: a payload without a place is not a sheet', () => { + renderSheet({}, { sheet: { id: 'rtkind', payload: {} } }) + expect(screen.queryByRole('dialog')).not.toBeInTheDocument() + }) + + it('FE-MOB-RTKIND-008: a failing write leaves the sheet open rather than claiming it saved', async () => { + const { shell } = renderSheet({ + setRoadtripStopKind: vi.fn(async () => { throw new Error('offline') }), + }) + + fireEvent.click(screen.getByRole('button', { name: 'Fuel' })) + + // The planner's own call reports the failure; what matters here is that the sheet does + // not close on a write that never landed. + await waitFor(() => expect(screen.getByRole('dialog', { name: 'Kind of stop' })).toBeInTheDocument()) + expect(shell.closeSheet).not.toHaveBeenCalled() + }) +})