1687 Fix: unsubmitted changes get stomped on (#1768)

* Prevent stomping unsaved changes in input fields

* Prevent putting unsaved changes in initialEntry

* Remove redundant entry requerying and rely on events as source of truth

* Prevent duplicate calls to parrentSetter

* Fix subscribers not updated when dirty changed

* More elegant names

* Add comment

* Remove unused import

Author: myieye

frontend/pnpm-lock.yaml

@@ -13,9 +13,9 @@ catalog:
   tslib: ^2.6.2
   tailwindcss: ^3.4.17
   eslint: ^9.29.0
-  vitest: ^3.2.2
-  "@vitest/ui": ^3.2.2
-  "@vitest/browser": ^3.2.2
+  vitest: ^3.2.3
+  "@vitest/ui": ^3.2.3
+  "@vitest/browser": ^3.2.3
   svelte-exmarkdown: ^4.0.3
   "@typescript-eslint/eslint-plugin": ^8.24.0
   "@typescript-eslint/parser": ^8.24.0

frontend/pnpm-workspace.yaml

@@ -29,6 +29,7 @@
     "test:ui": "vitest --ui",
     "test:watch": "vitest",
     "test:storybook": "vitest --project=storybook",
+    "test:unit": "vitest --project=unit",
     "check": "svelte-check",
     "lint": "eslint .",
     "lint:report": "eslint . --output-file eslint_report.json --format json",
@@ -46,7 +47,6 @@
     "@lingui/format-json": "^5.2.0",
     "@lingui/vite-plugin": "^5.2.0",
     "@mdi/js": "^7.4.47",
-    "playwright": "catalog:",
     "@playwright/test": "catalog:",
     "@storybook/addon-a11y": "^9.0.8",
     "@storybook/addon-docs": "^9.0.8",
@@ -69,9 +69,9 @@
     "eslint": "catalog:",
     "eslint-plugin-storybook": "^9.0.8",
     "eslint-plugin-svelte": "catalog:",
-    "happy-dom": "^17.1.0",
     "mode-watcher": "^1.0.7",
     "paneforge": "1.0.0-next.4",
+    "playwright": "catalog:",
     "storybook": "^9.0.8",
     "svelte": "catalog:",
     "svelte-check": "catalog:",
@@ -96,6 +96,7 @@
     "@microsoft/signalr": "^8.0.0",
     "autoprefixer": "^10.4.19",
     "fast-json-patch": "^3.1.1",
+    "jsdom": "^26.1.0",
     "just-throttle": "^4.2.0",
     "postcss": "catalog:",
     "prosemirror-commands": "^1.7.0",

frontend/viewer/package.json

@@ -1,6 +1,6 @@
 <script lang="ts">
   import type { IEntry, ISense } from '$lib/dotnet-types';
-  import {useWritingSystemService} from './writing-system-service.svelte';
+  import {asString, useWritingSystemService} from './writing-system-service.svelte';
   import { usePartsOfSpeech } from './parts-of-speech.svelte';
   import type {HTMLAttributes} from 'svelte/elements';
   import {Icon} from '$lib/components/ui/icon';
@@ -53,19 +53,19 @@
           wsId: ws.wsId,
           wsAbbr: ws.abbreviation,
           gloss: sense.gloss[ws.wsId],
-          definition: wsService.asString(sense.definition[ws.wsId]),
+          definition: asString(sense.definition[ws.wsId]),
           color: wsService.wsColor(ws.wsId, 'analysis'),
         }))
         .filter(({gloss, definition}) => gloss || definition),
       exampleSentences: sense.exampleSentences.map(example => ({
         id: example.id,
         sentences: [
           ...wsService.vernacular.map(ws => ({
-            text: wsService.asString(example.sentence[ws.wsId]),
+            text: asString(example.sentence[ws.wsId]),
             color: wsService.wsColor(ws.wsId, 'vernacular'),
           })),
           ...wsService.analysis.map(ws => ({
-            text: wsService.asString(example.translation[ws.wsId]),
+            text: asString(example.translation[ws.wsId]),
             color: wsService.wsColor(ws.wsId, 'analysis'),
           })),
         ].filter(({text}) => !!text),

frontend/viewer/src/lib/DictionaryEntry.svelte

@@ -1,9 +1,9 @@
 <script lang="ts">
   import {type IMultiString, type IWritingSystem} from '$lib/dotnet-types';
   import type {ReadonlyDeep} from 'type-fest';
-  import {Input} from '../ui/input';
   import {tryUseFieldBody} from '../editor/field/field-root.svelte';
   import {Label} from '../ui/label';
+  import StompSafeInput from '../stomp/stomp-safe-input.svelte';
 
   const fieldBodyProps = tryUseFieldBody();
   const labeledBy = fieldBodyProps?.labelId;
@@ -36,7 +36,7 @@
     <div class="grid gap-y-2 @lg/editor:grid-cols-subgrid col-span-full items-baseline"
       title={`${ws.name} (${ws.wsId})`}>
       <Label id={labelId} for={inputId}>{ws.abbreviation}</Label>
-      <Input bind:value={value[ws.wsId]}
+      <StompSafeInput bind:value={value[ws.wsId]}
         id={inputId}
         aria-labelledby="{labeledBy ?? ''} {labelId}"
         {readonly}

frontend/viewer/src/lib/components/field-editors/multi-ws-input.svelte

@@ -1,10 +1,10 @@
 <script lang="ts">
   import {type IRichMultiString, type IWritingSystem} from '$lib/dotnet-types';
   import type {ReadonlyDeep} from 'type-fest';
-  import LcmRichTextEditor from '../lcm-rich-text-editor/lcm-rich-text-editor.svelte';
   import type {IRichString} from '$lib/dotnet-types/generated-types/MiniLcm/Models/IRichString';
   import {tryUseFieldBody} from '../editor/field/field-root.svelte';
   import {Label} from '../ui/label';
+  import StompSafeLcmRichTextEditor from '../stomp/stomp-safe-lcm-rich-text-editor.svelte';
 
   const fieldBodyProps = tryUseFieldBody();
   const labelledBy = fieldBodyProps?.labelId;
@@ -43,7 +43,7 @@
     <div class="grid gap-y-2 @lg/editor:grid-cols-subgrid col-span-full items-baseline"
       title={`${ws.name} (${ws.wsId})`}>
       <Label id={labelId} for={inputId}>{ws.abbreviation}</Label>
-      <LcmRichTextEditor
+      <StompSafeLcmRichTextEditor
         bind:value={value[ws.wsId]}
         normalWs={ws.wsId}
         id={inputId}

frontend/viewer/src/lib/components/field-editors/rich-multi-ws-input.svelte

@@ -1,9 +1,9 @@
 <script lang="ts">
   import type { IWritingSystem } from '$lib/dotnet-types';
   import type { ReadonlyDeep } from 'type-fest';
-  import LcmRichTextEditor from '../lcm-rich-text-editor/lcm-rich-text-editor.svelte';
   import type {IRichString} from '$lib/dotnet-types/generated-types/MiniLcm/Models/IRichString';
   import {tryUseFieldBody} from '../editor/field/field-root.svelte';
+  import StompSafeLcmRichTextEditor from '../stomp/stomp-safe-lcm-rich-text-editor.svelte';
 
   const fieldBodyProps = tryUseFieldBody();
   const labelledBy = fieldBodyProps?.labelId;
@@ -21,15 +21,14 @@
 
   const { readonly = false, writingSystem: ws, onchange, autofocus } = $derived(constProps);
 
-
   function onRichTextChange() {
     value?.spans.forEach((span) => span.ws ??= ws.wsId);
     onchange?.(value);
   }
 </script>
 
-<LcmRichTextEditor
-  bind:value={value}
+<StompSafeLcmRichTextEditor
+  bind:value
   normalWs={ws.wsId}
   onchange={onRichTextChange}
   {readonly}

frontend/viewer/src/lib/components/field-editors/rich-ws-input.svelte

@@ -1,8 +1,8 @@
 <script lang="ts">
   import type { IWritingSystem } from '$lib/dotnet-types';
   import type { ReadonlyDeep } from 'type-fest';
-  import { Input } from '../ui/input';
   import {tryUseFieldBody} from '../editor/field/field-root.svelte';
+  import StompSafeInput from '../stomp/stomp-safe-input.svelte';
 
   const fieldBodyProps = tryUseFieldBody();
   const labelledBy = fieldBodyProps?.labelId;
@@ -22,7 +22,7 @@
   const { readonly = false, writingSystem: ws, onchange } = $derived(constProps);
 </script>
 
-<Input
+<StompSafeInput
   {readonly}
   bind:value
   title={`${ws.name} (${ws.wsId})`}

frontend/viewer/src/lib/components/field-editors/ws-input.svelte

@@ -92,9 +92,8 @@
         if (!newState.doc.eq(editor.state.doc)) {
           //todo, eventually we might want to let the user edit span props, not sure if node attributes or marks are the correct way to handle that
           //I suspect marks is the right way though.
-          if (!value) value = {spans: []};
-          value.spans = newState.doc.children.map((child) => richSpanFromNode(child))
-            .filter(s => s.text);
+          value = { spans: newState.doc.children.map((child) => richSpanFromNode(child))
+            .filter(s => s.text) };
           dirty = true;
         }
         editor.updateState(newState);

frontend/viewer/src/lib/components/lcm-rich-text-editor/lcm-rich-text-editor.svelte

@@ -0,0 +1,95 @@
+import {afterEach, beforeEach, describe, expect, it} from 'vitest';
+
+import {StompGuard} from './stomp-guard.svelte';
+import {tick} from 'svelte';
+import {watch} from 'runed';
+
+describe('StompGuard', () => {
+
+  let stompGuard: StompGuard<number>;
+  let parentValue = $state(42);
+  let cleanup: () => void;
+
+  beforeEach(() => {
+    cleanup = $effect.root(() => {
+      stompGuard = new StompGuard(() => parentValue, (value) => parentValue = value);
+    });
+  });
+
+  afterEach(() => {
+    cleanup();
+  });
+
+  it('initializes with the parent value', () => {
+    expect(stompGuard.value).toBe(parentValue);
+  });
+
+  it('accepts parent values when not dirty', () => {
+    parentValue = 100; // parent change
+    expect(stompGuard.value).toBe(100); // was accepted
+  });
+
+  it('pushs changes to parent', () => {
+    stompGuard.value = 100; // make dirty
+    expect(stompGuard.value).toBe(100);
+    expect(parentValue).toBe(100); // parent was updated
+  });
+
+  it('ignores parent values when dirty', () => {
+    stompGuard.value = 100; // make dirty
+    expect(stompGuard.value).toBe(100);
+    parentValue = 200; // parent change
+    expect(stompGuard.value).toBe(100); // was ignored
+  });
+
+  it('reverts new parent values when ignored', async () => {
+    stompGuard.value = 100; // make dirty
+    parentValue = 200; // parent change
+    expect(stompGuard.value).toBe(100); // was ignored
+    await tick();
+    expect(parentValue).toBe(100); // parent was reverted
+  });
+
+  it('starts accepting parent changes again after a flush', () => {
+    stompGuard.value = 100; // make dirty
+    stompGuard.commitAndUnlock();
+    parentValue = 200; // parent change
+    expect(stompGuard.value).toBe(200); // was accepted
+  });
+
+  it('does NOT guard against stomping deep changes, because StompGuard can\'t detect them', async () => {
+    let parentObjValue = $state({value: 42});
+    let objStompGuard: StompGuard<{value: number}>;
+    const cleanup = $effect.root(() => {
+      objStompGuard = new StompGuard(() => parentObjValue, (value) => parentObjValue = value);
+    });
+    objStompGuard!.value.value = 100; // deep change from child
+    expect(objStompGuard!.value.value).toBe(100);
+    await tick();
+    parentObjValue.value = 200; // parent change
+    expect(objStompGuard!.value.value).toBe(200); // stomped the deep change
+    cleanup();
+  });
+
+  it('keeps subscribers up to date when it becomes dirty', async () => {
+    const parentValue = $state(42);
+    let stompGuard: StompGuard<number>;
+    let derivedValue: number | undefined = undefined;
+    const cleanup = $effect.root(() => {
+      stompGuard = new StompGuard(
+        () => parentValue,
+        /* no-op to ensure subscribers are actually using the guarded value */
+        (_value) => { });
+      watch(() => stompGuard.value, (newValue) => {
+        derivedValue = newValue;
+      });
+    });
+    await tick(); // let effects run
+    expect(derivedValue).toBe(42); // Ensure the derived value is subscribed
+
+    stompGuard!.value = 100; // make dirty/update
+    await tick();
+    expect(derivedValue).toBe(100); // derived value is listening to the guard value
+    cleanup();
+  });
+});