mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
fix(studio): keep "Keep mine" from throwing the document away
After a save conflict, Keep mine set the version and unblocked writes but never re-queued the local document, and write() had already cleared the queue before the request. So if the user picked Keep mine and then simply closed the editor without typing anything else, saveNow() found nothing pending and did nothing: their afternoon of edits existed only in memory, the indicator said idle, and the server kept the other version. The exact outcome the conflict dialog promises to prevent. Ctrl+Z was intercepted while typing in a text field too. Studio commits text on blur, so mid-edit the typing is not in the undo history yet: the browser's own text undo was suppressed and a previous layout change was undone underneath the open editor instead. Reads as random data loss. Also: delete no longer removes locked elements, the auto-layout no longer freezes an empty GPS path when tracks resolve after the one-shot build, the back-cover tally cannot print "0 places", the map fetch aborts when Studio closes, and a double Enter in the list-name field stops creating two lists. The page-pattern gate had quietly stopped covering anything: the Desktop-shim refactor moved every real container out of the default export the checker reads, and its regex missed useRef<T>(null) anyway. Widened on both counts, and the state it immediately found moved into the hooks.
This commit is contained in:
@@ -2,9 +2,12 @@
|
||||
// src/pages/PATTERN.md). A *Page.tsx default-export component should wire a
|
||||
// co-located use<Page>() 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()}`)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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' } })
|
||||
|
||||
@@ -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) }))
|
||||
|
||||
@@ -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<AbortController>())
|
||||
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<BookElement>)
|
||||
}
|
||||
})
|
||||
.catch(() => { /* No roads is a state the map already draws. */ })
|
||||
.finally(() => { roadJobs.current.delete(job) })
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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`
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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 (
|
||||
<div className="min-h-screen bg-zinc-50 dark:bg-zinc-950">
|
||||
|
||||
@@ -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());
|
||||
|
||||
@@ -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<JoinTripState>('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 })
|
||||
|
||||
@@ -29,6 +29,8 @@ export function useJourneyDetail() {
|
||||
const { current, loading, notFound, loadJourney, updateEntry, deleteEntry, reorderEntries, uploadPhotos, deletePhoto } = useJourneyStore()
|
||||
const mapRef = useRef<JourneyMapHandle>(null)
|
||||
const fullMapRef = useRef<JourneyMapHandle>(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<string | null>(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,
|
||||
|
||||
@@ -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<typeof import('../../api/websocket')>(),
|
||||
addListener: vi.fn(),
|
||||
removeListener: vi.fn(),
|
||||
joinBook: vi.fn(),
|
||||
leaveBook: vi.fn(),
|
||||
sendBookCursor: vi.fn(),
|
||||
}))
|
||||
|
||||
const autoLayout = await vi.importActual<typeof import('../../components/Studio/autoLayout')>(
|
||||
'../../components/Studio/autoLayout',
|
||||
)
|
||||
const buildBook = vi.fn(autoLayout.buildBook)
|
||||
vi.mock('../../components/Studio/autoLayout', async importOriginal => {
|
||||
const actual = await importOriginal<typeof import('../../components/Studio/autoLayout')>()
|
||||
return { ...actual, buildBook: (...args: Parameters<typeof actual.buildBook>) => 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([])
|
||||
})
|
||||
})
|
||||
@@ -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()
|
||||
|
||||
Reference in New Issue
Block a user