fix(timeline): track form dirtiness via change callbacks, not an $effect
Some checks failed
CI / Unit & Component Tests (pull_request) Failing after 4m17s
CI / OCR Service Tests (pull_request) Successful in 24s
CI / Backend Unit Tests (pull_request) Successful in 5m23s
CI / fail2ban Regex (pull_request) Successful in 47s
CI / Semgrep Security Scan (pull_request) Successful in 26s
CI / Compose Bucket Idempotency (pull_request) Successful in 1m8s
SDD Gate / RTM Check (pull_request) Successful in 16s
SDD Gate / Contract Validate (pull_request) Successful in 25s
SDD Gate / Constitution Impact (pull_request) Successful in 17s
Some checks failed
CI / Unit & Component Tests (pull_request) Failing after 4m17s
CI / OCR Service Tests (pull_request) Successful in 24s
CI / Backend Unit Tests (pull_request) Successful in 5m23s
CI / fail2ban Regex (pull_request) Successful in 47s
CI / Semgrep Security Scan (pull_request) Successful in 26s
CI / Compose Bucket Idempotency (pull_request) Successful in 1m8s
SDD Gate / RTM Check (pull_request) Successful in 16s
SDD Gate / Contract Validate (pull_request) Successful in 25s
SDD Gate / Constitution Impact (pull_request) Successful in 17s
The round-2 dirty-guard used an $effect that both read and wrote its `dirtyArmed` $state, so the self-write re-triggered the effect and `dirty` flipped true on mount — the beforeNavigate confirm then fired on every navigation away from an untouched form (caught by the round-3 clean-agent review + the Svelte autofixer, which flags assigning state inside $effect). Replace it with the component's existing idiomatic pattern: DatePrecisionField, PersonMultiSelect, and DocumentMultiSelect each gain an optional `onchange` callback fired on a real user edit, and EventForm passes `markDirty` to all three. Now date/precision/end-date and picker add/remove mark the form dirty exactly like title/type/description already did — no effect, no mount-timing trap. The new props are optional, so the other consumers (WhoWhenSection, the document forms) are unaffected. Svelte autofixer: clean. Addresses PR #832 review (round-3 clean-agent concern). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -15,6 +15,8 @@ interface Props {
|
|||||||
emptyLabel?: string;
|
emptyLabel?: string;
|
||||||
/** id of the search input so a <label for=...> can be associated. */
|
/** id of the search input so a <label for=...> can be associated. */
|
||||||
inputId?: string;
|
inputId?: string;
|
||||||
|
/** Called when the selection changes (add/remove) — lets a parent track dirtiness. */
|
||||||
|
onchange?: () => void;
|
||||||
}
|
}
|
||||||
|
|
||||||
let {
|
let {
|
||||||
@@ -22,7 +24,8 @@ let {
|
|||||||
placeholder = m.geschichte_editor_search_document(),
|
placeholder = m.geschichte_editor_search_document(),
|
||||||
hiddenInputName = 'documentIds',
|
hiddenInputName = 'documentIds',
|
||||||
emptyLabel = undefined,
|
emptyLabel = undefined,
|
||||||
inputId = undefined
|
inputId = undefined,
|
||||||
|
onchange = undefined
|
||||||
}: Props = $props();
|
}: Props = $props();
|
||||||
|
|
||||||
let searchTerm = $state('');
|
let searchTerm = $state('');
|
||||||
@@ -54,10 +57,12 @@ function selectDocument(doc: DocumentOption) {
|
|||||||
selectedDocuments = [...selectedDocuments, doc];
|
selectedDocuments = [...selectedDocuments, doc];
|
||||||
searchTerm = '';
|
searchTerm = '';
|
||||||
picker.close();
|
picker.close();
|
||||||
|
onchange?.();
|
||||||
}
|
}
|
||||||
|
|
||||||
function removeDocument(id: string | undefined) {
|
function removeDocument(id: string | undefined) {
|
||||||
selectedDocuments = selectedDocuments.filter((d) => d.id !== id);
|
selectedDocuments = selectedDocuments.filter((d) => d.id !== id);
|
||||||
|
onchange?.();
|
||||||
}
|
}
|
||||||
</script>
|
</script>
|
||||||
|
|
||||||
|
|||||||
@@ -13,13 +13,16 @@ interface Props {
|
|||||||
emptyLabel?: string;
|
emptyLabel?: string;
|
||||||
/** id of the search input so a <label for=...> can be associated. */
|
/** id of the search input so a <label for=...> can be associated. */
|
||||||
inputId?: string;
|
inputId?: string;
|
||||||
|
/** Called when the selection changes (add/remove) — lets a parent track dirtiness. */
|
||||||
|
onchange?: () => void;
|
||||||
}
|
}
|
||||||
|
|
||||||
let {
|
let {
|
||||||
selectedPersons = $bindable([]),
|
selectedPersons = $bindable([]),
|
||||||
hiddenInputName = 'receiverIds',
|
hiddenInputName = 'receiverIds',
|
||||||
emptyLabel = undefined,
|
emptyLabel = undefined,
|
||||||
inputId = undefined
|
inputId = undefined,
|
||||||
|
onchange = undefined
|
||||||
}: Props = $props();
|
}: Props = $props();
|
||||||
|
|
||||||
let searchTerm = $state('');
|
let searchTerm = $state('');
|
||||||
@@ -65,10 +68,12 @@ function selectPerson(person: Person) {
|
|||||||
searchTerm = '';
|
searchTerm = '';
|
||||||
showDropdown = false;
|
showDropdown = false;
|
||||||
results = [];
|
results = [];
|
||||||
|
onchange?.();
|
||||||
}
|
}
|
||||||
|
|
||||||
function removePerson(id: string | undefined) {
|
function removePerson(id: string | undefined) {
|
||||||
selectedPersons = selectedPersons.filter((p) => p.id !== id);
|
selectedPersons = selectedPersons.filter((p) => p.id !== id);
|
||||||
|
onchange?.();
|
||||||
}
|
}
|
||||||
</script>
|
</script>
|
||||||
|
|
||||||
|
|||||||
@@ -36,6 +36,7 @@ let {
|
|||||||
dateLabel = m.form_label_date(),
|
dateLabel = m.form_label_date(),
|
||||||
dateRequired = true,
|
dateRequired = true,
|
||||||
dateError = '',
|
dateError = '',
|
||||||
|
onchange = undefined,
|
||||||
dateTestId = undefined,
|
dateTestId = undefined,
|
||||||
precisionTestId = undefined,
|
precisionTestId = undefined,
|
||||||
endDateInnerTestId = undefined
|
endDateInnerTestId = undefined
|
||||||
@@ -52,6 +53,8 @@ let {
|
|||||||
dateRequired?: boolean;
|
dateRequired?: boolean;
|
||||||
/** Server-side date error (e.g. blank required field) wired to the field's aria-invalid. */
|
/** Server-side date error (e.g. blank required field) wired to the field's aria-invalid. */
|
||||||
dateError?: string;
|
dateError?: string;
|
||||||
|
/** Called on any user edit (date, precision, end-date) — lets a parent track dirtiness. */
|
||||||
|
onchange?: () => void;
|
||||||
dateTestId?: string;
|
dateTestId?: string;
|
||||||
precisionTestId?: string;
|
precisionTestId?: string;
|
||||||
endDateInnerTestId?: string;
|
endDateInnerTestId?: string;
|
||||||
@@ -102,12 +105,14 @@ function handleDateInput(e: Event) {
|
|||||||
dateDisplay = result.display;
|
dateDisplay = result.display;
|
||||||
dateIso = result.iso;
|
dateIso = result.iso;
|
||||||
dateDirty = true;
|
dateDirty = true;
|
||||||
|
onchange?.();
|
||||||
}
|
}
|
||||||
|
|
||||||
function handleEndDateInput(e: Event) {
|
function handleEndDateInput(e: Event) {
|
||||||
const result = handleGermanDateInput(e);
|
const result = handleGermanDateInput(e);
|
||||||
endDisplay = result.display;
|
endDisplay = result.display;
|
||||||
endDateIso = result.iso;
|
endDateIso = result.iso;
|
||||||
|
onchange?.();
|
||||||
}
|
}
|
||||||
|
|
||||||
$effect(() => {
|
$effect(() => {
|
||||||
@@ -161,6 +166,7 @@ $effect(() => {
|
|||||||
id="{dateInputName}Precision"
|
id="{dateInputName}Precision"
|
||||||
name={precisionInputName}
|
name={precisionInputName}
|
||||||
bind:value={precision}
|
bind:value={precision}
|
||||||
|
onchange={() => onchange?.()}
|
||||||
class="block min-h-[48px] w-full rounded border border-line px-2 py-3 text-sm shadow-sm focus:outline-none focus-visible:ring-2 focus-visible:ring-focus-ring"
|
class="block min-h-[48px] w-full rounded border border-line px-2 py-3 text-sm shadow-sm focus:outline-none focus-visible:ring-2 focus-visible:ring-focus-ring"
|
||||||
>
|
>
|
||||||
{#each PRECISIONS as p (p.value)}
|
{#each PRECISIONS as p (p.value)}
|
||||||
|
|||||||
@@ -101,26 +101,15 @@ beforeNavigate(({ cancel }) => {
|
|||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// Every editable control routes its change through markDirty so the
|
||||||
|
// beforeNavigate guard catches edits to the date/precision/end-date and the
|
||||||
|
// pickers too — not just title/type/description (their onchange callbacks call
|
||||||
|
// this). No $effect: marking dirty from the actual edit events avoids a
|
||||||
|
// snapshot-vs-effect mount-timing trap.
|
||||||
function markDirty() {
|
function markDirty() {
|
||||||
dirty = true;
|
dirty = true;
|
||||||
}
|
}
|
||||||
|
|
||||||
// title/type/description set `dirty` through their own oninput/onchange hooks,
|
|
||||||
// but the date/precision/end-date (inside DatePrecisionField) and the two pickers
|
|
||||||
// have no such hook — so changing only a date or a linked person would otherwise
|
|
||||||
// slip past the beforeNavigate guard. Watch those values and mark dirty on any
|
|
||||||
// change after the initial prop snapshot (the first run only arms the watcher).
|
|
||||||
let dirtyArmed = $state(false);
|
|
||||||
$effect(() => {
|
|
||||||
void dateIso;
|
|
||||||
void precision;
|
|
||||||
void endDateIso;
|
|
||||||
void selectedPersons.length;
|
|
||||||
void selectedDocuments.length;
|
|
||||||
if (dirtyArmed) dirty = true;
|
|
||||||
else dirtyArmed = true;
|
|
||||||
});
|
|
||||||
|
|
||||||
// Guards a submit with a blank title client-side. The server re-validates and
|
// Guards a submit with a blank title client-side. The server re-validates and
|
||||||
// owns the authoritative fail(400) with per-field flags.
|
// owns the authoritative fail(400) with per-field flags.
|
||||||
function handleSubmit(e: SubmitEvent) {
|
function handleSubmit(e: SubmitEvent) {
|
||||||
@@ -238,6 +227,7 @@ async function confirmDelete(e: SubmitEvent) {
|
|||||||
precisionInputName="precision"
|
precisionInputName="precision"
|
||||||
dateLabel={m.form_label_date()}
|
dateLabel={m.form_label_date()}
|
||||||
dateError={dateError}
|
dateError={dateError}
|
||||||
|
onchange={markDirty}
|
||||||
dateTestId="event-date"
|
dateTestId="event-date"
|
||||||
precisionTestId="event-precision"
|
precisionTestId="event-precision"
|
||||||
endDateInnerTestId="event-end-date"
|
endDateInnerTestId="event-end-date"
|
||||||
@@ -279,6 +269,7 @@ async function confirmDelete(e: SubmitEvent) {
|
|||||||
inputId="event-persons-input"
|
inputId="event-persons-input"
|
||||||
hiddenInputName="personIds"
|
hiddenInputName="personIds"
|
||||||
emptyLabel={m.event_editor_persons_empty()}
|
emptyLabel={m.event_editor_persons_empty()}
|
||||||
|
onchange={markDirty}
|
||||||
/>
|
/>
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
@@ -295,6 +286,7 @@ async function confirmDelete(e: SubmitEvent) {
|
|||||||
inputId="event-documents-input"
|
inputId="event-documents-input"
|
||||||
hiddenInputName="documentIds"
|
hiddenInputName="documentIds"
|
||||||
emptyLabel={m.event_editor_documents_empty()}
|
emptyLabel={m.event_editor_documents_empty()}
|
||||||
|
onchange={markDirty}
|
||||||
/>
|
/>
|
||||||
</div>
|
</div>
|
||||||
</div>
|
</div>
|
||||||
|
|||||||
Reference in New Issue
Block a user