Skip to content

Commit dd51312

Browse files
ddaspitddaspit
committed
SF-3418 Lynx fix text is being inserted at wrong index
1 parent ed2c2a6 commit dd51312

File tree

10 files changed

+215
-31
lines changed

10 files changed

+215
-31
lines changed

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { DeltaOperation, StringMap } from 'rich-text';
66
import { BehaviorSubject, Subscription } from 'rxjs';
77
import { isString } from '../../../type-utils';
88
import { TextDoc, TextDocId } from '../../core/models/text-doc';
9-
import { LynxRangeConverter } from '../../translate/editor/lynx/insights/lynx-editor';
9+
import { LynxTextModelConverter } from '../../translate/editor/lynx/insights/lynx-editor';
1010
import { getVerseStrFromSegmentRef, isBadDelta } from '../utils';
1111
import { getAttributesAtPosition, getRetainCount } from './quill-util';
1212
import { USFM_STYLE_DESCRIPTIONS } from './usfm-style-descriptions';
@@ -122,7 +122,7 @@ class SegmentInfo {
122122
* See text.component.spec.ts for some unit tests.
123123
*/
124124
@Injectable()
125-
export class TextViewModel implements OnDestroy, LynxRangeConverter {
125+
export class TextViewModel implements OnDestroy, LynxTextModelConverter {
126126
editor?: Quill;
127127

128128
private readonly _segments: Map<string, Range> = new Map<string, Range>();
@@ -582,6 +582,10 @@ export class TextViewModel implements OnDestroy, LynxRangeConverter {
582582
return { index: startEditorPos, length: endEditorPos - startEditorPos };
583583
}
584584

585+
dataDeltaToEditorDelta(dataDelta: Delta): Delta {
586+
return this.addEmbeddedElementsToDelta(dataDelta);
587+
}
588+
585589
private countSequentialEmbedsStartingAt(startEditorPosition: number): number {
586590
const embedEditorPositions = this.embedPositions;
587591
// add up the leading embeds

‎src/SIL.XForge.Scripture/ClientApp/src/app/shared/text/text.component.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
@if (showInsights && editor != null) {
2-
<app-lynx-insight-editor-objects [editor]="editor" [lynxRangeConverter]="viewModel" />
2+
<app-lynx-insight-editor-objects [editor]="editor" [lynxTextModelConverter]="viewModel" />
33
}
44

55
<!-- translate="no" is to prevent tools like Google translate from editing the text -->

‎src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/lynx/insights/base-services/insight-render.service.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,16 @@
11
import { Injectable } from '@angular/core';
2-
import { LynxableEditor, LynxRangeConverter } from '../lynx-editor';
2+
import { LynxableEditor, LynxTextModelConverter } from '../lynx-editor';
33
import { LynxInsight } from '../lynx-insight';
44

55
@Injectable()
66
export abstract class InsightRenderService {
7-
abstract render(insights: LynxInsight[], editor: LynxableEditor, rangeConverter: LynxRangeConverter): void;
7+
abstract render(insights: LynxInsight[], editor: LynxableEditor, textModelConverter: LynxTextModelConverter): void;
88
abstract removeAllInsightFormatting(editor: LynxableEditor): void;
9-
abstract renderActionOverlay(insights: LynxInsight[], editor: LynxableEditor, actionOverlayActive: boolean): void;
9+
abstract renderActionOverlay(
10+
insights: LynxInsight[],
11+
editor: LynxableEditor,
12+
textModelConverter: LynxTextModelConverter,
13+
actionOverlayActive: boolean
14+
): void;
1015
abstract renderCursorActiveState(insightIds: string[], editor: LynxableEditor): void;
1116
}

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ export interface LynxEditor {
1919
getRoot(): HTMLElement;
2020
}
2121

22-
export interface LynxRangeConverter {
22+
export interface LynxTextModelConverter {
2323
/**
2424
* Translates a range from the data model to the editor.
2525
* Useful when embeds that are present only in the editor model may affect
@@ -28,6 +28,14 @@ export interface LynxRangeConverter {
2828
* @returns The corresponding range in the editor model, or the original range as a fallback.
2929
*/
3030
dataRangeToEditorRange(dataRange: Range): Range;
31+
/**
32+
* Translates the data model delta to the editor model delta.
33+
* Useful when embeds that are present only in the editor model may affect
34+
* update ops from Lynx.
35+
* @param dataDelta The data model delta.
36+
* @returns The corresponding editor model delta.
37+
*/
38+
dataDeltaToEditorDelta(dataDelta: Delta): Delta;
3139
}
3240

3341
@Injectable({ providedIn: 'root' })

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

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { map, observeOn, scan } from 'rxjs/operators';
77
import { quietTakeUntilDestroyed } from 'xforge-common/util/rxjs-util';
88
import { EditorReadyService } from '../base-services/editor-ready.service';
99
import { InsightRenderService } from '../base-services/insight-render.service';
10-
import { LynxableEditor, LynxRangeConverter } from '../lynx-editor';
10+
import { LynxableEditor, LynxTextModelConverter } from '../lynx-editor';
1111
import { LynxInsight, LynxInsightDisplayState, LynxInsightRange } from '../lynx-insight';
1212
import { LynxInsightOverlayService } from '../lynx-insight-overlay.service';
1313
import { LynxInsightStateService } from '../lynx-insight-state.service';
@@ -24,7 +24,7 @@ export class LynxInsightEditorObjectsComponent implements OnInit, OnDestroy {
2424
private readonly dataIdProp = LynxInsightBlot.idDatasetPropName;
2525

2626
@Input() editor?: LynxableEditor;
27-
@Input() lynxRangeConverter?: LynxRangeConverter;
27+
@Input() lynxTextModelConverter?: LynxTextModelConverter;
2828

2929
private isEditorMouseDown = false;
3030

@@ -39,7 +39,7 @@ export class LynxInsightEditorObjectsComponent implements OnInit, OnDestroy {
3939
) {}
4040

4141
ngOnInit(): void {
42-
if (this.editor == null || this.lynxRangeConverter == null) {
42+
if (this.editor == null || this.lynxTextModelConverter == null) {
4343
return;
4444
}
4545

@@ -89,7 +89,7 @@ export class LynxInsightEditorObjectsComponent implements OnInit, OnDestroy {
8989
return merge(
9090
// Render blots when insights change
9191
this.insightState.filteredChapterInsights$.pipe(
92-
tap(insights => this.insightRenderService.render(insights, this.editor!, this.lynxRangeConverter!))
92+
tap(insights => this.insightRenderService.render(insights, this.editor!, this.lynxTextModelConverter!))
9393
),
9494
// Check display state to render action overlay or cursor active state
9595
this.insightState.displayState$.pipe(
@@ -109,6 +109,7 @@ export class LynxInsightEditorObjectsComponent implements OnInit, OnDestroy {
109109
this.insightRenderService.renderActionOverlay(
110110
activeInsights,
111111
this.editor!,
112+
this.lynxTextModelConverter!,
112113
!!curr.actionOverlayActive
113114
);
114115
}
@@ -196,13 +197,13 @@ export class LynxInsightEditorObjectsComponent implements OnInit, OnDestroy {
196197
}
197198

198199
private async handleTextChange(delta: Delta): Promise<void> {
199-
if (this.editor == null) {
200+
if (this.editor == null || this.lynxTextModelConverter == null) {
200201
return;
201202
}
202203

203204
const edits = await this.lynxWorkspaceService.getOnTypeEdits(delta);
204205
for (const edit of edits) {
205-
this.editor.updateContents(edit, 'user');
206+
this.editor.updateContents(this.lynxTextModelConverter.dataDeltaToEditorDelta(edit), 'user');
206207
}
207208
}
208209
}

‎src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/lynx/insights/lynx-insight-overlay.service.spec.ts

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { fakeAsync, TestBed, tick } from '@angular/core/testing';
44
import { Subject } from 'rxjs';
55
import { anything, capture, instance, mock, verify, when } from 'ts-mockito';
66
import { configureTestingModule } from 'xforge-common/test-utils';
7-
import { LynxEditor } from './lynx-editor';
7+
import { LynxEditor, LynxTextModelConverter } from './lynx-editor';
88
import { LynxInsight } from './lynx-insight';
99
import { LynxInsightOverlayRef, LynxInsightOverlayService } from './lynx-insight-overlay.service';
1010
import { LynxInsightOverlayComponent } from './lynx-insight-overlay/lynx-insight-overlay.component';
@@ -29,9 +29,9 @@ describe('LynxInsightOverlayService', () => {
2929
describe('open()', () => {
3030
it('should not open overlay when insights array is empty', fakeAsync(() => {
3131
const env = new TestEnvironment();
32-
const { origin, editor } = env.createTestData();
32+
const { origin, editor, textModelConverter } = env.createTestData();
3333

34-
const result = env.service.open(origin, [], editor);
34+
const result = env.service.open(origin, [], editor, textModelConverter);
3535
tick();
3636

3737
expect(result).toBeUndefined();
@@ -248,13 +248,21 @@ class TestEnvironment {
248248
});
249249
}
250250

251-
createTestData(): { origin: HTMLElement; editor: LynxEditor; editorMock: LynxEditor; insights: LynxInsight[] } {
251+
createTestData(): {
252+
origin: HTMLElement;
253+
editor: LynxEditor;
254+
editorMock: LynxEditor;
255+
insights: LynxInsight[];
256+
textModelConverter: LynxTextModelConverter;
257+
} {
252258
const origin = document.createElement('div');
253259
const editorMock = mock<LynxEditor>();
254260
when(editorMock.getScrollingContainer()).thenReturn(this.containerElement);
255261
const editor = instance(editorMock);
256262
const insights = [{ id: 'insight1' } as LynxInsight];
257-
return { origin, editor, editorMock, insights };
263+
const textModelConverterMock = mock<LynxTextModelConverter>();
264+
when(textModelConverterMock.dataDeltaToEditorDelta(anything())).thenCall(delta => delta);
265+
return { origin, editor, editorMock, insights, textModelConverter: instance(textModelConverterMock) };
258266
}
259267

260268
/**
@@ -306,7 +314,7 @@ class TestEnvironment {
306314

307315
when(mockOverlay.create(anything())).thenReturn(this.overlayRefs[refIndex].instance);
308316
const testData = this.createTestData();
309-
const result = this.service.open(testData.origin, testData.insights, testData.editor)!;
317+
const result = this.service.open(testData.origin, testData.insights, testData.editor, testData.textModelConverter)!;
310318
tick(); // Process setTimeout
311319

312320
return { ...testData, result };

‎src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/lynx/insights/lynx-insight-overlay.service.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { ComponentPortal } from '@angular/cdk/portal';
33
import { ScrollDispatcher } from '@angular/cdk/scrolling';
44
import { Injectable, NgZone } from '@angular/core';
55
import { asyncScheduler, observeOn, Subject, take, takeUntil } from 'rxjs';
6-
import { LynxEditor } from './lynx-editor';
6+
import { LynxEditor, LynxTextModelConverter } from './lynx-editor';
77
import { LynxInsight } from './lynx-insight';
88
import { LynxInsightOverlayComponent } from './lynx-insight-overlay/lynx-insight-overlay.component';
99

@@ -30,7 +30,12 @@ export class LynxInsightOverlayService {
3030
return this.openRef != null;
3131
}
3232

33-
open(origin: HTMLElement, insights: LynxInsight[], editor: LynxEditor): LynxInsightOverlayRef | undefined {
33+
open(
34+
origin: HTMLElement,
35+
insights: LynxInsight[],
36+
editor: LynxEditor,
37+
textModelConverter: LynxTextModelConverter
38+
): LynxInsightOverlayRef | undefined {
3439
if (insights.length === 0) {
3540
return undefined;
3641
}
@@ -45,6 +50,7 @@ export class LynxInsightOverlayService {
4550

4651
componentRef.instance.insights = insights;
4752
componentRef.instance.editor = editor;
53+
componentRef.instance.textModelConverter = textModelConverter;
4854
componentRef.instance.insightDismiss.pipe(take(1)).subscribe(() => this.close());
4955
componentRef.instance.insightHover
5056
.pipe(takeUntil(overlayRef.closed$))

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

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
import { Component, ViewChild } from '@angular/core';
2+
import { ComponentFixture, fakeAsync, flush, TestBed } from '@angular/core/testing';
3+
import { By } from '@angular/platform-browser';
4+
import { TextDocId } from 'src/app/core/models/text-doc';
5+
import { anything, instance, mock, verify, when } from 'ts-mockito';
6+
import { configureTestingModule, TestTranslocoModule } from 'xforge-common/test-utils';
7+
import { UICommonModule } from 'xforge-common/ui-common.module';
8+
import { LynxEditor, LynxTextModelConverter } from '../lynx-editor';
9+
import { EDITOR_INSIGHT_DEFAULTS, LynxInsight, LynxInsightAction, LynxInsightConfig } from '../lynx-insight';
10+
import { LynxInsightOverlayService } from '../lynx-insight-overlay.service';
11+
import { LynxInsightStateService } from '../lynx-insight-state.service';
12+
import { LynxWorkspaceService } from '../lynx-workspace.service';
13+
import { LynxInsightOverlayComponent } from './lynx-insight-overlay.component';
14+
15+
const mockLynxInsightStateService = mock(LynxInsightStateService);
16+
const mockLynxInsightOverlayService = mock(LynxInsightOverlayService);
17+
const mockLynxWorkspaceService = mock(LynxWorkspaceService);
18+
const mockLynxEditor = mock<LynxEditor>();
19+
const mockTextModelConverter = mock<LynxTextModelConverter>();
20+
21+
// Default insight config
22+
const defaultInsightConfig: LynxInsightConfig = {
23+
filter: { types: ['info', 'warning', 'error'], scope: 'chapter' },
24+
sortOrder: 'severity',
25+
queryParamName: 'insight',
26+
actionOverlayApplyPrimaryActionChord: { altKey: true, shiftKey: true, key: 'Enter' },
27+
panelLinkTextGoalLength: 30
28+
};
29+
30+
@Component({
31+
template: `<app-lynx-insight-overlay
32+
#overlay
33+
[insights]="insights"
34+
[editor]="editor"
35+
[textModelConverter]="textModelConverter"
36+
></app-lynx-insight-overlay>`
37+
})
38+
class HostComponent {
39+
@ViewChild('overlay') component!: LynxInsightOverlayComponent;
40+
insights: LynxInsight[] = [];
41+
editor?: LynxEditor;
42+
textModelConverter?: LynxTextModelConverter;
43+
}
44+
45+
fdescribe('LynxInsightOverlayComponent', () => {
46+
configureTestingModule(() => ({
47+
imports: [UICommonModule, TestTranslocoModule],
48+
declarations: [HostComponent, LynxInsightOverlayComponent],
49+
providers: [
50+
{ provide: LynxInsightStateService, useMock: mockLynxInsightStateService },
51+
{ provide: LynxInsightOverlayService, useMock: mockLynxInsightOverlayService },
52+
{ provide: LynxWorkspaceService, useMock: mockLynxWorkspaceService },
53+
{ provide: EDITOR_INSIGHT_DEFAULTS, useValue: defaultInsightConfig }
54+
]
55+
}));
56+
57+
it('should create', fakeAsync(() => {
58+
const env = new TestEnvironment();
59+
expect(env.component).toBeTruthy();
60+
}));
61+
62+
it('should update contents when primary action clicked', fakeAsync(() => {
63+
const env = new TestEnvironment();
64+
65+
env.clickPrimaryActionLink();
66+
67+
verify(mockTextModelConverter.dataDeltaToEditorDelta(anything())).once();
68+
verify(mockLynxEditor.updateContents(anything(), 'user')).once();
69+
expect().nothing();
70+
}));
71+
});
72+
73+
class TestEnvironment {
74+
fixture: ComponentFixture<HostComponent>;
75+
hostComponent: HostComponent;
76+
component: LynxInsightOverlayComponent;
77+
78+
constructor() {
79+
// Setup the test environment
80+
this.fixture = TestBed.createComponent(HostComponent);
81+
this.hostComponent = this.fixture.componentInstance;
82+
this.setupEditor();
83+
this.component = this.hostComponent.component;
84+
}
85+
86+
private setupEditor(): void {
87+
const editor = instance(mockLynxEditor);
88+
const textModelConverter = instance(mockTextModelConverter);
89+
90+
const insight = this.createTestInsight();
91+
const action = this.createTestAction(insight, true);
92+
93+
// Set up mocks for the editor root element
94+
const mockRoot = document.createElement('div');
95+
when(mockLynxEditor.getRoot()).thenReturn(mockRoot);
96+
when(mockLynxEditor.focus()).thenReturn();
97+
when(mockLynxEditor.updateContents(anything(), anything())).thenReturn();
98+
when(mockTextModelConverter.dataDeltaToEditorDelta(anything())).thenCall(delta => delta);
99+
when(mockLynxWorkspaceService.getActions(anything())).thenResolve([action]);
100+
101+
this.hostComponent.insights = [insight];
102+
this.hostComponent.editor = editor;
103+
this.hostComponent.textModelConverter = textModelConverter;
104+
this.fixture.detectChanges();
105+
flush();
106+
this.fixture.detectChanges();
107+
}
108+
109+
clickPrimaryActionLink(): void {
110+
const link = this.fixture.debugElement.query(By.css('.primary-action a'));
111+
link.triggerEventHandler('click');
112+
this.fixture.detectChanges();
113+
}
114+
115+
private createTestInsight(props: Partial<LynxInsight> = {}): LynxInsight {
116+
return {
117+
id: props.id ?? 'test-insight-1',
118+
type: props.type ?? 'warning',
119+
textDocId: props.textDocId ?? new TextDocId('project01', 40, 1),
120+
range: props.range ?? { index: 5, length: 10 },
121+
code: props.code ?? 'TEST001',
122+
source: props.source ?? 'test-source',
123+
description: props.description ?? 'Test insight description',
124+
moreInfo: props.moreInfo,
125+
data: props.data,
126+
...props
127+
};
128+
}
129+
130+
private createTestAction(insight: LynxInsight, isPrimary: boolean = false): LynxInsightAction {
131+
return {
132+
id: `action-${insight.id}`,
133+
insight: insight,
134+
label: `Action for ${insight.description}`,
135+
description: 'Test action description',
136+
isPrimary: isPrimary,
137+
ops: [{ insert: 'test action text' }]
138+
};
139+
}
140+
}

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
import { DOCUMENT } from '@angular/common';
22
import { Component, DestroyRef, ElementRef, EventEmitter, Inject, Input, OnInit, Output } from '@angular/core';
33
import { MAT_TOOLTIP_DEFAULT_OPTIONS } from '@angular/material/tooltip';
4+
import { Delta } from 'quill';
45
import { fromEvent } from 'rxjs';
56
import { quietTakeUntilDestroyed } from 'xforge-common/util/rxjs-util';
6-
import { LynxEditor } from '../lynx-editor';
7+
import { LynxEditor, LynxTextModelConverter } from '../lynx-editor';
78
import { EDITOR_INSIGHT_DEFAULTS, LynxInsight, LynxInsightAction, LynxInsightConfig } from '../lynx-insight';
89
import { LynxInsightOverlayService } from '../lynx-insight-overlay.service';
910
import { LynxInsightStateService } from '../lynx-insight-state.service';
@@ -36,6 +37,7 @@ export class LynxInsightOverlayComponent implements OnInit {
3637
}
3738

3839
@Input() editor?: LynxEditor;
40+
@Input() textModelConverter?: LynxTextModelConverter;
3941

4042
/** Emits when insight is dismissed by user. */
4143
@Output() insightDismiss = new EventEmitter<LynxInsight>();
@@ -110,11 +112,11 @@ export class LynxInsightOverlayComponent implements OnInit {
110112
}
111113

112114
selectAction(action: LynxInsightAction): void {
113-
if (this.editor == null) {
115+
if (this.editor == null || this.textModelConverter == null) {
114116
return;
115117
}
116118

117-
this.editor.updateContents(action.ops, 'user');
119+
this.editor.updateContents(this.textModelConverter.dataDeltaToEditorDelta(new Delta(action.ops)), 'user');
118120

119121
if (this.focusedInsight == null) {
120122
throw new Error('No focused insight');

0 commit comments

Comments
 (0)