From 6150fc7be5d3490459befcc64a8c40eff44192ae Mon Sep 17 00:00:00 2001
From: Marcel <marcel@familienarchiv>
Date: Sun, 14 Jun 2026 00:55:34 +0200
Subject: [PATCH] fix(timeline): track form dirtiness via change callbacks, not
 an $effect
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 .../lib/document/DocumentMultiSelect.svelte   |  7 +++++-
 .../src/lib/person/PersonMultiSelect.svelte   |  7 +++++-
 .../primitives/DatePrecisionField.svelte      |  6 +++++
 frontend/src/lib/timeline/EventForm.svelte    | 24 +++++++------------
 4 files changed, 26 insertions(+), 18 deletions(-)

diff --git a/frontend/src/lib/document/DocumentMultiSelect.svelte b/frontend/src/lib/document/DocumentMultiSelect.svelte
index f190c5fb..e736b13d 100644
--- a/frontend/src/lib/document/DocumentMultiSelect.svelte
+++ b/frontend/src/lib/document/DocumentMultiSelect.svelte
@@ -15,6 +15,8 @@ interface Props {
 	emptyLabel?: string;
 	/** id of the search input so a <label for=...> can be associated. */
 	inputId?: string;
+	/** Called when the selection changes (add/remove) — lets a parent track dirtiness. */
+	onchange?: () => void;
 }
 
 let {
@@ -22,7 +24,8 @@ let {
 	placeholder = m.geschichte_editor_search_document(),
 	hiddenInputName = 'documentIds',
 	emptyLabel = undefined,
-	inputId = undefined
+	inputId = undefined,
+	onchange = undefined
 }: Props = $props();
 
 let searchTerm = $state('');
@@ -54,10 +57,12 @@ function selectDocument(doc: DocumentOption) {
 	selectedDocuments = [...selectedDocuments, doc];
 	searchTerm = '';
 	picker.close();
+	onchange?.();
 }
 
 function removeDocument(id: string | undefined) {
 	selectedDocuments = selectedDocuments.filter((d) => d.id !== id);
+	onchange?.();
 }
 </script>
 
diff --git a/frontend/src/lib/person/PersonMultiSelect.svelte b/frontend/src/lib/person/PersonMultiSelect.svelte
index 998cb98a..a4d5a600 100644
--- a/frontend/src/lib/person/PersonMultiSelect.svelte
+++ b/frontend/src/lib/person/PersonMultiSelect.svelte
@@ -13,13 +13,16 @@ interface Props {
 	emptyLabel?: string;
 	/** id of the search input so a <label for=...> can be associated. */
 	inputId?: string;
+	/** Called when the selection changes (add/remove) — lets a parent track dirtiness. */
+	onchange?: () => void;
 }
 
 let {
 	selectedPersons = $bindable([]),
 	hiddenInputName = 'receiverIds',
 	emptyLabel = undefined,
-	inputId = undefined
+	inputId = undefined,
+	onchange = undefined
 }: Props = $props();
 
 let searchTerm = $state('');
@@ -65,10 +68,12 @@ function selectPerson(person: Person) {
 	searchTerm = '';
 	showDropdown = false;
 	results = [];
+	onchange?.();
 }
 
 function removePerson(id: string | undefined) {
 	selectedPersons = selectedPersons.filter((p) => p.id !== id);
+	onchange?.();
 }
 </script>
 
diff --git a/frontend/src/lib/shared/primitives/DatePrecisionField.svelte b/frontend/src/lib/shared/primitives/DatePrecisionField.svelte
index a1b21f63..c65d294e 100644
--- a/frontend/src/lib/shared/primitives/DatePrecisionField.svelte
+++ b/frontend/src/lib/shared/primitives/DatePrecisionField.svelte
@@ -36,6 +36,7 @@ let {
 	dateLabel = m.form_label_date(),
 	dateRequired = true,
 	dateError = '',
+	onchange = undefined,
 	dateTestId = undefined,
 	precisionTestId = undefined,
 	endDateInnerTestId = undefined
@@ -52,6 +53,8 @@ let {
 	dateRequired?: boolean;
 	/** Server-side date error (e.g. blank required field) wired to the field's aria-invalid. */
 	dateError?: string;
+	/** Called on any user edit (date, precision, end-date) — lets a parent track dirtiness. */
+	onchange?: () => void;
 	dateTestId?: string;
 	precisionTestId?: string;
 	endDateInnerTestId?: string;
@@ -102,12 +105,14 @@ function handleDateInput(e: Event) {
 	dateDisplay = result.display;
 	dateIso = result.iso;
 	dateDirty = true;
+	onchange?.();
 }
 
 function handleEndDateInput(e: Event) {
 	const result = handleGermanDateInput(e);
 	endDisplay = result.display;
 	endDateIso = result.iso;
+	onchange?.();
 }
 
 $effect(() => {
@@ -161,6 +166,7 @@ $effect(() => {
 		id="{dateInputName}Precision"
 		name={precisionInputName}
 		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"
 	>
 		{#each PRECISIONS as p (p.value)}
diff --git a/frontend/src/lib/timeline/EventForm.svelte b/frontend/src/lib/timeline/EventForm.svelte
index 38d752ea..36184adf 100644
--- a/frontend/src/lib/timeline/EventForm.svelte
+++ b/frontend/src/lib/timeline/EventForm.svelte
@@ -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() {
 	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
 // owns the authoritative fail(400) with per-field flags.
 function handleSubmit(e: SubmitEvent) {
@@ -238,6 +227,7 @@ async function confirmDelete(e: SubmitEvent) {
 							precisionInputName="precision"
 							dateLabel={m.form_label_date()}
 							dateError={dateError}
+							onchange={markDirty}
 							dateTestId="event-date"
 							precisionTestId="event-precision"
 							endDateInnerTestId="event-end-date"
@@ -279,6 +269,7 @@ async function confirmDelete(e: SubmitEvent) {
 						inputId="event-persons-input"
 						hiddenInputName="personIds"
 						emptyLabel={m.event_editor_persons_empty()}
+						onchange={markDirty}
 					/>
 				</div>
 
@@ -295,6 +286,7 @@ async function confirmDelete(e: SubmitEvent) {
 						inputId="event-documents-input"
 						hiddenInputName="documentIds"
 						emptyLabel={m.event_editor_documents_empty()}
+						onchange={markDirty}
 					/>
 				</div>
 			</div>