SF-3608 Fix lynx blot not re-rendering with note embed changes (#3549)

Author: siltomato

src/SIL.XForge.Scripture/ClientApp/src/app/shared/text/text-view-model.ts

@@ -3,7 +3,7 @@ import { VerseRef } from '@sillsdev/scripture';
 import { cloneDeep } from 'lodash-es';
 import Quill, { Delta, EmitterSource, Range } from 'quill';
 import { DeltaOperation, StringMap } from 'rich-text';
-import { BehaviorSubject, Subscription } from 'rxjs';
+import { BehaviorSubject, Subject, Subscription } from 'rxjs';
 import { isString } from '../../../type-utils';
 import { TextDoc, TextDocId } from '../../core/models/text-doc';
 import { LynxTextModelConverter } from '../../translate/editor/lynx/insights/lynx-editor';
@@ -128,18 +128,22 @@ class SegmentInfo {
 export class TextViewModel implements OnDestroy, LynxTextModelConverter {
   editor?: Quill;
 
-  private readonly _segments: Map<string, Range> = new Map<string, Range>();
   private changesSub?: Subscription;
   private onCreateSub?: Subscription;
   private textDoc?: TextDoc;
   private textDocId?: TextDocId;
+
   /**
    * A mapping of IDs of elements embedded into the quill editor to their positions.
    * These elements are in addition to the text data i.e. Note threads
    */
   private _embeddedElements: Map<string, EmbedPosition> = new Map<string, EmbedPosition>();
 
-  segments$ = new BehaviorSubject<ReadonlyMap<string, Range>>(this._segments);
+  private readonly _segments: Map<string, Range> = new Map<string, Range>();
+  private readonly segments$ = new BehaviorSubject<ReadonlyMap<string, Range>>(this._segments);
+
+  private readonly _embedPositionsChanged$ = new Subject<void>();
+  readonly embedPositionsChanged$ = this._embedPositionsChanged$.asObservable();
 
   get segments(): IterableIterator<[string, Range]> {
     return this._segments.entries();
@@ -780,6 +784,7 @@ export class TextViewModel implements OnDestroy, LynxTextModelConverter {
     }
 
     this.segments$.next(this._segments);
+    this._embedPositionsChanged$.next();
 
     return convertDelta.compose(fixDelta).chop();
   }

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/lynx/insights/lynx-editor.ts

@@ -1,5 +1,6 @@
 import { Injectable } from '@angular/core';
 import Quill, { Delta, Op, Range } from 'quill';
+import { Observable } from 'rxjs';
 import { QuillLynxEditorAdapter } from './quill-services/quill-lynx-editor-adapter';
 
 export type LynxableEditor = Quill; // Add future editor as union type
@@ -44,6 +45,13 @@ export interface LynxTextModelConverter {
    * Useful when embeds that are present only in the editor model may affect position offsets from Lynx.
    */
   getEmbedCountsToOffsetFunc(): LynxCountToOffsetFunc;
+
+  /**
+   * Observable that emits when embedded element positions change.
+   * This is useful for detecting when note embeds are added/removed (including remotely),
+   * which affect the mapping between data model positions and editor positions.
+   */
+  readonly embedPositionsChanged$: Observable<void>;
 }
 
 @Injectable({ providedIn: 'root' })

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/lynx/insights/lynx-insight-editor-objects/lynx-insight-editor-objects.component.spec.ts

@@ -2,7 +2,7 @@ import { Component, DestroyRef, NO_ERRORS_SCHEMA, ViewChild } from '@angular/cor
 import { ComponentFixture, fakeAsync, flush, TestBed, tick } from '@angular/core/testing';
 import { Delta } from 'quill';
 import { LynxInsightFilter, LynxInsightType } from 'realtime-server/lib/esm/scriptureforge/models/lynx-insight';
-import { BehaviorSubject } from 'rxjs';
+import { BehaviorSubject, Subject } from 'rxjs';
 import { anything, instance, mock, verify, when } from 'ts-mockito';
 import { ActivatedBookChapterService, RouteBookChapter } from 'xforge-common/activated-book-chapter.service';
 import { configureTestingModule } from 'xforge-common/test-utils';
@@ -350,6 +350,76 @@ describe('LynxInsightEditorObjectsComponent', () => {
       verify(mockInsightRenderService.removeAllInsightFormatting(anything())).atLeast(1);
     }));
   });
