A swath of UI bug fixes and small improvements

.vscode/settings.json

@@ -43,5 +43,8 @@
     ],
     "iconify.preview.exclude": [
       "**/icon-class.ts"
-    ]
+    ],
+    "search.exclude": {
+      "**/dist": true
+    }
 }

backend/FwLite/LcmCrdt/RemoteSync/CrdtHttpSyncService.cs

@@ -88,7 +88,17 @@ async Task ISyncable.AddRangeFromSync(IEnumerable<Commit> commits)
 
     async Task<ChangesResult<Commit>> ISyncable.GetChanges(SyncState otherHeads)
     {
-        var changes = await restSyncClient.GetChanges(projectId, otherHeads);
+        var changesResponse = await restSyncClient.GetChanges(projectId, otherHeads);
+        if (changesResponse.Error is not null)
+        {
+            // The inner exception is almost certainly more interesting than the wrapping ApiException
+            // (e.g. a JsonException if trying to download a change object that is too new for this version of FieldWorks Lite)
+            var error = changesResponse.Error?.InnerException ?? changesResponse.Error!;
+            throw new CrdtSyncException("FieldWorks Lite is likely out of date. Failed to download dictionary changes.",
+                CrdtSyncException.CrdtSyncStep.Download, error);
+        }
+
+        var changes = changesResponse.Content;
         ArgumentNullException.ThrowIfNull(changes);
         foreach (var commit in changes.MissingFromClient)
         {
@@ -128,5 +138,5 @@ public interface ISyncHttp
     internal Task<SyncState> GetSyncState(Guid id);
 
     [Post("/api/crdt/{id}/changes")]
-    internal Task<ChangesResult<Commit>> GetChanges(Guid id, SyncState otherHeads);
+    internal Task<ApiResponse<ChangesResult<Commit>>> GetChanges(Guid id, SyncState otherHeads);
 }

backend/FwLite/LcmCrdt/RemoteSync/CrdtSyncException.cs

@@ -0,0 +1,17 @@
+namespace LcmCrdt.RemoteSync;
+
+public class CrdtSyncException : Exception
+{
+    public enum CrdtSyncStep { Upload, Download }
+
+    public CrdtSyncStep Step { get; init; }
+
+    public CrdtSyncException(string message, CrdtSyncStep step) : base(message)
+    {
+        Step = step;
+    }
+    public CrdtSyncException(string message, CrdtSyncStep step, Exception innerException) : base(message, innerException)
+    {
+        Step = step;
+    }
+}

frontend/viewer/.storybook/decorators/FWLiteDecorator.svelte

@@ -29,6 +29,8 @@
   import {extract} from 'runed';
   import {setupGlobalErrorHandlers} from '$lib/errors/global-errors';
   import {TooltipProvider} from '$lib/components/ui/tooltip';
+  import {initRootLocation} from '$lib/services/root-location-service';
+  import {readable} from 'svelte/store';
 
 
   let { children }: { children: Snippet } = $props();
@@ -39,6 +41,7 @@
   initView();
   const storyContext = useSvelteStoryContext();
   setupGlobalErrorHandlers();
+  initRootLocation(readable());
 
   const {
     themePicker = true,

frontend/viewer/src/App.svelte

@@ -2,14 +2,9 @@
   import {TooltipProvider} from '$lib/components/ui/tooltip';
   import {setupGlobalErrorHandlers} from '$lib/errors/global-errors';
   import {ModeWatcher} from 'mode-watcher';
-  import {navigate, Route, Router} from 'svelte-routing';
+  import {Router} from 'svelte-routing';
   import NotificationOutlet from './lib/notifications/NotificationOutlet.svelte';
-  import Sandbox from '$lib/sandbox/Sandbox.svelte';
-  import DotnetProjectView from './DotnetProjectView.svelte';
-  import HomeView from './home/HomeView.svelte';
-  import TestProjectView from './TestProjectView.svelte';
-
-  let url = '';
+  import AppRoutes from './AppRoutes.svelte';
 
   setupGlobalErrorHandlers();
 </script>
@@ -19,49 +14,8 @@
 
 <TooltipProvider delayDuration={300}>
   <div class="app">
-    <Router {url}>
-      <Route path="/project/:code/*" let:params>
-        <Router {url}>
-          {#key params.code}
-            <DotnetProjectView code={params.code} type="crdt" />
-          {/key}
-        </Router>
-      </Route>
-      <Route path="/fwdata/:name/*" let:params>
-        <Router {url}>
-          {#key params.name}
-            <DotnetProjectView code={params.name} type="fwdata" />
-          {/key}
-        </Router>
-      </Route>
-      <Route path="/paratext/project/:code/*" let:params>
-        <Router {url}>
-          {#key params.code}
-            <DotnetProjectView code={params.code} type="crdt" paratext />
-          {/key}
-        </Router>
-      </Route>
-      <Route path="/paratext/fwdata/:name/*" let:params>
-        <Router {url}>
-          {#key params.name}
-            <DotnetProjectView code={params.name} type="fwdata" paratext />
-          {/key}
-        </Router>
-      </Route>
-      <Route path="/testing/project-view/*">
-        <Router {url}>
-          <TestProjectView />
-        </Router>
-      </Route>
-      <Route path="/">
-        <HomeView />
-      </Route>
-      <Route path="/sandbox">
-        <Sandbox />
-      </Route>
-      <Route path="/*">
-        {setTimeout(() => navigate('/', { replace: true }))}
-      </Route>
+    <Router>
+      <AppRoutes />
     </Router>
     <NotificationOutlet/>
   </div>

frontend/viewer/src/AppRoutes.svelte

@@ -0,0 +1,57 @@
+<script lang="ts">
+  import { setupGlobalErrorHandlers } from '$lib/errors/global-errors';
+  import { navigate, Route, Router, useLocation } from 'svelte-routing';
+  import Sandbox from '$lib/sandbox/Sandbox.svelte';
+  import DotnetProjectView from './DotnetProjectView.svelte';
+  import HomeView from './home/HomeView.svelte';
+  import TestProjectView from './TestProjectView.svelte';
+  import { initRootLocation } from '$lib/services/root-location-service';
+
+  let url = '';
+
+  setupGlobalErrorHandlers();
+  initRootLocation(useLocation());
+</script>
+
+<Route path="/project/:code/*" let:params>
+  <Router {url}>
+    {#key params.code}
+      <DotnetProjectView code={params.code} type="crdt" />
+    {/key}
+  </Router>
+</Route>
+<Route path="/fwdata/:name/*" let:params>
+  <Router {url}>
+    {#key params.name}
+      <DotnetProjectView code={params.name} type="fwdata" />
+    {/key}
+  </Router>
+</Route>
+<Route path="/paratext/project/:code/*" let:params>
+  <Router {url}>
+    {#key params.code}
+      <DotnetProjectView code={params.code} type="crdt" paratext />
+    {/key}
+  </Router>
+</Route>
+<Route path="/paratext/fwdata/:name/*" let:params>
+  <Router {url}>
+    {#key params.name}
+      <DotnetProjectView code={params.name} type="fwdata" paratext />
+    {/key}
+  </Router>
+</Route>
+<Route path="/testing/project-view/*">
+  <Router {url}>
+    <TestProjectView />
+  </Router>
+</Route>
+<Route path="/">
+  <HomeView />
+</Route>
+<Route path="/sandbox">
+  <Sandbox />
+</Route>
+<Route path="/*">
+  {setTimeout(() => navigate('/', { replace: true }))}
+</Route>

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

@@ -1,5 +1,5 @@
 <script lang="ts" module>
-  import {type Node} from 'prosemirror-model';
+  import {Fragment, Slice, type Node} from 'prosemirror-model';
   import {cn} from '$lib/utils';
   import {textSchema} from './editor-schema';
 
@@ -54,7 +54,8 @@
     } & HTMLAttributes<HTMLDivElement> = $props();
 
   let elementRef: HTMLElement | null = $state(null);
-  let dirty = $state(false);
+  // should not be state unless we catch errors. See onBlur.
+  let dirty = false;
   let editor: EditorView | null = null;
 
   const isUsingKeyboard = new IsUsingKeyboard();
@@ -92,6 +93,37 @@
       editable() {
         return !readonly;
       },
+      handlePaste(view, _event, slice) {
+        if (view.state.doc.content.size === 0) {
+          // if the field is cleared, the resulting selection breaks paste (on older devices at least)
+          selectAll(view);
+        }
+
+        // When a user copies a whole field. It includes the trailing <br>.
+        // Pasting it often causes errors, so we remove them.
+        function withoutBrs(nodes: readonly Node[]): Node[] {
+          return nodes
+            .filter(node => node.type.name !== textSchema.nodes.br.name)
+            .map(node => {
+              if (node.isText) return node;
+              return node.copy(Fragment.fromArray(withoutBrs(node.content.content)));
+            });
+        }
+        const cleanFragment = Fragment.fromArray(withoutBrs(slice.content.content));
+        const cleanSlice = new Slice(cleanFragment, slice.openStart, slice.openEnd);
+
+        // Below code is copied from prosemirror's doPaste
+        // https://github.com/ProseMirror/prosemirror-view/blob/381c163b0abde96cabd609a8c4fc72ed2891b0e1/src/input.ts#L624
+        function sliceSingleNode(slice: Slice): Node | null {
+            return slice.openStart == 0 && slice.openEnd == 0 && slice.content.childCount == 1 ? slice.content.firstChild : null;
+        }
+        let singleNode = sliceSingleNode(cleanSlice);
+        let tr = singleNode
+            ? view.state.tr.replaceSelectionWith(singleNode, false)
+            : view.state.tr.replaceSelection(cleanSlice);
+        view.dispatch(tr.scrollIntoView().setMeta('paste', true).setMeta('uiEvent', 'paste'));
+        return true;
+      },
       handleDOMEvents: {
         pointerdown() {
           pointerDown = true;
@@ -122,8 +154,16 @@
     if (usingKeyboard) { // tabbed in
       if (IsMobile.value) {
         if (prevSelection) {
-          const prevSelectionForCurrentDoc = Selection.fromJSON(editor.state.doc, prevSelection.toJSON());
-          setSelection(prevSelectionForCurrentDoc);
+          // We can land here when the field gets cleared for some reason.
+          // In that case fromJSON doesn't like the prevSelection (on older devices at least)
+          if (editor.state.doc.content.size) {
+            try {
+              const prevSelectionForCurrentDoc = Selection.fromJSON(editor.state.doc, prevSelection.toJSON());
+              setSelection(prevSelectionForCurrentDoc);
+            } catch {
+              console.warn('Could not restore previous selection', prevSelection.toJSON(), editor.state.doc);
+            }
+          }
           prevSelection = undefined;
         } else {
           setSelection(Selection.atEnd(editor.state.doc));
@@ -135,6 +175,9 @@
   }
 
   function onblur(editor: EditorView) {
+    // we cannot set and $state variables here, because if this is called due to navigation
+    // it's too late to update state and doing so will throw.
+    // See stomp-safe-lcm-rich-text-editor for how we handle that case.
     if (dirty && value) {
       onchange(value);
       dirty = false;
@@ -184,6 +227,11 @@
             if (dispatch) dispatch(state.tr.insertText(newLine));
             return true;
           },
+          // eslint-disable-next-line @typescript-eslint/naming-convention
+          'Backspace': (state) => {
+            // If the field is empty, backspace results in an error (on older devices at least)
+            return state.doc.content.size === 0;
+          },
         }),
         keymap(baseKeymap)
       ]

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

@@ -1,34 +1,47 @@
 <script lang="ts">
-  import {untrack, type ComponentProps} from 'svelte';
+  import {onDestroy, untrack, type ComponentProps} from 'svelte';
   import {StompGuard} from './stomp-guard.svelte';
   import {mergeProps} from 'bits-ui';
   import LcmRichTextEditor from '../lcm-rich-text-editor/lcm-rich-text-editor.svelte';
   import {useIdleService} from '$lib/services/idle-service';
+  import {useRootLocation} from '$lib/services/root-location-service';
 
   type Props = ComponentProps<typeof LcmRichTextEditor> & { onchange: () => void };
 
   let { value = $bindable(), onchange, ...rest}: Props = $props();
 
+  const locationService = useRootLocation();
+
   const guard = new StompGuard(
     () => value,
     (newValue) => value = newValue
   );
 
-  let idleService = useIdleService();
-  function onIdle() {
+  function commitAnyChanges() {
     if (guard.isDirty) {
       guard.commitAndUnlock();
       onchange();
     }
   }
 
+  let idleService = useIdleService();
   $effect(() => {
-    if (idleService.isIdle) untrack(onIdle);
+    if (idleService.isIdle) untrack(commitAnyChanges);
+  });
+  onDestroy(() => {
+    // This is just a precaution. I'm not aware of a scenario where we actually need to commit at this point.
+    commitAnyChanges();
+
+    return locationService.subscribe(() => {
+      // This handler is required, because the contenteditable blur event is too late when blurring due to navigation.
+      // Calling subscribe seems to be the only reliable way to get the callback triggered in that case.
+      commitAnyChanges();
+    });
   });
 </script>
 
 <LcmRichTextEditor bind:value={guard.value} {...mergeProps({
   onchange: () => {
-    guard.commitAndUnlock();
+    commitAnyChanges();
   }
-}, { onchange }, rest)} />
+}, rest)} />

frontend/viewer/src/lib/project-context.svelte.ts

@@ -6,7 +6,7 @@ import type {
 import type {
   ISyncServiceJsInvokable
 } from '$lib/dotnet-types/generated-types/FwLiteShared/Services/ISyncServiceJsInvokable';
-import {resource, type ResourceOptions, type ResourceReturn} from 'runed';
+import {resource, watchOnce, type ResourceOptions, type ResourceReturn} from 'runed';
 import {SvelteMap} from 'svelte/reactivity';
 import type {IProjectData} from '$lib/dotnet-types/generated-types/LcmCrdt/IProjectData';
 
@@ -130,6 +130,23 @@ export class ProjectContext {
         if (!api) return Promise.resolve(initialValue);
         return factory(api);
       }), {initialValue, ...options});
+
+    // If throttling and the api is not yet defined, the resource will likely throttle/swallow the refetch
+    // call triggered by the api becoming defined. As a result it will never be loaded. So, we explicitly ensure an initial load.
+    if (options?.throttle && !this.#api) {
+      function initialLoad(api: IMiniLcmJsInvokable) {
+        factory(api).then(res.mutate).catch(console.error);
+      }
+
+      if (this.#api) {
+        initialLoad(this.#api);
+      } else {
+        watchOnce(() => this.#api, () => {
+          if (this.#api) initialLoad(this.#api);
+          else console.warn('apiResource: initialLoad expected api to be defined after watchOnce');
+        });
+      }
+    }
     return res;
   }
 

frontend/viewer/src/lib/project-stats.ts

@@ -15,7 +15,7 @@ export function useProjectStats() {
       totalEntryCount,
     }
   }, {
-    throttle: 3000,
+    debounce: 500,
     onAdd: (resource) => {
       projectEventBus.onEntryDeleted(() => void resource.refetch());
       projectEventBus.onEntryUpdated(() => void resource.refetch());