fix(tours): the checklist keeps to its tour, its state and its focus

A decision answered after another tour was opened no longer replaces that
tour's checklist. A decision made offline on a checklist already older than
the route stays marked as such, and once online a bench from a write made
offline waits for the queue to replay, then takes in the server's. While
the checklist reads again it stays in view, locked, so a panel open on the
map keeps its note. A saved tour turned into a paddle tour asks to be saved
first, a vanished obstacle stays among the rows, Next follows the list
order and keeps the focus in the panel, a note closes only once saved, the
earlier choice in review is named for screen readers too, planning a
portage is offered to editors only, and the map card scrolls within the
screen. A tooltip of several lines breaks at them. A page test waits for
the tours row instead of reading it at once.
This commit is contained in:
Maurice
2026-10-17 02:07:22 +00:00
parent 2a4b71b3d9
commit a7f5a114e6
15 changed files with 404 additions and 89 deletions
@@ -7,7 +7,7 @@ import type { ObstacleDecision, ObstacleDecisionRecord, TourObstacleCheck, TourO
import { buildSettings } from '../../../../tests/helpers/factories';
import { earlierEntry, obstacleCheck, obstacleEntry } from '../../../../tests/helpers/paddleCheck';
import { fireEvent, render, screen, within } from '../../../../tests/helpers/render';
import { fireEvent, render, screen, waitFor, within } from '../../../../tests/helpers/render';
import { seedStore } from '../../../../tests/helpers/store';
import { useSettingsStore } from '../../../store/settingsStore';
import { ChecklistObstaclePanel, ObstacleDecisionPanel } from './ObstacleDecisionPanel';
@@ -52,7 +52,7 @@ function decidedEntry(decision: ObstacleDecision = 'pass', note: string | null =
type PanelProps = ComponentProps<typeof ObstacleDecisionPanel>;
const onDecide = vi.fn<PanelProps['onDecide']>();
const onNote = vi.fn<PanelProps['onNote']>();
const onNote = vi.fn<PanelProps['onNote']>(() => Promise.resolve(true));
const onClear = vi.fn<PanelProps['onClear']>();
const onKeep = vi.fn<PanelProps['onKeep']>();
/** Trip member 1 is Anna; anyone else has left the trip. */
@@ -98,6 +98,7 @@ function checklistOf(
stale: false,
dirty: false,
canDecide: true,
canPlanPortage: true,
savingKey: null,
selectedKey: null,
dialogKey: null,
@@ -108,10 +109,10 @@ function checklistOf(
nextOf: () => null,
next: vi.fn<PaddleChecklist['next']>(),
planPortage: vi.fn<PaddleChecklist['planPortage']>(),
decide: vi.fn<PaddleChecklist['decide']>(() => Promise.resolve()),
setNote: vi.fn<PaddleChecklist['setNote']>(() => Promise.resolve()),
clear: vi.fn<PaddleChecklist['clear']>(() => Promise.resolve()),
keep: vi.fn<PaddleChecklist['keep']>(() => Promise.resolve()),
decide: vi.fn<PaddleChecklist['decide']>(() => Promise.resolve(true)),
setNote: vi.fn<PaddleChecklist['setNote']>(() => Promise.resolve(true)),
clear: vi.fn<PaddleChecklist['clear']>(() => Promise.resolve(true)),
keep: vi.fn<PaddleChecklist['keep']>(() => Promise.resolve(true)),
memberName,
...overrides,
};
@@ -166,7 +167,8 @@ describe('ObstacleDecisionPanel', () => {
it('FE-PADDLE-DECIDE-003: in review the earlier choice keeps a dashed ring, nothing is checked, the reason shows', () => {
renderPanel(obstacleEntry({ status: 'review', changed: ['route'], decision: record('pass') }));
const earlier = screen.getByRole('radio', { name: 'Paddle through' });
// Named with its review state too, so it is told apart by more than its ring.
const earlier = screen.getByRole('radio', { name: 'Paddle through, Check again' });
expect(earlier).toHaveAttribute('aria-checked', 'false');
expect(earlier).toHaveClass('border-dashed');
expect(earlier).not.toHaveClass('bg-accent');
@@ -251,7 +253,7 @@ describe('ObstacleDecisionPanel', () => {
expect(screen.getByTestId('obstacle-on-site')).toHaveTextContent('unknown');
});
it('FE-PADDLE-DECIDE-009: a decision takes a note of two rows and at most 500 characters, saved trimmed', () => {
it('FE-PADDLE-DECIDE-009: a decision takes a note of two rows and at most 500 characters, saved trimmed', async () => {
renderPanel(decidedEntry());
fireEvent.click(screen.getByRole('button', { name: 'Note' }));
@@ -264,7 +266,18 @@ describe('ObstacleDecisionPanel', () => {
fireEvent.click(screen.getByRole('button', { name: 'Save' }));
expect(onNote).toHaveBeenCalledTimes(1);
expect(onNote).toHaveBeenCalledWith('Links an der Bootsrutsche');
expect(screen.queryByRole('textbox')).not.toBeInTheDocument();
await waitFor(() => expect(screen.queryByRole('textbox')).not.toBeInTheDocument());
});
it('FE-PADDLE-DECIDE-021: a note whose save failed stays in the editor with its text', async () => {
onNote.mockResolvedValueOnce(false);
renderPanel(decidedEntry());
fireEvent.click(screen.getByRole('button', { name: 'Note' }));
fireEvent.change(screen.getByRole('textbox', { name: 'Note' }), { target: { value: 'Rechts umtragen' } });
fireEvent.click(screen.getByRole('button', { name: 'Save' }));
await waitFor(() => expect(onNote).toHaveBeenCalledWith('Rechts umtragen'));
await Promise.resolve();
expect(screen.getByRole('textbox', { name: 'Note' })).toHaveValue('Rechts umtragen');
});
it('FE-PADDLE-DECIDE-010: a saved note reads clamped; Cancel and an unchanged save keep it, emptying it removes it', () => {
@@ -385,20 +398,28 @@ describe('ObstacleDecisionPanel', () => {
});
describe('ChecklistObstaclePanel', () => {
it('FE-PADDLE-DECIDE-017: shows nothing for an object the checklist does not list or while it is not ready', () => {
it('FE-PADDLE-DECIDE-017: shows nothing without a checklist or for an object it does not list, and stays while it reads again', () => {
const { container, rerender } = render(<ChecklistObstaclePanel checklist={checklistOf()} featureRef="node/2" />);
expect(container).toBeEmptyDOMElement();
rerender(
<ChecklistObstaclePanel checklist={checklistOf(obstacleCheck(), { status: 'loading' })} featureRef="node/1" />
);
rerender(<ChecklistObstaclePanel checklist={checklistOf(null, { status: 'loading' })} featureRef="node/1" />);
expect(container).toBeEmptyDOMElement();
// Reading again keeps the panel open, locked, so a note being written is not lost.
rerender(
<ChecklistObstaclePanel
checklist={checklistOf(obstacleCheck(), { status: 'loading', canDecide: false })}
featureRef="node/1"
/>
);
expect(screen.getByTestId('obstacle-decision-panel')).toBeInTheDocument();
expect(screen.getByRole('radio', { name: 'Paddle through' })).toBeDisabled();
rerender(<ChecklistObstaclePanel checklist={checklistOf()} featureRef="node/1" />);
expect(screen.getByTestId('obstacle-decision-panel')).toBeInTheDocument();
});
it('FE-PADDLE-DECIDE-018: decides, plans the portage, notes, removes and moves on through the checklist', () => {
it('FE-PADDLE-DECIDE-018: decides, plans the portage, notes, removes and moves on through the checklist', async () => {
const entry = obstacleEntry({ status: 'todo', decision: record('portage') });
const following = obstacleEntry({ key: 'node/2', refs: ['node/2'], name: 'Wehr Pielenhofen', along_m: 3400 });
const checklist = checklistOf(obstacleCheck({ entries: [entry, following] }), {
@@ -421,6 +442,37 @@ describe('ChecklistObstaclePanel', () => {
fireEvent.click(screen.getByRole('button', { name: 'Next' }));
expect(checklist.next).toHaveBeenCalledWith(entry);
expect(screen.getByText(`Decided ${DECIDED_ON} by Anna`)).toBeInTheDocument();
await waitFor(() => expect(screen.queryByRole('textbox')).not.toBeInTheDocument());
});
it('FE-PADDLE-DECIDE-022: someone who may not edit the tour is not offered to plan the portage', () => {
const entry = obstacleEntry({ status: 'todo', decision: record('portage') });
render(
<ChecklistObstaclePanel
checklist={checklistOf(obstacleCheck({ entries: [entry] }), { canPlanPortage: false, canDecide: false })}
featureRef="node/1"
/>
);
expect(screen.queryByRole('button', { name: 'Plan the portage' })).not.toBeInTheDocument();
});
it('FE-PADDLE-DECIDE-023: moving on keeps the focus in the panel when the next entry has no Next of its own', () => {
const entry = obstacleEntry({ status: 'todo', decision: record('portage') });
const following = obstacleEntry({ key: 'node/2', refs: ['node/2'], name: 'Wehr Pielenhofen', along_m: 3400 });
const check = obstacleCheck({ entries: [entry, following] });
let featureRef = 'node/1';
const checklist = checklistOf(check, {
nextOf: (item) => (item.key === entry.key ? following : null),
next: vi.fn<PaddleChecklist['next']>(() => {
featureRef = 'node/2';
}),
});
const { rerender } = render(<ChecklistObstaclePanel checklist={checklist} featureRef={featureRef} />);
const next = screen.getByRole('button', { name: 'Next' });
next.focus();
fireEvent.click(next);
rerender(<ChecklistObstaclePanel checklist={checklist} featureRef={featureRef} />);
expect(screen.getByTestId('obstacle-decision-panel')).toHaveFocus();
});
it('FE-PADDLE-DECIDE-019: a decision about an object the data no longer holds is kept through the checklist', () => {
@@ -18,7 +18,7 @@ import {
Unlink,
X,
} from 'lucide-react';
import { useState, type KeyboardEvent, type ReactNode } from 'react';
import { useLayoutEffect, useRef, useState, type KeyboardEvent, type ReactNode } from 'react';
import { useTranslation } from '../../../i18n';
import { fs } from '../../shared/DialogShell';
import { Tooltip } from '../../shared/Tooltip';
@@ -104,6 +104,8 @@ function DecisionChoices({
>
<span className="flex-none">{CHOICE_ICON[choice]}</span>
<span className="min-w-0 truncate">{t(`tours.paddle.check.decision.${choice}`)}</span>
{/* The earlier choice is told apart by more than its dashed ring. */}
{look === 'earlier' && <span className="sr-only">{`, ${t('tours.paddle.check.status.review')}`}</span>}
</button>
);
})}
@@ -236,15 +238,21 @@ function NoteBlock({
}: {
note: string | null;
disabled: boolean;
onNote: (note: string | null) => void;
/** Resolves to whether the note was saved; a failed save keeps the text being written. */
onNote: (note: string | null) => Promise<boolean>;
}) {
const { t } = useTranslation();
// The text being written, or null while the note is only read.
const [draft, setDraft] = useState<string | null>(null);
const save = (text: string) => {
const next = text.trim() || null;
if (next !== note) onNote(next);
setDraft(null);
if (next === note) {
setDraft(null);
return;
}
void onNote(next).then((saved) => {
if (saved) setDraft(null);
});
};
if (draft !== null)
return (
@@ -364,7 +372,7 @@ export function ObstacleDecisionPanel({
saving: boolean;
memberName: (id: number | null) => string | null;
onDecide: (decision: ObstacleDecision) => void;
onNote: (note: string | null) => void;
onNote: (note: string | null) => Promise<boolean>;
onClear: () => void;
onKeep: () => void;
/** Moves on to the next entry still asking for a look; null when there is none. */
@@ -376,8 +384,28 @@ export function ObstacleDecisionPanel({
const locked = !canDecide || saving;
const gone = entry.status === 'gone';
const label = entry.name ?? t(`tours.paddle.obstacle.kind.${entry.kind}`);
const root = useRef<HTMLDivElement>(null);
const movedOn = useRef(false);
// Moving on keeps the focus in the panel, also when the next entry has no Next button of its own.
useLayoutEffect(() => {
if (!movedOn.current) return;
movedOn.current = false;
if (!root.current?.contains(document.activeElement)) root.current?.focus({ preventScroll: true });
});
const next = onNext
? () => {
movedOn.current = true;
onNext();
}
: null;
return (
<div data-testid="obstacle-decision-panel" aria-busy={saving || undefined} className="flex flex-col gap-2.5">
<div
ref={root}
tabIndex={-1}
data-testid="obstacle-decision-panel"
aria-busy={saving || undefined}
className="flex flex-col gap-2.5 outline-none"
>
<div className="flex flex-wrap items-center gap-1.5 empty:hidden">
<ObstacleStatusBadges entry={entry} />
</div>
@@ -391,14 +419,14 @@ export function ObstacleDecisionPanel({
value={entry.local_check ? formatCheckDate(entry.local_check, locale) : t('tours.paddle.check.unknown')}
/>
</div>
{entry.decision && <NoteBlock note={entry.decision.note} disabled={locked} onNote={onNote} />}
{entry.decision && <NoteBlock key={entry.key} note={entry.decision.note} disabled={locked} onNote={onNote} />}
<MetaLine
decision={entry.decision}
memberName={memberName}
clearable={!gone}
disabled={locked}
onClear={onClear}
onNext={onNext}
onNext={next}
/>
</div>
);
@@ -408,17 +436,16 @@ export function ObstacleDecisionPanel({
export function ChecklistEntryPanel({ checklist, entry }: { checklist: PaddleChecklist; entry: TourObstacleEntry }) {
return (
<ObstacleDecisionPanel
key={entry.key}
entry={entry}
canDecide={checklist.canDecide}
saving={checklist.savingKey === entry.key}
memberName={checklist.memberName}
onDecide={(decision) => void checklist.decide(entry, decision)}
onNote={(note) => void checklist.setNote(entry, note)}
onNote={(note) => checklist.setNote(entry, note)}
onClear={() => void checklist.clear(entry)}
onKeep={() => void checklist.keep(entry)}
onNext={checklist.nextOf(entry) ? () => checklist.next(entry) : null}
onPlanPortage={() => checklist.planPortage(entry)}
onPlanPortage={checklist.canPlanPortage ? () => checklist.planPortage(entry) : null}
/>
);
}
@@ -426,10 +453,11 @@ export function ChecklistEntryPanel({ checklist, entry }: { checklist: PaddleChe
/**
* The checklist's part of the map card of a paddle object (PAD-06): the
* decision panel of the entry the object belongs to, below its facts, or
* nothing while the checklist is not ready or does not list the object.
* nothing while there is no checklist or it does not list the object.
*/
export function ChecklistObstaclePanel({ checklist, featureRef }: { checklist: PaddleChecklist; featureRef: string }) {
const entry = checklist.status === 'ready' ? checklist.entryForRef(featureRef) : null;
// While the checklist reads again it keeps showing, locked, so the panel open on an object stays open.
const entry = checklist.check ? checklist.entryForRef(featureRef) : null;
if (!entry) return null;
return (
<div className="mt-3 border-t border-edge-faint pt-3">
@@ -29,6 +29,7 @@ function checklistOf(
stale: false,
dirty: false,
canDecide: true,
canPlanPortage: true,
savingKey: null,
selectedKey: null,
dialogKey: null,
@@ -39,10 +40,10 @@ function checklistOf(
nextOf: () => null,
next: vi.fn<PaddleChecklist['next']>(),
planPortage: vi.fn<PaddleChecklist['planPortage']>(),
decide: vi.fn<PaddleChecklist['decide']>(() => Promise.resolve()),
setNote: vi.fn<PaddleChecklist['setNote']>(() => Promise.resolve()),
clear: vi.fn<PaddleChecklist['clear']>(() => Promise.resolve()),
keep: vi.fn<PaddleChecklist['keep']>(() => Promise.resolve()),
decide: vi.fn<PaddleChecklist['decide']>(() => Promise.resolve(true)),
setNote: vi.fn<PaddleChecklist['setNote']>(() => Promise.resolve(true)),
clear: vi.fn<PaddleChecklist['clear']>(() => Promise.resolve(true)),
keep: vi.fn<PaddleChecklist['keep']>(() => Promise.resolve(true)),
memberName: (id) => (id === 1 ? 'Anna' : null),
...overrides,
};
@@ -110,10 +110,12 @@ const ADVISED_LOCK = ownPortage({
const BYPASSED_LOCK = ownPortage({ kind: 'lock', index: 5, from_m: 5600, to_m: 5600, covers: [], bypassed: true });
/** All of it out of route order, as the server may list it. */
const ROUTE = obstacleCheck({ entries: [NEAR_LOCK, WEIR, BANNED_WEIR], own: [ADVISED_LOCK, OWN_PORTAGE] });
/** The weir, and an earlier decision about a sill the data no longer holds. */
const GONE = earlierEntry();
/** The weir, and an earlier decision about a sill the route no longer meets. */
const GONE = earlierEntry({ status: 'off_route' });
const GONE_NAME = 'Alte Sohlschwelle';
const WITH_EARLIER = obstacleCheck({ entries: [WEIR, GONE] });
/** The weir, and a decision about a sill the complete data no longer holds: it counts, so it stays in view. */
const VANISHED = earlierEntry({ key: 'node/8', refs: ['node/8'], name: 'Verschwundenes Wehr' });
/** What usePaddleChecklist hands the card: a ready list matching the saved route, nothing selected, every action a spy. */
function checklistOf(overrides: Partial<PaddleChecklist> = {}): PaddleChecklist {
@@ -125,6 +127,7 @@ function checklistOf(overrides: Partial<PaddleChecklist> = {}): PaddleChecklist
stale: false,
dirty: false,
canDecide: true,
canPlanPortage: true,
savingKey: null,
selectedKey: null,
dialogKey: null,
@@ -132,10 +135,10 @@ function checklistOf(overrides: Partial<PaddleChecklist> = {}): PaddleChecklist
select: vi.fn(),
selectOwn: vi.fn(),
closeDialog: vi.fn(),
decide: vi.fn(async () => undefined),
setNote: vi.fn(async () => undefined),
clear: vi.fn(async () => undefined),
keep: vi.fn(async () => undefined),
decide: vi.fn(async () => true),
setNote: vi.fn(async () => true),
clear: vi.fn(async () => true),
keep: vi.fn(async () => true),
nextOf: vi.fn(() => null),
next: vi.fn(),
planPortage: vi.fn(),
@@ -366,7 +369,7 @@ describe('PaddleCheckCard', () => {
const earlier = screen.getByRole('button', { name: /^Alte Sohlschwelle/ });
// No longer on the route, so no place along it.
expect(earlier).not.toHaveTextContent('km');
expect(earlier).toHaveTextContent('status gone');
expect(earlier).toHaveTextContent('status off_route');
fireEvent.click(earlier);
expect(select).toHaveBeenCalledWith(GONE);
});
@@ -389,6 +392,14 @@ describe('PaddleCheckCard', () => {
expect(screen.queryByText(GONE_NAME)).not.toBeInTheDocument();
});
it('FE-PADDLE-CHECKCARD-018: a decision about an object the data no longer holds stays among the rows, after the route', () => {
renderCard(checklistOf({ check: obstacleCheck({ entries: [VANISHED, WEIR] }) }));
const rows = rowButtons();
expect(rows).toHaveLength(2);
expect(rows[1]).toHaveTextContent('Verschwundenes Wehr');
expect(screen.queryByRole('button', { name: SAYS.earlier })).not.toBeInTheDocument();
});
it('FE-PADDLE-CHECKCARD-015: without rows it says nothing is known on covered data, and that there is none where none is', () => {
const empty = obstacleCheck({ entries: [], own: [] });
const bench = paddleBench({ entries: 0, open: 0 });
@@ -330,7 +330,8 @@ function EmptyLine({ summary }: { summary: PaddleCheck | null }) {
/** The rows along the route, then the earlier decisions; dimmed, never hidden, while they may not match the route. */
function CheckList({ checklist, check }: { checklist: PaddleChecklist; check: TourObstacleCheck }) {
const { rows, earlier } = useMemo(() => checklistRows(check), [check]);
const dimmed = checklist.dirty || checklist.stale;
// Dimmed while it may not match the route, and while the list it came from reads again.
const dimmed = checklist.dirty || checklist.stale || checklist.status === 'loading';
return (
<div data-testid="paddle-check-list" className={`flex flex-col gap-1.5 ${dimmed ? 'opacity-60' : ''}`}>
{rows.length > 0 ? (
@@ -134,7 +134,7 @@ export default function PaddleObstacleCard({
const color = paddleKindCss(feature.kind);
return (
<article
className={`rounded-2xl p-3 text-content ${width ? '' : 'w-max min-w-[min(18rem,calc(100vw_-_2rem))] max-w-[22.5rem]'}`}
className={`max-h-[70vh] overflow-y-auto overscroll-contain rounded-2xl p-3 text-content ${width ? '' : 'w-max min-w-[min(18rem,calc(100vw_-_2rem))] max-w-[22.5rem]'}`}
style={{ ...MAP_CARD_GLASS, width }}
data-testid="paddle-obstacle-card"
>
@@ -188,7 +188,7 @@ describe('focusPointsOf', () => {
});
describe('checklistRows', () => {
it('PCM-014: lists the obstacles and the own portages and locks in route order, the earlier decisions apart', () => {
it('PCM-014: lists the obstacles and the own portages and locks in route order, then the vanished ones, the earlier decisions apart', () => {
const weir = obstacleEntry();
const rapids = obstacleEntry({
key: 'node/2',
@@ -200,9 +200,10 @@ describe('checklistRows', () => {
});
const dam = obstacleEntry({ key: 'node/3', refs: ['node/3'], kind: 'dam', along_m: 2600 });
const gone = earlierEntry();
const offRoute = earlierEntry({ key: 'node/10', refs: ['node/10'], status: 'off_route' });
const portage = ownPortage();
const lock = ownPortage({ kind: 'lock', index: 2, from_m: 2000, to_m: 2000, covers: [] });
const check = obstacleCheck({ entries: [dam, weir, gone, rapids], own: [lock, portage] });
const check = obstacleCheck({ entries: [dam, weir, gone, offRoute, rapids], own: [lock, portage] });
expect(checklistRows(check)).toEqual({
rows: [
@@ -211,8 +212,9 @@ describe('checklistRows', () => {
{ type: 'obstacle', entry: weir },
{ type: 'own', own: lock },
{ type: 'obstacle', entry: dam },
{ type: 'obstacle', entry: gone },
],
earlier: [gone],
earlier: [offRoute],
});
});
@@ -261,3 +263,13 @@ describe('nextPendingEntry', () => {
expect(nextPendingEntry(obstacleCheck({ entries: [first] }), first)).toBeNull();
});
});
describe('nextPendingEntry at one metre', () => {
it('PCM-019: two pending entries at the same metre follow each other in list order', () => {
const first = obstacleEntry({ key: 'node/1', along_m: 1200 });
const second = obstacleEntry({ key: 'node/2', refs: ['node/2'], along_m: 1200 });
const check = obstacleCheck({ entries: [first, second] });
expect(nextPendingEntry(check, first)?.key).toBe('node/2');
expect(nextPendingEntry(check, second)).toBeNull();
});
});
@@ -89,27 +89,36 @@ export type ChecklistRow = { type: 'obstacle'; entry: TourObstacleEntry } | { ty
/**
* The rows of a checklist in route order, the paddler's own portages and
* locks among the obstacles at their metre, and apart from them the earlier
* decisions the route or the data no longer carries.
* locks among the obstacles at their metre, then the decisions about objects
* the complete data no longer holds (they count as to check again, so they
* stay in view). Apart from them the earlier decisions the route no longer
* meets or the data cannot tell.
*/
export function checklistRows(check: TourObstacleCheck): { rows: ChecklistRow[]; earlier: TourObstacleEntry[] } {
const metreOf = (row: ChecklistRow) => (row.type === 'obstacle' ? (row.entry.along_m ?? 0) : row.own.from_m);
const rows: ChecklistRow[] = [
const onRoute: ChecklistRow[] = [
...check.entries.filter((entry) => entry.relation !== null).map((entry) => ({ type: 'obstacle' as const, entry })),
...check.own.map((own) => ({ type: 'own' as const, own })),
].sort((a, b) => metreOf(a) - metreOf(b));
return { rows, earlier: check.entries.filter((entry) => entry.relation === null) };
const earlier = check.entries.filter((entry) => entry.relation === null);
const gone = earlier.filter((entry) => entry.status === 'gone');
return {
rows: [...onRoute, ...gone.map((entry) => ({ type: 'obstacle' as const, entry }))],
earlier: earlier.filter((entry) => entry.status !== 'gone'),
};
}
const PENDING = new Set(['open', 'todo', 'review']);
/** The next entry along the route still asking for a look; from an earlier decision, the first one. */
/**
* The next entry still asking for a look, in the order the checklist lists
* them (route order, entries at one metre in list order); from an earlier
* decision, the first one.
*/
export function nextPendingEntry(check: TourObstacleCheck, entry: TourObstacleEntry): TourObstacleEntry | null {
const from = entry.along_m ?? -1;
return (
check.entries
.filter((candidate) => candidate.relation !== null && PENDING.has(candidate.status))
.filter((candidate) => candidate.key !== entry.key && (candidate.along_m ?? 0) > from)
.sort((a, b) => (a.along_m ?? 0) - (b.along_m ?? 0))[0] ?? null
);
const current = check.entries
.filter((candidate) => candidate.relation !== null)
.sort((a, b) => (a.along_m ?? 0) - (b.along_m ?? 0));
const from = current.findIndex((candidate) => candidate.key === entry.key);
return current.slice(from + 1).find((candidate) => PENDING.has(candidate.status)) ?? null;
}
@@ -1,5 +1,5 @@
/**
* UPC-001 to UPC-023: the obstacle checklist of the paddle tour open in the
* UPC-001 to UPC-030: the obstacle checklist of the paddle tour open in the
* tours workspace (PAD-06, PAD-09), as its card and the map share it.
*
* Pinned: off for anything but an open paddle tour in an active workspace,
@@ -219,7 +219,7 @@ beforeEach(() => {
describe('usePaddleChecklist: what it reads', () => {
it('UPC-001: is off for a hike, a neutral planner and a new tour that is no paddle tour, and reads nothing', () => {
const hike = paddleTour({ tour_type: 'hike', boat_type: null, water_context: null, paddle_check: null });
const { result, rerender } = renderChecklist(options({ tours: [hike] }));
const { result, rerender } = renderChecklist(options({ tours: [hike], planner: planner({ isPaddle: false }) }));
expect(result.current).toMatchObject({ status: 'off', tour: null, check: null, summary: null, canDecide: false });
rerender(options({ planner: planner({ mode: { type: 'neutral' } }) }));
@@ -375,7 +375,7 @@ describe('usePaddleChecklist: deciding', () => {
const onSaved = vi.fn<(tour: TourListItem) => void>();
const { result, rerender } = await renderReady(options({ onSaved }));
let saving: Promise<void> = Promise.resolve();
let saving: Promise<boolean> = Promise.resolve(true);
act(() => {
saving = result.current.decide(ENTRY, 'pass');
});
@@ -436,7 +436,7 @@ describe('usePaddleChecklist: deciding', () => {
const onSaved = vi.fn<(tour: TourListItem) => void>();
const { result } = await renderReady(options({ onSaved }));
let saving: Promise<void> = Promise.resolve();
let saving: Promise<boolean> = Promise.resolve(true);
act(() => {
saving = result.current.decide(ENTRY, 'portage');
});
@@ -662,3 +662,102 @@ describe('usePaddleChecklist: the map and the route', () => {
expect(setSelectedWaypointId).toHaveBeenCalledTimes(1);
});
});
describe('usePaddleChecklist after review', () => {
const answer = (check: typeof CHECK, revision: string, place = 7): TourDetailResponse => ({
tour: paddleTour({ place_id: place, paddle_check: paddleBench({ revision }) }),
waypoints: [],
obstacle_check: check,
});
it('UPC-024: a save answered after another tour opened leaves that tour as it read itself', async () => {
const other = paddleTour({ place_id: 8, paddle_check: paddleBench({ revision: 'rev-8' }) });
const save = deferred<TourDetailResponse>();
repo.saveObstacleReviews.mockReturnValueOnce(save.promise);
const props = options({ tours: [TOUR, other] });
const { result, rerender } = await renderReady(props);
act(() => {
void result.current.decide(ENTRY, 'pass');
});
const otherCheck = obstacleCheck({
revision: 'rev-8',
entries: [obstacleEntry({ key: 'node/8', refs: ['node/8'] })],
});
repo.obstacleCheck.mockResolvedValue({ check: otherCheck, stale: false });
rerender({ ...props, planner: planner({ mode: { type: 'edit-saved', placeId: 8 } }) });
await waitFor(() => expect(result.current.check?.entries[0]?.key).toBe('node/8'));
expect(result.current.savingKey).toBeNull();
await act(async () => {
save.resolve(
answer(obstacleCheck({ revision: 'rev-2', entries: [obstacleEntry({ status: 'decided' })] }), 'rev-2')
);
await save.promise;
});
expect(result.current.status).toBe('ready');
expect(result.current.check?.entries[0]?.key).toBe('node/8');
expect(props.onSaved).toHaveBeenCalledWith(expect.objectContaining({ place_id: 7 }));
});
it('UPC-025: a save whose checklist is of another revision than its bench stays marked stale', async () => {
const { result, rerender } = await renderReady();
repo.saveObstacleReviews.mockResolvedValueOnce(answer(obstacleCheck({ revision: 'rev-1' }), 'local:u1'));
await act(async () => {
await result.current.decide(ENTRY, 'pass');
});
rerender(options({ tours: [paddleTour({ paddle_check: paddleBench({ revision: 'local:u1' }) })] }));
expect(result.current.stale).toBe(true);
});
it('UPC-026: a read that finds a newer bench on the server hands the tour on and is not stale', async () => {
const newer = paddleTour({ paddle_check: paddleBench({ revision: 'rev-9' }) });
repo.obstacleCheck.mockResolvedValue({ check: obstacleCheck({ revision: 'rev-9' }), stale: true, tour: newer });
const props = options({ tours: [paddleTour({ paddle_check: paddleBench({ revision: 'local:u1' }) })] });
const { result, rerender } = renderChecklist(props);
await waitFor(() => expect(props.onSaved).toHaveBeenCalledWith(newer));
rerender({ ...props, tours: [newer] });
expect(result.current).toMatchObject({ status: 'ready', stale: false });
});
it('UPC-027: while the same tour reads again its last checklist stays, locked', async () => {
const { result, rerender } = await renderReady();
const again = deferred<CheckRead>();
repo.obstacleCheck.mockReturnValueOnce(again.promise);
rerender(options({ tours: [paddleTour({ paddle_check: paddleBench({ revision: 'rev-3' }) })] }));
expect(result.current.status).toBe('loading');
expect(result.current.check).toEqual(CHECK);
expect(result.current.canDecide).toBe(false);
await act(async () => {
again.resolve({ check: obstacleCheck({ revision: 'rev-3' }), stale: false });
await again.promise;
});
expect(result.current.status).toBe('ready');
});
it('UPC-028: a saved tour turned into a paddle tour and not saved yet asks to be saved first', () => {
const hike = paddleTour({ tour_type: 'hike', boat_type: null, water_context: null, paddle_check: null });
const { result } = renderChecklist(options({ tours: [hike], planner: planner({ isPaddle: true }) }));
expect(result.current).toMatchObject({ status: 'new', tour: null, canDecide: false });
expect(repo.obstacleCheck).not.toHaveBeenCalled();
});
it('UPC-029: planning a portage is offered to editors only', async () => {
expect((await renderReady()).result.current.canPlanPortage).toBe(true);
expect((await renderReady(options({ canEdit: false }))).result.current.canPlanPortage).toBe(false);
});
it('UPC-030: a note whose save failed answers false, one that went through true', async () => {
const { result } = await renderReady();
repo.saveObstacleReviews.mockRejectedValueOnce(new Error('offline'));
let saved: boolean | undefined;
await act(async () => {
saved = await result.current.setNote(ENTRY, 'Links');
});
expect(saved).toBe(false);
repo.saveObstacleReviews.mockResolvedValueOnce(answer(CHECK, 'rev-1'));
await act(async () => {
saved = await result.current.setNote(ENTRY, 'Links');
});
expect(saved).toBe(true);
});
});
@@ -10,7 +10,7 @@ import {
type TourOwnEntry,
type TripMember,
} from '@trek/shared';
import { useCallback, useEffect, useMemo, useState } from 'react';
import { useCallback, useEffect, useEffectEvent, useMemo, useState } from 'react';
import { useNetworkMode } from '../../../hooks/useNetworkMode';
import { useTranslation } from '../../../i18n';
import { applyObstacleOps } from '../../../repo/obstacleCheckLocal';
@@ -43,6 +43,7 @@ interface PaddleChecklistOptions {
}
interface Loaded {
placeId: number;
key: string;
check: TourObstacleCheck | null;
stale: boolean;
@@ -57,27 +58,59 @@ function openPaddleTour(mode: TourPlannerController['mode'], tours: readonly Tou
}
/** Reads the checklist for the bench the list shows, again whenever its revision or the connection changes. */
function useChecklistRead(tripId: number, tour: TourListItem | null, active: boolean) {
/** The read key of a tour's checklist: a new bench revision or a change of the connection reads it again. */
const readKey = (placeId: number | null, revision: string | null, offline: boolean) =>
`${placeId}:${revision}:${offline}`;
/**
* Reads the checklist for the bench the list shows, again whenever its
* revision or the connection changes. A read that finds the server further
* than the list (a queued write replayed meanwhile) hands the newer tour on,
* so the list and its bench catch up.
*/
function useChecklistRead(
tripId: number,
tour: TourListItem | null,
active: boolean,
onNewer: (tour: TourListItem) => void
) {
const { offline } = useNetworkMode();
const placeId = tour?.place_id ?? null;
const revision = tour?.paddle_check?.revision ?? null;
const key = `${placeId}:${revision}:${offline}`;
const key = readKey(placeId, revision, offline);
const [loaded, setLoaded] = useState<Loaded | null>(null);
// Handing the newer tour on is no reason to read again.
const takeNewer = useEffectEvent(onNewer);
useEffect(() => {
if (!active || placeId === null) return;
const controller = new AbortController();
tourRepo
.obstacleCheck(tripId, placeId, revision, controller.signal)
.then(({ check, stale }) => {
if (!controller.signal.aborted) setLoaded({ key, check, stale, failed: false });
.then(({ check, stale, tour: answered }) => {
if (controller.signal.aborted) return;
const newer = answered?.paddle_check?.revision;
if (answered && newer && newer !== revision) {
setLoaded({
placeId,
key: readKey(placeId, newer, offline),
check,
stale: check?.revision !== newer,
failed: false,
});
takeNewer(answered);
return;
}
setLoaded({ placeId, key, check, stale, failed: false });
})
.catch(() => {
if (!controller.signal.aborted) setLoaded({ key, check: null, stale: false, failed: true });
if (!controller.signal.aborted) setLoaded({ placeId, key, check: null, stale: false, failed: true });
});
return () => controller.abort();
}, [active, key, placeId, revision, tripId]);
}, [active, key, offline, placeId, revision, tripId]);
const current = loaded?.key === key ? loaded : null;
return { current, offline, key, setLoaded };
// While the same tour reads again, what it read before stays in view, locked, so a panel open on it stays open.
const previous = !current && loaded?.placeId === placeId && loaded.check ? loaded : null;
return { current, previous, offline, key, setLoaded };
}
/** Where a portage for an obstacle starts on a planned tour: the control point whose leg passes it. */
@@ -194,14 +227,16 @@ function useChecklistNavigation(
export function usePaddleChecklist(options: PaddleChecklistOptions) {
const { tripId, planner, tours, canEdit, active, trackPaddlePlan } = options;
const tour = useMemo(() => openPaddleTour(planner.mode, tours), [planner.mode, tours]);
const read = useChecklistRead(tripId, tour, active);
const read = useChecklistRead(tripId, tour, active, options.onSaved);
const writes = useChecklistWrites(options, tour, read);
const check = writes.optimistic ?? read.current?.check ?? null;
const check = writes.optimistic ?? read.current?.check ?? read.previous?.check ?? null;
const navigation = useChecklistNavigation(planner, check, trackPaddlePlan);
const dirty = planner.hasUnsavedChanges || !!trackPaddlePlan?.dirty;
const status = checklistStatus({
active,
newPaddleDraft: planner.mode.type === 'new-draft' && planner.isPaddle,
// A new paddle draft, or a saved tour turned into one and not saved yet: no server check exists for either.
newPaddleDraft:
planner.isPaddle && (planner.mode.type === 'new-draft' || (planner.mode.type === 'edit-saved' && !tour)),
tour,
current: read.current,
offline: read.offline,
@@ -216,6 +251,8 @@ export function usePaddleChecklist(options: PaddleChecklistOptions) {
stale: !!read.current?.stale,
dirty,
canDecide: canEdit && status === 'ready' && !dirty && saved && writes.savingKey === null,
/** Planning a portage edits the tour, so only someone who may edit it is offered it. */
canPlanPortage: canEdit,
savingKey: writes.savingKey,
...navigation,
...writes.actions,
@@ -235,30 +272,38 @@ function useChecklistWrites(
const { t } = useTranslation();
const toast = useToast();
const userId = useAuthStore((state) => state.user?.id ?? null);
const [pending, setPending] = useState<{ key: string; check: TourObstacleCheck } | null>(null);
const [pending, setPending] = useState<{ placeId: number; key: string; check: TourObstacleCheck } | null>(null);
const base = read.current?.check ?? null;
const placeId = tour?.place_id ?? null;
const { setLoaded, offline } = read;
const save = useCallback(
async (key: string, op: ObstacleReviewOp) => {
if (!base || placeId === null) return;
if (!base || placeId === null) return false;
setPending({
placeId,
key,
check: applyObstacleOps(base, [op], { now: new Date().toISOString(), userId, revision: 'pending' }),
});
try {
const result = await tourRepo.saveObstacleReviews(tripId, placeId, { ops: [op] }, userId);
const revision = result.tour.paddle_check?.revision ?? null;
setLoaded({
key: `${placeId}:${revision}:${offline}`,
check: result.obstacle_check ?? base,
stale: false,
const check = result.obstacle_check ?? base;
const next: Loaded = {
placeId,
key: readKey(placeId, revision, offline),
check,
// A checklist still judged on an older route than the bench stays marked as such.
stale: revision !== null && check.revision !== revision,
failed: false,
});
};
// Another tour opened meanwhile keeps what it read; this one's answer reaches the list and the cache.
setLoaded((current) => (current && !current.key.startsWith(`${placeId}:`) ? current : next));
onSaved(result.tour);
return true;
} catch {
toast.error(t('tours.paddle.marks.failed'));
return false;
} finally {
setPending(null);
}
@@ -285,5 +330,7 @@ function useChecklistWrites(
}),
[save]
);
return { optimistic: pending?.check ?? null, savingKey: pending?.key ?? null, actions };
// A save still running for a tour left meanwhile shows nothing on the one open now.
const mine = pending && pending.placeId === placeId ? pending : null;
return { optimistic: mine?.check ?? null, savingKey: mine?.key ?? null, actions };
}
+13 -1
View File
@@ -1,4 +1,4 @@
// FE-W4TIP-001 to FE-W4TIP-014
// FE-W4TIP-001 to FE-W4TIP-015
import { createRef, useRef } from 'react'
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'
import { render, screen, fireEvent, act } from '../../../tests/helpers/render'
@@ -181,4 +181,16 @@ describe('Tooltip — inside a component that owns the ref', () => {
expect(screen.getByRole('button')).toHaveAttribute('data-clicked', '1')
})
it('FE-W4TIP-015: a label of several lines breaks at them and stays within the window, a single line does not wrap', () => {
const { unmount } = render(<Tooltip label={'3 open\nNo safety or legal clearance.'}><button>x</button></Tooltip>)
hover(screen.getByRole('button'))
expect(screen.getByRole('tooltip')).toHaveStyle({ whiteSpace: 'pre-line' })
expect(screen.getByRole('tooltip').style.maxWidth).toContain('100vw')
unmount()
render(<Tooltip label="Delete"><button>y</button></Tooltip>)
hover(screen.getByRole('button'))
expect(screen.getByRole('tooltip')).toHaveStyle({ whiteSpace: 'nowrap' })
})
})
+3 -1
View File
@@ -92,7 +92,9 @@ export function Tooltip({ label, placement = 'bottom', delay = 250, disabled, ch
fontWeight: 500,
padding: '5px 10px',
borderRadius: 8,
whiteSpace: 'nowrap',
// A label of several lines (the paddle check bench) breaks at its lines and wraps within the window.
whiteSpace: label.includes('\n') ? 'pre-line' : 'nowrap',
maxWidth: label.includes('\n') ? 'min(22rem, calc(100vw - 12px))' : undefined,
boxShadow: '0 4px 12px rgba(0,0,0,0.15)',
fontFamily: "var(--font-system)",
transformOrigin: placement === 'top' ? 'bottom center' : placement === 'bottom' ? 'top center' : placement === 'left' ? 'center right' : 'center left',
@@ -308,7 +308,8 @@ describe('TripPlannerPage — shell', () => {
expect(await screen.findByText('Plan a tour')).toBeInTheDocument()
expect(screen.getByRole('button', { name: 'Plan new tour' })).toBeInTheDocument()
expect(screen.queryByLabelText('Tour name')).not.toBeInTheDocument()
expect(screen.getByRole('option', { name: /Editable ridge tour/i })).not.toHaveAttribute('aria-selected', 'true')
// The tours list fills in after the rail: wait for its row instead of reading it at once.
expect(await screen.findByRole('option', { name: /Editable ridge tour/i })).not.toHaveAttribute('aria-selected', 'true')
expect(plannerActions.openTour).not.toHaveBeenCalled()
})
+34 -3
View File
@@ -1,4 +1,4 @@
// FE-REPO-TOUR-030 to FE-REPO-TOUR-038
// FE-REPO-TOUR-030 to FE-REPO-TOUR-040
// A paddle tour's checklist through the offline core (PAD-06, PAD-09): read
// from the cache while its revision matches the bench, decisions saved online
// and cached, saved offline under a local revision and queued, and a route
@@ -92,14 +92,14 @@ describe('tourRepo.obstacleCheck', () => {
})
)
);
expect(await tourRepo.obstacleCheck(3, 40, 'rev-2')).toEqual({ check: fresh, stale: false });
expect(await tourRepo.obstacleCheck(3, 40, 'rev-2')).toMatchObject({ check: fresh, stale: false });
expect((await offlineDb.tours.get(40))?.obstacle_check).toEqual(fresh);
});
it('FE-REPO-TOUR-032: offline, an older cached checklist stands in and reads as stale', async () => {
setOnline(false);
await offlineDb.tours.put({ ...paddleTour(), trip_id: 3, waypoints, obstacle_check: obstacleCheck() });
expect(await tourRepo.obstacleCheck(3, 40, 'local:abc')).toEqual({ check: obstacleCheck(), stale: true });
expect(await tourRepo.obstacleCheck(3, 40, 'local:abc')).toMatchObject({ check: obstacleCheck(), stale: true });
});
});
@@ -204,3 +204,34 @@ describe('a route written offline and the list cache', () => {
expect(tours[0]).not.toHaveProperty('obstacle_check');
});
});
describe('after review', () => {
it('FE-REPO-TOUR-039: a decision queued on a checklist already stale keeps it stale against the bench', async () => {
setOnline(false);
vi.spyOn(mutationQueue, 'sendSoon').mockImplementation(() => undefined);
await offlineDb.tours.put({
...paddleTour({ paddle_check: paddleBench({ revision: 'local:route' }) }),
trip_id: 3,
waypoints,
obstacle_check: obstacleCheck(),
});
const result = await tourRepo.saveObstacleReviews(3, 40, decide, 4);
expect(result.tour.paddle_check?.revision).toBe('local:route');
expect(result.obstacle_check?.revision).not.toBe('local:route');
expect((await tourRepo.obstacleCheck(3, 40, 'local:route')).stale).toBe(true);
});
it('FE-REPO-TOUR-040: online, a bench of a write made offline waits for the queue, then reads the server and hands its tour on', async () => {
await offlineDb.tours.put({ ...paddleTour(), trip_id: 3, waypoints, obstacle_check: obstacleCheck() });
const flush = vi.spyOn(mutationQueue, 'flush').mockResolvedValue();
const server_ = paddleTour({ paddle_check: paddleBench({ revision: 'rev-5' }) });
server.use(
http.get('/api/trips/3/tours/40', () =>
HttpResponse.json({ tour: server_, waypoints, obstacle_check: obstacleCheck({ revision: 'rev-5' }) })
)
);
const read = await tourRepo.obstacleCheck(3, 40, 'local:abc');
expect(flush).toHaveBeenCalled();
expect(read).toMatchObject({ check: { revision: 'rev-5' }, tour: { paddle_check: { revision: 'rev-5' } } });
});
});
+14 -5
View File
@@ -268,18 +268,22 @@ export const tourRepo = {
* PAD-06: a paddle tour's checklist for the bench the list shows. The
* cached one serves while its revision matches; otherwise the detail is read
* (the cache standing in offline), and a checklist of another revision than
* the bench's reads as stale.
* the bench's reads as stale. A bench of a write made here offline waits for
* the queue to replay once online, and the tour read with the detail comes
* back with it, so the list can take in the server's bench.
*/
async obstacleCheck(
tripId: number | string,
placeId: number,
revision: string | null,
signal?: AbortSignal,
): Promise<{ check: TourObstacleCheck | null; stale: boolean }> {
): Promise<{ check: TourObstacleCheck | null; stale: boolean; tour?: TourListItem }> {
if (revision?.startsWith('local:') && !isEffectivelyOffline()) await mutationQueue.flush()
const cached = await offlineDb.tours.get(placeId)
if (revision !== null && cached?.obstacle_check?.revision === revision) return { check: cached.obstacle_check, stale: false }
const check = (await tourRepo.detail(tripId, placeId, signal)).obstacle_check ?? null
return { check, stale: check !== null && revision !== null && check.revision !== revision }
const detail = await tourRepo.detail(tripId, placeId, signal)
const check = detail.obstacle_check ?? null
return { check, stale: check !== null && revision !== null && check.revision !== revision, tour: detail.tour }
},
/**
@@ -300,9 +304,14 @@ export const tourRepo = {
if (!existing?.obstacle_check) throw new Error('Checklist not available offline')
const id = generateUUID()
const revision = `local:${id}`
// A checklist still judged on an older route than the bench keeps that gap, so it stays marked stale.
const routeStale = !!existing.paddle_check && existing.paddle_check.revision !== existing.obstacle_check.revision
const check = applyObstacleOps(existing.obstacle_check, body.ops, { now: new Date().toISOString(), userId, revision })
const bench = existing.paddle_check
? { ...paddleCheckSummaryOf(check.entries, check.own, existing.paddle_check), revision }
? {
...paddleCheckSummaryOf(check.entries, check.own, existing.paddle_check),
revision: routeStale ? existing.paddle_check.revision : revision,
}
: existing.paddle_check
const tour: CachedTour = { ...existing, obstacle_check: check, paddle_check: bench }
await offlineDb.tours.put(tour)