+
+  describe('embed position changes', () => {
+    it('should re-render insights when embed positions change', fakeAsync(() => {
+      // Start with editor not ready
+      const env = new TestEnvironment({ initialEditorReady: false });
+      const testInsight = env.createTestInsight();
+
+      env.setFilteredInsights([testInsight]);
+      tick();
+      flush();
+
+      // No render calls, as editor not ready yet
+      verify(mockInsightRenderService.renderActionOverlay(anything(), anything(), anything(), anything())).never();
+      verify(mockInsightRenderService.render(anything(), anything())).never();
+
+      // Editor ready should trigger first render of insights and action overlay
+      env.setEditorReady(true);
+      tick();
+      flush();
+      verify(mockInsightRenderService.render(anything(), anything())).once();
+      verify(mockInsightRenderService.renderActionOverlay(anything(), anything(), anything(), anything())).once();
+
+      // Embed position change should trigger second insights render, but not action overlay render
+      env.triggerEmbedPositionChange();
+      tick(env.component.embedPositionsChangedDebounceTime);
+      flush();
+      verify(mockInsightRenderService.render(anything(), anything())).twice();
+      verify(mockInsightRenderService.renderActionOverlay(anything(), anything(), anything(), anything())).once();
+    }));
+
+    it('should not reset overlay state when embed positions change', fakeAsync(() => {
+      // Start with editor not ready
+      const env = new TestEnvironment({ initialEditorReady: false });
+      const testInsight = env.createTestInsight();
+
+      env.setFilteredInsights([testInsight]);
+      tick();
+      flush();
+
+      // No render calls, as editor not ready yet
+      verify(mockInsightRenderService.renderActionOverlay(anything(), anything(), anything(), anything())).never();
+      verify(mockInsightRenderService.render(anything(), anything())).never();
+
+      // Editor ready should trigger first render of insights and action overlay
+      env.setEditorReady(true);
+      tick();
+      flush();
+      verify(mockInsightRenderService.render(anything(), anything())).once();
+      verify(mockInsightRenderService.renderActionOverlay(anything(), anything(), anything(), anything())).once();
+
+      // Set display state to trigger action overlay
+      env.setDisplayState({
+        activeInsightIds: [testInsight.id],
+        actionOverlayActive: true,
+        promptActive: true,
+        cursorActiveInsightIds: []
+      });
+      tick();
+      flush();
+      verify(mockInsightRenderService.render(anything(), anything())).once(); // No new insight render
+      verify(mockInsightRenderService.renderActionOverlay(anything(), anything(), anything(), anything())).twice();
+
+      // Embed position change should trigger second insights render, but not action overlay render
+      env.triggerEmbedPositionChange();
+      tick(env.component.embedPositionsChangedDebounceTime);
+      flush();
+      verify(mockInsightRenderService.render(anything(), anything())).twice();
+      verify(mockInsightRenderService.renderActionOverlay(anything(), anything(), anything(), anything())).twice();
+    }));
+  });
 });
 
 @Component({
@@ -390,6 +460,7 @@ class TestEnvironment {
   private filterSubject: BehaviorSubject<LynxInsightFilter>;
   private filteredInsightCountsByTypeSubject: BehaviorSubject<Record<LynxInsightType, number>>;
   private taskRunningStatusSubject: BehaviorSubject<boolean>;
+  private embedPositionsChangedSubject: Subject<void>;
 
   constructor(args: TestEnvArgs = {}) {
     const textModelConverter = instance(mockTextModelConverter);
@@ -415,6 +486,8 @@ class TestEnvironment {
     this.filteredInsightCountsByTypeSubject = new BehaviorSubject<any>({ info: 0, warning: 0, error: 0 });
     this.taskRunningStatusSubject = new BehaviorSubject<boolean>(false);
 
+    this.embedPositionsChangedSubject = new Subject<void>();
+
     // Create mock editor
     const mockRoot = document.createElement('div');
     const actualEditor = {
@@ -460,6 +533,7 @@ class TestEnvironment {
     when(mockOverlayService.isOpen).thenReturn(false);
     when(mockLynxWorkspaceService.getOnTypeEdits(anything(), anything())).thenResolve([]);
     when(mockTextModelConverter.dataDeltaToEditorDelta(anything())).thenCall((delta: Delta) => delta);
+    when(mockTextModelConverter.embedPositionsChanged$).thenReturn(this.embedPositionsChangedSubject);
 
     // Setup text model converter to return ranges as-is (prevents null range issues)
     when(mockTextModelConverter.dataRangeToEditorRange(anything())).thenCall((range: LynxInsightRange) => range);
@@ -518,6 +592,10 @@ class TestEnvironment {
     }
   }
 
+  triggerEmbedPositionChange(): void {
+    this.embedPositionsChangedSubject.next();
+  }
+
   createTestInsight(props: Partial<LynxInsight> = {}): LynxInsight {
     return {
       id: props.id ?? 'test-insight-1',

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/lynx/insights/lynx-insight-editor-objects/lynx-insight-editor-objects.component.ts

@@ -14,7 +14,7 @@ import {
   switchMap,
   tap
 } from 'rxjs';
-import { distinctUntilChanged, map, observeOn, scan } from 'rxjs/operators';
+import { debounceTime, distinctUntilChanged, map, observeOn, scan, withLatestFrom } from 'rxjs/operators';
 import { ActivatedBookChapterService } from 'xforge-common/activated-book-chapter.service';
 import { quietTakeUntilDestroyed } from 'xforge-common/util/rxjs-util';
 import { EditorReadyService } from '../base-services/editor-ready.service';
@@ -39,15 +39,16 @@ import { LynxInsightActionPromptComponent } from '../lynx-insight-action-prompt/
   ]
 })
 export class LynxInsightEditorObjectsComponent implements OnChanges, OnInit, OnDestroy {
-  readonly insightSelector = `.${LynxInsightBlot.superClassName}`;
-
-  private readonly dataIdProp = LynxInsightBlot.idDatasetPropName;
-
   @Input() editor?: LynxableEditor;
   @Input() lynxTextModelConverter?: LynxTextModelConverter;
   @Input() autoCorrectionsEnabled: boolean = false;
   @Input() insightsEnabled: boolean = false;
 
+  readonly embedPositionsChangedDebounceTime = 100;
+
+  readonly insightSelector = `.${LynxInsightBlot.superClassName}`;
+  private readonly dataIdProp = LynxInsightBlot.idDatasetPropName;
+
   private isEditorMouseDown = false;
   private insightsEnabled$ = new BehaviorSubject<boolean>(this.insightsEnabled);
 
@@ -153,7 +154,9 @@ export class LynxInsightEditorObjectsComponent implements OnChanges, OnInit, OnD
               }
 
               return merge(
-                // Render blots when insights change
+                // Render blots when insights change OR when note embeds are added/removed.
+                // Only reset chapterInsightsRendered$ when insights change (not embeds)
+                // to avoid clearing insight overlay.
                 this.insightState.filteredChapterInsights$.pipe(
                   switchMap(insights => {
                     chapterInsightsRendered$.next(false);
@@ -166,6 +169,18 @@ export class LynxInsightEditorObjectsComponent implements OnChanges, OnInit, OnD
                     ).pipe(tap(() => chapterInsightsRendered$.next(true)));
                   })
                 ),
+                (this.lynxTextModelConverter?.embedPositionsChanged$ ?? EMPTY).pipe(
+                  debounceTime(this.embedPositionsChangedDebounceTime),
+                  withLatestFrom(this.insightState.filteredChapterInsights$),
+                  switchMap(([_, insights]) => {
+                    return from(
+                      this.insightRenderService.render(
+                        insights.map(insight => this.adjustInsightRange(insight)),
+                        this.editor!
+                      )
+                    );
+                  })
+                ),
                 // Ensure insights are rendered before responding to display state changes,
                 // as overlay needs to anchor to insight elements in the editor.
                 chapterInsightsRendered$.pipe(