diff --git a/client/scripts/check-page-pattern.mjs b/client/scripts/check-page-pattern.mjs index 5d7d789c9..05acc51a9 100644 --- a/client/scripts/check-page-pattern.mjs +++ b/client/scripts/check-page-pattern.mjs @@ -2,9 +2,12 @@ // src/pages/PATTERN.md). A *Page.tsx default-export component should wire a // co-located use() hook into JSX — it must not own state/effects itself. // -// We scan only the default-export component body (from `export default function` -// up to the next top-level `function` declaration or EOF), so presentational -// sub-components and helper hooks living in the same file are not flagged. +// We scan the default-export component body (from `export default function` up +// to the next top-level `function` declaration or EOF), and the same span of a +// top-level `*PageDesktop`/`*PageMobile` — since ViewportRoute started picking +// the branch, the default export is a two-line shim and the real container is +// one of those two. Everything else at top level stays exempt: presentational +// sub-components and helper hooks live in these files on purpose. // Context hooks like useTranslation/useParams are fine; the smell is stateful // logic — useState/useReducer/useEffect/useLayoutEffect/useMemo/useCallback/useRef. import { readdirSync, readFileSync } from 'node:fs' @@ -13,23 +16,38 @@ import { fileURLToPath } from 'node:url' const pagesDir = join(dirname(fileURLToPath(import.meta.url)), '..', 'src', 'pages') const BANNED = ['useState', 'useReducer', 'useEffect', 'useLayoutEffect', 'useMemo', 'useCallback', 'useRef'] -const bannedRe = new RegExp(`\\b(${BANNED.join('|')})\\s*\\(`) +// `[<(]` rather than `\(`: a generic call — `useRef<() => void>(null)` — is the +// same thing, and it slipped through for as long as the pattern only knew `(`. +const bannedRe = new RegExp(`\\b(${BANNED.join('|')})\\s*[<(]`) + +// Declarations the widened scan caught that are not this change's to move. +// Listed by the name they declare rather than by line, so an entry dies with +// its declaration instead of drifting onto whatever follows it. The rest of +// each listed file is still checked. +const KNOWN_ESCAPES = { + 'TripPlannerPage.tsx': ['glMap', 'bookingExpense'], +} const violations = [] for (const file of readdirSync(pagesDir)) { if (!file.endsWith('Page.tsx') || file.endsWith('.test.tsx')) continue const src = readFileSync(join(pagesDir, file), 'utf8') const lines = src.split('\n') - const start = lines.findIndex(l => /export default function/.test(l)) - if (start === -1) continue - // The page body ends at the next top-level declaration (a `function` at - // column 0) — everything after that is a sub-component or helper. - let end = lines.length - for (let i = start + 1; i < lines.length; i++) { - if (/^(function |const [A-Z]\w* = )/.test(lines[i])) { end = i; break } - } - for (let i = start; i < end; i++) { - if (bannedRe.test(lines[i])) { + const allowed = KNOWN_ESCAPES[file] ?? [] + const starts = [] + lines.forEach((line, i) => { + if (/export default function/.test(line) || /^function \w+Page(Desktop|Mobile)\b/.test(line)) starts.push(i) + }) + for (const start of starts) { + // The page body ends at the next top-level declaration (a `function` at + // column 0) — everything after that is a sub-component or helper. + let end = lines.length + for (let i = start + 1; i < lines.length; i++) { + if (/^(function |const [A-Z]\w* = )/.test(lines[i])) { end = i; break } + } + for (let i = start; i < end; i++) { + if (!bannedRe.test(lines[i])) continue + if (allowed.some(name => new RegExp(`\\b${name}\\b`).test(lines[i]))) continue violations.push(`${file}:${i + 1} ${lines[i].trim()}`) } } diff --git a/client/src/components/Collections/ListEditorModal.test.tsx b/client/src/components/Collections/ListEditorModal.test.tsx index bac602ebd..71e1907b5 100644 --- a/client/src/components/Collections/ListEditorModal.test.tsx +++ b/client/src/components/Collections/ListEditorModal.test.tsx @@ -172,6 +172,21 @@ describe('ListEditorModal', () => { expect(actions.createCollection).not.toHaveBeenCalled() }) + it('FE-COMP-LISTEDITOR-026: a second Enter while the create is in flight does not make a second list', async () => { + let release: (value: { id: number; owner_id: number; name: string }) => void = () => {} + actions.createCollection.mockReturnValueOnce(new Promise(resolve => { release = resolve })) + const { props } = setup() + const nameInput = screen.getByPlaceholderText('e.g. Tokyo 2025') + fireEvent.change(nameInput, { target: { value: 'Lisbon' } }) + fireEvent.keyDown(nameInput, { key: 'Enter' }) + await waitFor(() => expect(actions.createCollection).toHaveBeenCalledTimes(1)) + fireEvent.keyDown(nameInput, { key: 'Enter' }) + release({ id: 77, owner_id: 1, name: 'Lisbon' }) + await waitFor(() => expect(props.onClose).toHaveBeenCalled()) + expect(actions.createCollection).toHaveBeenCalledTimes(1) + expect(props.onCreated).toHaveBeenCalledTimes(1) + }) + it('FE-COMP-LISTEDITOR-011: picking a swatch changes the saved colour', async () => { setup() fireEvent.change(screen.getByPlaceholderText('e.g. Tokyo 2025'), { target: { value: 'Oslo' } }) diff --git a/client/src/components/Collections/ListEditorModal.tsx b/client/src/components/Collections/ListEditorModal.tsx index 9b0609cb1..ec59dbd46 100644 --- a/client/src/components/Collections/ListEditorModal.tsx +++ b/client/src/components/Collections/ListEditorModal.tsx @@ -122,7 +122,9 @@ export default function ListEditorModal({ target, onClose, onCreated, onRequestD const save = async () => { const trimmed = name.trim() - if (!trimmed) return + // Enter fires while the create is still in flight, and `createdId` is only + // set once it comes back — without this the second one creates a second list. + if (!trimmed || saving) return // Normalise + keep only links with a url; drop blank rows. const cleanLinks = links .map(l => ({ label: l.label?.trim() || undefined, url: normalizeLinkUrl(l.url) })) diff --git a/client/src/components/Studio/StudioTravelPanel.tsx b/client/src/components/Studio/StudioTravelPanel.tsx index 29c0594cb..e3a4c61c2 100644 --- a/client/src/components/Studio/StudioTravelPanel.tsx +++ b/client/src/components/Studio/StudioTravelPanel.tsx @@ -1,3 +1,4 @@ +import { useEffect, useRef } from 'react' import type { BookElement, BookMetric, BookPageSetup, JourneyStats } from '@trek/shared' import { BOOK_METRICS } from '@trek/shared' import { useStudioStore } from '../../store/studioStore' @@ -95,6 +96,19 @@ export function StudioTravelPanel({ const spread = doc?.spreads[active] const single = !!spread && spread.role !== 'inner' + /* + * The road lookups still running when the panel goes away. + * + * fetchRoads walks the legs at a deliberate pace, so a twenty-stop map keeps + * asking a public router for minutes after Studio is closed, and the answer + * lands in whatever document the store holds by then. + */ + const roadJobs = useRef(new Set()) + useEffect(() => () => { + roadJobs.current.forEach(c => c.abort()) + roadJobs.current.clear() + }, []) + const centre = (w: number, h: number) => { const W = single ? page.pageWidth : page.pageWidth * 2 return { x: (W - w) / 2, y: (page.pageHeight - h) / 2, w, h } @@ -304,13 +318,17 @@ export function StudioTravelPanel({ * free, and a failure leaves the leg exactly as it was. */ if (placed.kind === 'map' && placed.points.length > 1) { - void fetchRoads(placed.points.map(pt => ({ lat: pt.lat, lng: pt.lng }))) + const job = new AbortController() + roadJobs.current.add(job) + void fetchRoads(placed.points.map(pt => ({ lat: pt.lat, lng: pt.lng })), { signal: job.signal }) .then(roads => { + if (job.signal.aborted) return if (roads.some(r => r && r.length > 1)) { updateElement(active, placed.id, { roads } as Partial) } }) .catch(() => { /* No roads is a state the map already draws. */ }) + .finally(() => { roadJobs.current.delete(job) }) } } diff --git a/client/src/components/Studio/applyTemplate.ts b/client/src/components/Studio/applyTemplate.ts index 0472dba46..307079514 100644 --- a/client/src/components/Studio/applyTemplate.ts +++ b/client/src/components/Studio/applyTemplate.ts @@ -221,7 +221,7 @@ function fillBadge(el: BookElement & { kind: 'badge' }, entry: TemplateEntry, ct } } if (el.variant === 'distance' && ctx.stats?.distance) { - el.text = `${Math.round(ctx.stats.distance / 1000).toLocaleString('en-US')} km` + el.text = `${Math.round(ctx.stats.distance / 1000).toLocaleString(ctx.locale)} km` } } diff --git a/client/src/components/Studio/autoLayout.ts b/client/src/components/Studio/autoLayout.ts index 5d1889c13..15c45fc84 100644 --- a/client/src/components/Studio/autoLayout.ts +++ b/client/src/components/Studio/autoLayout.ts @@ -1369,7 +1369,7 @@ function summarySpread(input: AutoInput): BookSpread | null { els.push(badgeEl( { x: rx + 8 + lineW * 0.5 - 17, y: y - 11, w: 34, h: 8 }, 'distance', - { text: formatKm(stats.furthest) }, + { text: formatKm(stats.furthest, input.locale) }, { style: 'chip', textScale: 0.7 }, )) els.push(text({ x: rx + 4, y: y + 4, w: lineW * 0.5, h: 5 }, (home.label || '').toUpperCase(), { @@ -1396,9 +1396,9 @@ function furthestPoint(stats: JourneyStats) { return best } -/** Metres as the books print them: "1,189 km". */ -function formatKm(metres: number): string { - return `${Math.round(metres / 1000).toLocaleString('en-US')} km` +/** Metres as the books print them: "1,189 km", in the book's own language. */ +function formatKm(metres: number, locale: string): string { + return `${Math.round(metres / 1000).toLocaleString(locale)} km` } function countriesSpread(input: AutoInput, names: string[]): BookSpread | null { @@ -1477,7 +1477,7 @@ function backSpread(input: AutoInput): BookSpread { if (input.stats) { const closing: BookMetric[] = ([ ['days', input.stats.days], - ['places', input.stats.places || input.stats.steps], + ['places', input.stats.places], ['photos', input.stats.photos], ] as [BookMetric, number][]) .filter(([, v]) => v > 0) diff --git a/client/src/components/Studio/useBookStore.ts b/client/src/components/Studio/useBookStore.ts index 6358d7e66..7fc627946 100644 --- a/client/src/components/Studio/useBookStore.ts +++ b/client/src/components/Studio/useBookStore.ts @@ -55,6 +55,8 @@ export type SaveState = | { status: 'saving' } | { status: 'saved'; at: number } | { status: 'conflict'; current: BookRecord } + /** The journey is open to this user, but they may not write it (role 'viewer'). */ + | { status: 'readonly' } | { status: 'error' } export function useBookStore( @@ -73,6 +75,9 @@ export function useBookStore( const pending = useRef<{ document: BookDocument; title: string } | null>(null) const inFlight = useRef(false) + /** The title the last queued save carried, so a re-queue does not lose it. */ + const lastTitle = useRef('') + /** * The document the server and this client agree on, by identity. * @@ -173,6 +178,15 @@ export function useBookStore( if (res?.status === 409 && res.data?.current) { blocked.current = true setState({ status: 'conflict', current: res.data.current }) + } else if (res?.status === 403) { + /* + * A viewer can open Studio - the book is part of the journey they were + * invited to - but the server will not take their writes. Stop trying + * on the first refusal and say so: retrying forever would let someone + * lay out an entire book before finding out none of it was ever saved. + */ + blocked.current = true + setState({ status: 'readonly' }) } else { setState({ status: 'error' }) } @@ -193,6 +207,7 @@ export function useBookStore( */ const queueSave = useCallback((document: BookDocument, title: string) => { latest.current = document + lastTitle.current = title // The document we already agree on is not an edit — see `synced`. if (document === synced.current) return pending.current = { document, title } @@ -207,7 +222,10 @@ export function useBookStore( /** Write immediately — for closing the editor, or a save the user asked for. */ const saveNow = useCallback((document?: BookDocument, title?: string) => { - if (document) pending.current = { document, title: title ?? '' } + if (document) { + if (title != null) lastTitle.current = title + pending.current = { document, title: title ?? lastTitle.current } + } if (timer.current != null) window.clearTimeout(timer.current) return write() }, [write]) @@ -236,8 +254,20 @@ export function useBookStore( const keepMine = useCallback((current: BookRecord) => { version.current = current.version blocked.current = false + /* + * The refused document has to be queued again here, because write() cleared + * the queue before it made the request that came back 409. Without this, + * choosing "keep mine" left the work sitting in the tab: the autosave only + * fires on the next change, and closing the editor writes whatever is + * queued, which is nothing. + */ + if (latest.current && latest.current !== synced.current) { + pending.current = { document: latest.current, title: lastTitle.current } + void write() + return + } setState({ status: 'idle' }) - }, []) + }, [write]) /** * Someone else saved. diff --git a/client/src/pages/JourneyDetailPage.tsx b/client/src/pages/JourneyDetailPage.tsx index 0b95dce89..2feea2258 100644 --- a/client/src/pages/JourneyDetailPage.tsx +++ b/client/src/pages/JourneyDetailPage.tsx @@ -1,4 +1,3 @@ -import { useRef } from 'react' import { useAuthStore } from '../store/authStore' import { journeyApi } from '../api/client' import Navbar from '../components/Layout/Navbar' @@ -45,14 +44,12 @@ function JourneyDetailPageDesktop() { showInvite, setShowInvite, showAddTrip, setShowAddTrip, unlinkTrip, setUnlinkTrip, showSettings, setShowSettings, hideSkeletons, setHideSkeletons, - mapRef, fullMapRef, activeLocationId, handleMarkerClick, handleLocationClick, + mapRef, fullMapRef, galleryUploadRef, activeLocationId, handleMarkerClick, handleLocationClick, mapEntries, sidebarMapItems, tripDates, isMobile, tracks, feedEdge, scrollFeedTo, loadJourney, updateEntry, deleteEntry, reorderEntries, uploadPhotos, deletePhoto, } = useJourneyDetail() - const galleryUploadRef = useRef<(() => void) | null>(null) - if (loading || !current) { return (
diff --git a/client/src/pages/join/useJoinTrip.test.ts b/client/src/pages/join/useJoinTrip.test.ts index 788a325f3..59cb72a51 100644 --- a/client/src/pages/join/useJoinTrip.test.ts +++ b/client/src/pages/join/useJoinTrip.test.ts @@ -1,4 +1,4 @@ -// FE-PAGE-JOIN-001 to FE-PAGE-JOIN-008 +// FE-PAGE-JOIN-001 to FE-PAGE-JOIN-011 import { act, renderHook, waitFor } from '@testing-library/react'; import { tripInviteApi } from '../../api/client'; import { useJoinTrip } from './useJoinTrip'; @@ -11,8 +11,12 @@ vi.mock('react-router', () => ({ useParams: () => params, })); +const addToast = vi.fn(); + beforeEach(() => { navigate.mockClear(); + addToast.mockClear(); + window.__addToast = addToast; params = { token: 'inv-1' }; vi.spyOn(tripInviteApi, 'preview').mockResolvedValue({ title: 'Japan 2026' } as never); vi.spyOn(tripInviteApi, 'accept').mockResolvedValue({ trip_id: 42 } as never); @@ -20,6 +24,7 @@ beforeEach(() => { afterEach(() => { vi.restoreAllMocks(); + delete window.__addToast; }); describe('useJoinTrip', () => { @@ -74,8 +79,8 @@ describe('useJoinTrip', () => { await waitFor(() => expect(navigate).toHaveBeenCalledWith('/trips/7', { replace: true })); }); - it('FE-PAGE-JOIN-006: falls back to invalid when redeeming fails', async () => { - vi.mocked(tripInviteApi.accept).mockRejectedValue(new Error('already a member')); + it('FE-PAGE-JOIN-006: falls back to invalid when the invite itself is gone', async () => { + vi.mocked(tripInviteApi.accept).mockRejectedValue({ response: { status: 404, data: { error: 'Invalid or expired invite link' } } }); const { result } = renderHook(() => useJoinTrip()); await waitFor(() => expect(result.current.state).toBe('ready')); @@ -84,6 +89,34 @@ describe('useJoinTrip', () => { expect(navigate).not.toHaveBeenCalled(); }); + // A rate limit or a 500 is not an expired invite: saying so burns a link that + // still works, and the invalid screen offers no way back. + it('FE-PAGE-JOIN-010: a server failure reports itself and leaves the invite retryable', async () => { + vi.mocked(tripInviteApi.accept).mockRejectedValue({ response: { status: 429, data: { error: 'Too many attempts. Please try again later.' } } }); + const { result } = renderHook(() => useJoinTrip()); + await waitFor(() => expect(result.current.state).toBe('ready')); + + act(() => result.current.accept()); + await waitFor(() => expect(result.current.state).toBe('ready')); + expect(addToast).toHaveBeenCalledWith('Too many attempts. Please try again later.', 'error', undefined); + expect(navigate).not.toHaveBeenCalled(); + + // And the second press really does go out again. + vi.mocked(tripInviteApi.accept).mockResolvedValue({ trip_id: 42 } as never); + await act(() => { result.current.accept(); return Promise.resolve(); }); + await waitFor(() => expect(navigate).toHaveBeenCalledWith('/trips/42', { replace: true })); + }); + + it('FE-PAGE-JOIN-011: a network failure with no status is reported, not called invalid', async () => { + vi.mocked(tripInviteApi.accept).mockRejectedValue(new Error('Network Error')); + const { result } = renderHook(() => useJoinTrip()); + await waitFor(() => expect(result.current.state).toBe('ready')); + + act(() => result.current.accept()); + await waitFor(() => expect(addToast).toHaveBeenCalled()); + expect(result.current.state).toBe('ready'); + }); + it('FE-PAGE-JOIN-007: accept is a no-op without a token', () => { params = {}; const { result } = renderHook(() => useJoinTrip()); diff --git a/client/src/pages/join/useJoinTrip.ts b/client/src/pages/join/useJoinTrip.ts index 0a7b8f14d..22c575592 100644 --- a/client/src/pages/join/useJoinTrip.ts +++ b/client/src/pages/join/useJoinTrip.ts @@ -1,6 +1,9 @@ import { useEffect, useState } from 'react' import { useNavigate, useParams } from 'react-router' import { tripInviteApi } from '../../api/client' +import { useTranslation } from '../../i18n' +import { getApiErrorMessage } from '../../types' +import { useToast } from '../../components/shared/Toast' export type JoinTripState = 'loading' | 'ready' | 'joining' | 'invalid' @@ -12,6 +15,8 @@ export type JoinTripState = 'loading' | 'ready' | 'joining' | 'invalid' export function useJoinTrip() { const { token } = useParams<{ token: string }>() const navigate = useNavigate() + const { t } = useTranslation() + const toast = useToast() const [state, setState] = useState('loading') const [title, setTitle] = useState('') @@ -30,7 +35,17 @@ export function useJoinTrip() { setState('joining') tripInviteApi.accept(token) .then((data: { trip_id: number }) => navigate(`/trips/${data.trip_id}`, { replace: true })) - .catch(() => setState('invalid')) + .catch((err: unknown) => { + // Only a 404 means the invite itself is gone. A rate limit or a server + // having a bad minute used to be reported as an expired invite too, + // which sent the user back to the dashboard with no way to try again. + if ((err as { response?: { status?: number } }).response?.status === 404) { + setState('invalid') + return + } + setState('ready') + toast.error(getApiErrorMessage(err, t('common.error'))) + }) } const goToDashboard = () => navigate('/dashboard', { replace: true }) diff --git a/client/src/pages/journeyDetail/useJourneyDetail.ts b/client/src/pages/journeyDetail/useJourneyDetail.ts index e84ac47ad..d8c9edef9 100644 --- a/client/src/pages/journeyDetail/useJourneyDetail.ts +++ b/client/src/pages/journeyDetail/useJourneyDetail.ts @@ -29,6 +29,8 @@ export function useJourneyDetail() { const { current, loading, notFound, loadJourney, updateEntry, deleteEntry, reorderEntries, uploadPhotos, deletePhoto } = useJourneyStore() const mapRef = useRef(null) const fullMapRef = useRef(null) + /** The gallery hands its file picker up here, so the hero button can open it. */ + const galleryUploadRef = useRef<(() => void) | null>(null) const [activeLocationId, setActiveLocationId] = useState(null) const isMobile = useIsMobile() @@ -382,7 +384,7 @@ export function useJourneyDetail() { showInvite, setShowInvite, showAddTrip, setShowAddTrip, unlinkTrip, setUnlinkTrip, showSettings, setShowSettings, hideSkeletons, setHideSkeletons, - mapRef, fullMapRef, activeLocationId, handleMarkerClick, handleLocationClick, + mapRef, fullMapRef, galleryUploadRef, activeLocationId, handleMarkerClick, handleLocationClick, mapEntries, sidebarMapItems, tripDates, isMobile, tracks, feedEdge, scrollFeedTo, loadJourney, updateEntry, deleteEntry, reorderEntries, uploadPhotos, deletePhoto, diff --git a/client/src/pages/journeyStudio/useJourneyStudio.test.tsx b/client/src/pages/journeyStudio/useJourneyStudio.test.tsx new file mode 100644 index 000000000..f8e049964 --- /dev/null +++ b/client/src/pages/journeyStudio/useJourneyStudio.test.tsx @@ -0,0 +1,149 @@ +import { act, renderHook, waitFor } from '@testing-library/react' +import type { JourneyStats } from '@trek/shared' +import { useStudioStore } from '../../store/studioStore' +import { useJourneyStore } from '../../store/journeyStore' +import { journeyApi } from '../../api/client' + +/** + * Studio's shell state (#1973), for the two things that cost work rather than + * pixels: the keyboard reaching past an open text field, and the layout input + * being frozen before the track has arrived. + */ + +vi.mock('react-router', () => ({ + useParams: () => ({ id: '9' }), + useNavigate: () => vi.fn(), + useLocation: () => ({ state: null }), +})) + +vi.mock('../../api/websocket', async importOriginal => ({ + ...await importOriginal(), + addListener: vi.fn(), + removeListener: vi.fn(), + joinBook: vi.fn(), + leaveBook: vi.fn(), + sendBookCursor: vi.fn(), +})) + +const autoLayout = await vi.importActual( + '../../components/Studio/autoLayout', +) +const buildBook = vi.fn(autoLayout.buildBook) +vi.mock('../../components/Studio/autoLayout', async importOriginal => { + const actual = await importOriginal() + return { ...actual, buildBook: (...args: Parameters) => buildBook(...args) } +}) + +import { useJourneyStudio } from './useJourneyStudio' + +const TRACK = [[52.5, 13.4], [52.6, 13.5], [52.7, 13.6]] + +const stats = (): JourneyStats => ({ + journeyId: 9, distance: 1000, days: 2, steps: 2, photos: 0, places: 2, furthest: 0, + countries: [], trips: [], start: '2026-06-02', end: '2026-06-03', + points: [], +} as unknown as JourneyStats) + +/** A journey with one entry, which is all the auto layout needs to build. */ +function seedJourney() { + useJourneyStore.setState({ + current: { + id: 9, title: 'Iceland', entries: [ + { id: 1, type: 'entry', title: 'A day', story: 'Something happened.', entry_date: '2026-06-02', photos: [] }, + ], gallery: [], trips: [], + } as never, + loading: false, + }) +} + +beforeEach(() => { + seedJourney() + buildBook.mockClear() + useStudioStore.setState({ doc: null, selection: [], activeSpread: 0 }) + vi.spyOn(journeyApi, 'getBook').mockResolvedValue({ book: null } as never) + vi.spyOn(journeyApi, 'saveBook').mockResolvedValue({} as never) + vi.spyOn(journeyApi, 'stats').mockResolvedValue(stats() as never) + vi.spyOn(journeyApi, 'listTracks').mockResolvedValue({ tracks: [{ points: TRACK }] } as never) +}) + +afterEach(() => { + vi.restoreAllMocks() +}) + +/** Mount and wait until the one-shot build has run. */ +async function mountStudio() { + const view = renderHook(() => useJourneyStudio()) + await waitFor(() => expect(useStudioStore.getState().doc).not.toBeNull()) + return view +} + +describe('the keyboard', () => { + /* + * Studio text commits on blur, so the sentence being typed is not in the undo + * history yet: a Ctrl+Z inside a text box took back the change before it and + * left the mistyped word exactly where it was. + */ + it('leaves undo to the field somebody is typing in', async () => { + const undo = vi.fn() + useStudioStore.setState({ undo }) + await mountStudio() + + const field = document.createElement('textarea') + document.body.appendChild(field) + act(() => { + field.dispatchEvent(new KeyboardEvent('keydown', { key: 'z', ctrlKey: true, bubbles: true })) + }) + + expect(undo).not.toHaveBeenCalled() + field.remove() + }) + + it('undoes the document when nothing is being typed into', async () => { + const undo = vi.fn() + useStudioStore.setState({ undo }) + await mountStudio() + + act(() => { + window.dispatchEvent(new KeyboardEvent('keydown', { key: 'z', ctrlKey: true, bubbles: true })) + }) + + expect(undo).toHaveBeenCalledTimes(1) + }) +}) + +describe('the auto layout input', () => { + /* + * The build waits on the book and the figures, never on the track, so the + * input was frozen with an empty path — and a relayout afterwards drew the + * straight ruler line over a route somebody had walked. + */ + it('picks up the track that lands after the book was built', async () => { + // Held back on purpose: the tracks request is the slow one in practice, and + // the build does not wait for it. + let release: (value: unknown) => void = () => {} + vi.mocked(journeyApi.listTracks).mockReturnValue(new Promise(resolve => { release = resolve }) as never) + + const { result } = await mountStudio() + await act(async () => { + release({ tracks: [{ points: TRACK }] }) + await Promise.resolve() + }) + + act(() => { result.current.relayoutBook() }) + + expect(buildBook).toHaveBeenCalled() + const input = buildBook.mock.calls[buildBook.mock.calls.length - 1][0] + expect(input.path).toEqual([TRACK]) + }) + + it('leaves the path empty when the journey has no track', async () => { + vi.mocked(journeyApi.listTracks).mockResolvedValue({ tracks: [] } as never) + const { result } = await mountStudio() + await waitFor(() => expect(journeyApi.listTracks).toHaveBeenCalled()) + + act(() => { result.current.relayoutBook() }) + + const input = buildBook.mock.calls[buildBook.mock.calls.length - 1][0] + expect(input.path).toEqual([]) + }) +}) diff --git a/client/src/pages/journeyStudio/useJourneyStudio.ts b/client/src/pages/journeyStudio/useJourneyStudio.ts index 655fe6f55..38ddd841a 100644 --- a/client/src/pages/journeyStudio/useJourneyStudio.ts +++ b/client/src/pages/journeyStudio/useJourneyStudio.ts @@ -389,9 +389,21 @@ export function useJourneyStudio() { useEffect(() => { if (!Number.isFinite(journeyId)) return let cancelled = false + /* + * The frozen layout input is patched too, not just the state. + * + * The build effect runs as soon as the book and the figures are in, which is + * usually before this answers, and what it froze is what the Auto layout + * buttons hand to buildBook afterwards. Without the patch a relayout drew + * the straight line between stops over a route somebody had walked. + */ + const applyPath = (p: [number, number][][]) => { + setPath(p) + if (autoInput.current) autoInput.current = { ...autoInput.current, path: p } + } journeyApi.listTracks(journeyId) - .then(data => { if (!cancelled) setPath(bookPath(data.tracks)) }) - .catch(() => { if (!cancelled) setPath([]) }) + .then(data => { if (!cancelled) applyPath(bookPath(data.tracks)) }) + .catch(() => { if (!cancelled) applyPath([]) }) return () => { cancelled = true } }, [journeyId]) @@ -589,8 +601,13 @@ export function useJourneyStudio() { useEffect(() => { const onKey = (e: KeyboardEvent) => { - const tag = (e.target as HTMLElement)?.tagName - const typing = tag === 'INPUT' || tag === 'TEXTAREA' + const el = e.target as HTMLElement | null + const typing = el?.tagName === 'INPUT' || el?.tagName === 'TEXTAREA' || el?.isContentEditable === true + + // Undo belongs to the field while somebody is in one: Studio text commits + // on blur, so the sentence being typed is not in the history yet and the + // document-level undo would take back the change before it instead. + if (typing) return if ((e.ctrlKey || e.metaKey) && e.key.toLowerCase() === 'z') { e.preventDefault()