mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-23 23:46:35 +00:00
@
fix(export): print the reader clock format, and split window bookings in the feed Three v4.0.0 reports that all come down to a surface printing what is stored instead of what the reader asked for. The PDF (#2066) is assembled as an HTML string outside React, so no hook could reach it and no time format was ever passed in. It printed the column: four clock sites, all 24h whatever the personal setting or the admin default said, and a place time typed by a 12h user came out as "3:00 PM" for a 24h reader. Both export entry points now hand the format over the way they already hand over the locale, and the four sites go through formatClockTime. The journey editors (#2067) used a native <input type="time">, which paints 12h or 24h from the browser locale and cannot be told otherwise by any attribute. Every other time field in TREK has used CustomTimePicker for exactly that reason; the journey editor, its phone sheet and the phone place sheet were never migrated, so the same place form obeyed the setting on desktop and ignored it on a phone. The picker gained a disabled prop so the read-only journey sheet keeps the affordance the native input gave it. In the subscribed calendar (#2068), a booking that is a window rather than an activity read as one block across its whole range: a week of airport parking sat on all seven days. Both hand-overs now stand on their own — an hour each, carrying what the block used to say — and the block goes. Only when the two ends fall on different days: a same-day booking is one sitting, and a rental with only its pickup geocoded keeps the block, which is the only thing holding the return time and its zone at all. Rental cars were missing outright, because the planner writes reservation_time = NULL when its optional time pickers are left blank while still setting day_id, and nothing in the exporter ever read the linked days. It reads them now, as the last resort, for every type — so a hand-typed bus or train shows up as well. @
This commit is contained in:
@@ -5,6 +5,9 @@ import { http, HttpResponse, delay } from 'msw'
|
||||
import { localIsoDate } from '../../utils/localDate'
|
||||
import userEvent from '@testing-library/user-event'
|
||||
import { render, screen, waitFor, fireEvent } from '../../../tests/helpers/render'
|
||||
import { seedStore } from '../../../tests/helpers/store'
|
||||
import { buildSettings } from '../../../tests/helpers/factories'
|
||||
import { useSettingsStore } from '../../store/settingsStore'
|
||||
import { server } from '../../../tests/helpers/msw/server'
|
||||
import type { GalleryPhoto, JourneyEntry, JourneyPhoto, JourneyTrip } from '../../store/journeyStore'
|
||||
import type { ResilientResult, UploadProgress } from '../../utils/uploadQueue'
|
||||
@@ -92,6 +95,9 @@ const originalCreateObjectURL = URL.createObjectURL
|
||||
|
||||
beforeEach(() => {
|
||||
toastSpy.mockClear()
|
||||
// The time field reads time_format, so pin it: without this the cases that
|
||||
// assert a 24h string depend on whichever test ran before them (#2067).
|
||||
seedStore(useSettingsStore, { settings: buildSettings({ time_format: '24h' }) })
|
||||
window.__addToast = toastSpy
|
||||
Object.defineProperty(URL, 'createObjectURL', {
|
||||
configurable: true, writable: true, value: vi.fn(() => 'blob:preview'),
|
||||
@@ -865,9 +871,8 @@ describe('EntryEditor', () => {
|
||||
const user = userEvent.setup()
|
||||
const { onSave } = mountEditor(buildEntry({ id: 10, title: 'Old', entry_time: '14:30:00' }))
|
||||
|
||||
// The column carries HH:MM:SS; a time input only accepts HH:MM.
|
||||
// The column carries HH:MM:SS; the picker is seeded from a HH:MM slice.
|
||||
const timeInput = screen.getByDisplayValue('14:30')
|
||||
expect(timeInput).toHaveAttribute('type', 'time')
|
||||
|
||||
fireEvent.change(timeInput, { target: { value: '09:05' } })
|
||||
await user.click(screen.getByRole('button', { name: 'Save' }))
|
||||
@@ -876,6 +881,40 @@ describe('EntryEditor', () => {
|
||||
expect(onSave.mock.calls[0][0]).toEqual(expect.objectContaining({ entry_time: '09:05' }))
|
||||
})
|
||||
|
||||
// #2067 — the field used to be a native <input type="time">, which paints 12h or
|
||||
// 24h from the browser locale and cannot be told otherwise. These pin that the
|
||||
// user's setting decides what is shown, and that storage stays 24h either way.
|
||||
it('FE-JRN-EDITOR-048: a 24h user sees a 24h clock with no meridiem', () => {
|
||||
seedStore(useSettingsStore, { settings: buildSettings({ time_format: '24h' }) })
|
||||
const { container } = mountEditor(buildEntry({ id: 10, title: 'Old', entry_time: '14:30:00' }))
|
||||
|
||||
expect(screen.getByDisplayValue('14:30')).toBeInTheDocument()
|
||||
expect(container.textContent).not.toMatch(/\bPM\b/)
|
||||
})
|
||||
|
||||
it('FE-JRN-EDITOR-049: a 12h user sees the same stored time as a meridiem clock', () => {
|
||||
seedStore(useSettingsStore, { settings: buildSettings({ time_format: '12h' }) })
|
||||
mountEditor(buildEntry({ id: 10, title: 'Old', entry_time: '14:30:00' }))
|
||||
|
||||
expect(screen.getByDisplayValue('2:30 PM')).toBeInTheDocument()
|
||||
expect(screen.queryByDisplayValue('14:30')).not.toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('FE-JRN-EDITOR-050: a meridiem typed by a 12h user is still stored as 24h', async () => {
|
||||
const user = userEvent.setup()
|
||||
seedStore(useSettingsStore, { settings: buildSettings({ time_format: '12h' }) })
|
||||
const { onSave } = mountEditor(buildEntry({ id: 10, title: 'Old', entry_time: '14:30:00' }))
|
||||
|
||||
const field = screen.getByDisplayValue('2:30 PM')
|
||||
fireEvent.focus(field)
|
||||
fireEvent.change(field, { target: { value: '5:30 pm' } })
|
||||
fireEvent.blur(field)
|
||||
await user.click(screen.getByRole('button', { name: 'Save' }))
|
||||
|
||||
await waitFor(() => expect(onSave).toHaveBeenCalled())
|
||||
expect(onSave.mock.calls[0][0]).toEqual(expect.objectContaining({ entry_time: '17:30' }))
|
||||
})
|
||||
|
||||
it('FE-JRN-EDITOR-042: clearing the time sends null rather than an empty string', async () => {
|
||||
const user = userEvent.setup()
|
||||
const { onSave } = mountEditor(buildEntry({ id: 10, entry_time: '14:30:00' }))
|
||||
|
||||
@@ -13,6 +13,7 @@ import { MOOD_CONFIG, WEATHER_CONFIG } from '../../pages/journeyDetail/JourneyDe
|
||||
import { photoUrl, isValidGeoPoint, geoOnceErrorKey } from '../../pages/journeyDetail/JourneyDetailPage.helpers'
|
||||
import MarkdownToolbar from './MarkdownToolbar'
|
||||
import { DatePicker } from './JourneyDetailPageDatePicker'
|
||||
import CustomTimePicker from '../shared/CustomTimePicker'
|
||||
import { ProviderPicker, type ProviderPhotoGroup } from './JourneyDetailPageProviderPicker'
|
||||
|
||||
type PendingProviderGroup = ProviderPhotoGroup & { provider: string }
|
||||
@@ -601,19 +602,20 @@ export function EntryEditor({ entry, journeyId, tripDates, galleryPhotos, trips,
|
||||
{/* Time sat in state and went to the server, it just had no input here — so a
|
||||
draft's auto-stamped clock time showed up in the timeline, the map, the PDF
|
||||
and the public share, and the desktop had no way to correct it (#1614). */}
|
||||
<div className="grid grid-cols-[1fr_104px] gap-2">
|
||||
{/* 136px, not 104: the custom picker adds a clock button and, in 12h,
|
||||
shows "2:30 PM" where the native input showed a fixed-width HH:MM. */}
|
||||
<div className="grid grid-cols-[1fr_136px] gap-2">
|
||||
<div>
|
||||
<label className="text-[10px] font-semibold tracking-[0.12em] uppercase text-zinc-500 block mb-1.5">{t('journey.editor.date')}</label>
|
||||
<DatePicker value={entryDate} onChange={setEntryDate} tripDates={tripDates} />
|
||||
</div>
|
||||
<div>
|
||||
<label className="text-[10px] font-semibold tracking-[0.12em] uppercase text-zinc-500 block mb-1.5">{t('mobileJourney.time')}</label>
|
||||
<input
|
||||
type="time"
|
||||
value={entryTime}
|
||||
onChange={e => setEntryTime(e.target.value)}
|
||||
className="w-full h-[42px] px-3 rounded-xl border border-zinc-200 dark:border-zinc-700 bg-transparent text-[13px] text-zinc-900 dark:text-white outline-none focus:border-zinc-400 dark:focus:border-zinc-500 [font-variant-numeric:tabular-nums]"
|
||||
/>
|
||||
{/* A native <input type="time"> paints 12h or 24h from the browser
|
||||
locale, and no attribute overrides it — so it ignored the user's
|
||||
setting outright (#2067). The rest of TREK has used this picker
|
||||
for exactly that reason; the journey editor was never migrated. */}
|
||||
<CustomTimePicker value={entryTime} onChange={setEntryTime} />
|
||||
</div>
|
||||
</div>
|
||||
<div className="relative">
|
||||
|
||||
@@ -578,6 +578,73 @@ describe('downloadTripPDF remaining branches', () => {
|
||||
|
||||
const srcdoc = () => getIframe()!.srcdoc
|
||||
|
||||
// #2066 — the document is assembled outside React, so it read no setting at all
|
||||
// and printed the stored column. Four surfaces carried a clock; all four were 24h
|
||||
// whatever the reader had chosen.
|
||||
describe('time format (#2066)', () => {
|
||||
// The place chip reads the assignment's embedded place, not the places array,
|
||||
// so both have to carry the clock under test.
|
||||
const at = (placeTime: string, over: Record<string, unknown> = {}) => {
|
||||
const place = { ...placeWithDetails, place_time: placeTime }
|
||||
return {
|
||||
...richArgs,
|
||||
places: [place],
|
||||
assignments: { '10': [{ ...assignmentForDay, place }] },
|
||||
reservations: [{
|
||||
id: 700, title: 'Ferry', type: 'ferry', day_id: 10,
|
||||
reservation_time: '2025-06-01T09:05', reservation_end_time: '2025-06-01T16:45',
|
||||
}],
|
||||
...over,
|
||||
}
|
||||
}
|
||||
|
||||
it('FE-W5PDF-031: a 12h reader gets meridiem clocks on places and transports', async () => {
|
||||
await downloadTripPDF(at('14:30', { timeFormat: '12h' }) as never)
|
||||
const html = srcdoc()
|
||||
|
||||
expect(html).toContain('2:30 PM')
|
||||
expect(html).toContain('9:05 AM')
|
||||
expect(html).toContain('4:45 PM')
|
||||
expect(html).not.toContain('14:30')
|
||||
expect(html).not.toContain('16:45')
|
||||
})
|
||||
|
||||
it('FE-W5PDF-032: a 24h reader gets the same times without a meridiem', async () => {
|
||||
await downloadTripPDF(at('14:30', { timeFormat: '24h' }) as never)
|
||||
const html = srcdoc()
|
||||
|
||||
expect(html).toContain('14:30')
|
||||
expect(html).toContain('09:05')
|
||||
expect(html).toContain('16:45')
|
||||
expect(html).not.toContain('2:30 PM')
|
||||
})
|
||||
|
||||
// check_in / check_out come off the accommodation row and were the one pair
|
||||
// that printed the raw column in BOTH directions.
|
||||
it('FE-W5PDF-033: accommodation check-in and check-out follow the setting too', async () => {
|
||||
server.use(http.get('/api/trips/:id/accommodations', () => HttpResponse.json({
|
||||
accommodations: [{
|
||||
id: 1, place_id: 1, place_name: 'Hotel Roma', place_address: 'Via Roma 1',
|
||||
start_day_id: 10, end_day_id: 10, check_in: '15:00', check_out: '11:00', notes: null,
|
||||
}],
|
||||
})))
|
||||
|
||||
await downloadTripPDF(at('14:30', { timeFormat: '12h' }) as never)
|
||||
|
||||
expect(srcdoc()).toContain('3:00 PM')
|
||||
})
|
||||
|
||||
// A clock stored with a meridiem — the booking importer and a 12h user typing
|
||||
// into the place form both produce one — must not print as 3 AM for a 24h reader.
|
||||
it('FE-W5PDF-034: a stored meridiem is converted, not printed raw', async () => {
|
||||
await downloadTripPDF(at('3:00 PM', { timeFormat: '24h' }) as never)
|
||||
|
||||
const html = srcdoc()
|
||||
expect(html).toContain('15:00')
|
||||
expect(html).not.toContain('3:00 PM')
|
||||
})
|
||||
})
|
||||
|
||||
it('FE-W5PDF-001: a multi-day cruise is labelled start / ongoing / end with the right times', async () => {
|
||||
await downloadTripPDF(spanArgs([{
|
||||
id: 500, title: 'Nordic Cruise', type: 'cruise', day_id: 10, end_day_id: 12,
|
||||
|
||||
@@ -8,7 +8,8 @@ import { isDayInAccommodationRange, getDayOrder } from '../../utils/dayOrder'
|
||||
import { hidesOnMiddleDay, getTransportForDay, getMergedItems, getSpanPhase, getDisplayTimeForDay } from '../../utils/dayMerge'
|
||||
import { safeHexColor } from '../../utils/safeColor'
|
||||
import { renderIconMarkup } from '../../utils/iconMarkup'
|
||||
import { formatMoney, formatMoneySum, splitReservationDateTime, type MoneyEntry } from '../../utils/formatters'
|
||||
import { formatMoney, formatMoneySum, formatClockTime, splitReservationDateTime, type MoneyEntry } from '../../utils/formatters'
|
||||
import { useSettingsStore } from '../../store/settingsStore'
|
||||
import { fetchExchangeRates } from '../../hooks/useExchangeRates'
|
||||
import { getFlightLegs, getTrainLegs } from '../../utils/flightLegs'
|
||||
|
||||
@@ -178,14 +179,24 @@ interface downloadTripPDFProps {
|
||||
reservations?: any[]
|
||||
t: (key: string, params?: Record<string, string | number>) => string
|
||||
locale: string
|
||||
/**
|
||||
* '12h' | '24h'. The document is a plain HTML string assembled outside React,
|
||||
* so the setting cannot be read with a hook in here — it comes over the same
|
||||
* way `locale` does (#2066).
|
||||
*/
|
||||
timeFormat?: string
|
||||
}
|
||||
|
||||
// `assignments` is normalised here once — every read below (and fetchPlacePhotos)
|
||||
// relies on it being an object.
|
||||
export async function downloadTripPDF({ trip, days, places, assignments = {}, categories, dayNotes, reservations = [], t: _t, locale: _locale }: downloadTripPDFProps) {
|
||||
export async function downloadTripPDF({ trip, days, places, assignments = {}, categories, dayNotes, reservations = [], t: _t, locale: _locale, timeFormat: _timeFormat }: downloadTripPDFProps) {
|
||||
const breaksPerDay = pageBreakPerDay()
|
||||
const loc = _locale || undefined
|
||||
const tr = _t || (k => k)
|
||||
// The store read is the fallback, not the source: a caller that forgets the
|
||||
// prop still prints the reader's own format instead of silently reverting.
|
||||
const is12h = (_timeFormat || useSettingsStore.getState().settings.time_format || '24h') === '12h'
|
||||
const fmtTime = (v?: string | null) => formatClockTime(v, is12h)
|
||||
const sorted = [...(days || [])].sort((a, b) => a.day_number - b.day_number)
|
||||
const range = longDateRange(sorted, loc)
|
||||
const coverImg = safeImg(trip?.cover_image)
|
||||
@@ -358,8 +369,8 @@ export async function downloadTripPDF({ trip, days, places, assignments = {}, ca
|
||||
// span the arrival belongs to the arrival day, which already shows
|
||||
// it as its own time, and repeating it here would put tomorrow's
|
||||
// clock next to today's departure.
|
||||
const startTime = splitReservationDateTime(displayTime).time ?? ''
|
||||
const endTime = phase === 'single' ? (splitReservationDateTime(r.reservation_end_time).time ?? '') : ''
|
||||
const startTime = fmtTime(splitReservationDateTime(displayTime).time)
|
||||
const endTime = phase === 'single' ? fmtTime(splitReservationDateTime(r.reservation_end_time).time) : ''
|
||||
const time = [startTime, endTime].filter(Boolean).join(' – ')
|
||||
const titleHtml = `${spanLabel ? escHtml(spanLabel) + ': ' : ''}${escHtml(r.title)}`
|
||||
return `
|
||||
@@ -412,7 +423,7 @@ export async function downloadTripPDF({ trip, days, places, assignments = {}, ca
|
||||
</div>`
|
||||
|
||||
const chips = [
|
||||
place.place_time ? `<span class="chip">${svgClock}${escHtml(place.place_time)}</span>` : '',
|
||||
place.place_time ? `<span class="chip">${svgClock}${escHtml(fmtTime(place.place_time))}</span>` : '',
|
||||
place.price && Number.parseFloat(place.price) > 0 ? `<span class="chip chip-green">${svgMoney}${formatMoney(Number(place.price), place.currency || trip.currency, loc)}</span>` : '',
|
||||
].filter(Boolean).join('')
|
||||
|
||||
@@ -452,8 +463,8 @@ export async function downloadTripPDF({ trip, days, places, assignments = {}, ca
|
||||
const actionIcon = isCheckIn ? accommodationIconSvg('checkin')
|
||||
: isCheckOut ? accommodationIconSvg('checkout')
|
||||
: accommodationIconSvg('accommodation')
|
||||
const timeStr = isCheckIn ? (item.check_in || '')
|
||||
: isCheckOut ? (item.check_out || '')
|
||||
const timeStr = isCheckIn ? fmtTime(item.check_in)
|
||||
: isCheckOut ? fmtTime(item.check_out)
|
||||
: ''
|
||||
|
||||
return `
|
||||
|
||||
@@ -5,6 +5,7 @@ import Modal from '../shared/Modal'
|
||||
import { IcsSubscribeModal } from './IcsSubscribeModal'
|
||||
import { useToast } from '../shared/Toast'
|
||||
import type { Trip, Day, Place, Category, AssignmentsMap, Reservation, DayNote } from '../../types'
|
||||
import { useSettingsStore } from '../../store/settingsStore'
|
||||
|
||||
/**
|
||||
* What a GPX download can carry. Worded by what someone wants on their device
|
||||
@@ -57,6 +58,8 @@ export function TripExportModal({
|
||||
// Which row is working, so the dialog can say so instead of looking inert
|
||||
// while a 226 kB PDF builder is fetched and a document is rendered.
|
||||
const [busy, setBusy] = useState<string | null>(null)
|
||||
// The PDF is built outside React, so it cannot read this itself (#2066).
|
||||
const timeFormat = useSettingsStore(s => s.settings.time_format) || '24h'
|
||||
const fileBase = trip?.title || 'trip'
|
||||
|
||||
// Shared tail of every download: Firefox and Safari cancel the download when
|
||||
@@ -83,7 +86,7 @@ export function TripExportModal({
|
||||
// exported. A missing chunk lands in the catch and shows the same error
|
||||
// the export already had.
|
||||
const { downloadTripPDF } = await import('../PDF/TripPDF')
|
||||
await downloadTripPDF({ trip, days, places, assignments, categories, dayNotes: flatNotes, reservations, t, locale })
|
||||
await downloadTripPDF({ trip, days, places, assignments, categories, dayNotes: flatNotes, reservations, t, locale, timeFormat })
|
||||
onClose()
|
||||
} catch (e) {
|
||||
console.error('PDF error:', e)
|
||||
|
||||
@@ -10,9 +10,12 @@ interface CustomTimePickerProps {
|
||||
onChange: (value: string) => void
|
||||
placeholder?: string
|
||||
style?: React.CSSProperties
|
||||
/** Read-only surfaces keep the field, they just cannot type in it — same
|
||||
* affordance the native input they replaced offered (#2067). */
|
||||
disabled?: boolean
|
||||
}
|
||||
|
||||
export default function CustomTimePicker({ value, onChange, placeholder = '00:00', style = {} }: CustomTimePickerProps) {
|
||||
export default function CustomTimePicker({ value, onChange, placeholder = '00:00', style = {}, disabled = false }: CustomTimePickerProps) {
|
||||
const is12h = useSettingsStore(s => s.settings.time_format) === '12h'
|
||||
const [open, setOpen] = useState(false)
|
||||
const [inputFocused, setInputFocused] = useState(false)
|
||||
@@ -132,6 +135,7 @@ export default function CustomTimePicker({ value, onChange, placeholder = '00:00
|
||||
onFocus={() => setInputFocused(true)}
|
||||
onBlur={() => { setInputFocused(false); handleBlur() }}
|
||||
placeholder={is12h ? '2:30 PM' : placeholder}
|
||||
disabled={disabled}
|
||||
style={{
|
||||
flex: 1, border: 'none', outline: 'none', background: 'transparent',
|
||||
padding: '8px 10px 8px 14px', fontSize: 'calc(13px * var(--fs-scale-body, 1))', fontFamily: 'inherit',
|
||||
@@ -142,8 +146,9 @@ export default function CustomTimePicker({ value, onChange, placeholder = '00:00
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => setOpen(o => !o)}
|
||||
disabled={disabled}
|
||||
style={{
|
||||
background: 'none', border: 'none', cursor: 'pointer', padding: '8px 10px',
|
||||
background: 'none', border: 'none', cursor: disabled ? 'default' : 'pointer', padding: '8px 10px',
|
||||
display: 'flex', alignItems: 'center', color: 'var(--text-faint)',
|
||||
transition: 'color 0.15s', flexShrink: 0,
|
||||
}}
|
||||
|
||||
Reference in New Issue
Block a user