From b16c5360e3a577c0d30d7419004c08663e56583b Mon Sep 17 00:00:00 2001 From: siltomato Date: Mon, 11 Aug 2025 20:38:12 -0400 Subject: [PATCH 1/4] fix backwards keyboard text selection across lynx insight --- .../src/app/shared/text/text.component.ts | 27 ++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/shared/text/text.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/shared/text/text.component.ts index e86f0193962..b4eda2e0e73 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/shared/text/text.component.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/shared/text/text.component.ts @@ -263,6 +263,7 @@ export class TextComponent implements AfterViewInit, OnDestroy { private presenceActiveEditor$: Subject = new Subject(); private onPresenceDocReceive = (_presenceId: string, _range: Range | null): void => {}; private onPresenceChannelReceive = (_presenceId: string, _presenceData: PresenceData | null): void => {}; + private isShiftDown = false; constructor( private readonly destroyRef: DestroyRef, @@ -525,6 +526,24 @@ export class TextComponent implements AfterViewInit, OnDestroy { .subscribe(() => { this.updateLocalCursor(); }); + + fromEvent(document, 'keydown') + .pipe(quietTakeUntilDestroyed(this.destroyRef)) + .subscribe(event => { + this.isShiftDown = event.shiftKey; + }); + + fromEvent(document, 'keyup') + .pipe(quietTakeUntilDestroyed(this.destroyRef)) + .subscribe(event => { + // Call 'update()' when shift key is released, as update is disabled while shift is down + // to prevent incorrect cursor position updates while selecting text. + if (this.isShiftDown && !event.shiftKey) { + this.update(); + } + + this.isShiftDown = event.shiftKey; + }); } ngOnDestroy(): void { @@ -827,7 +846,13 @@ export class TextComponent implements AfterViewInit, OnDestroy { } async onSelectionChanged(range: Range | null): Promise { - this.update(); + // During selection expansion (keyboard or mouse), avoid calling update() + // which can cause incorrect cursor position updates. + // Update will be called once the shift key is released. + if (!this.isShiftDown) { + this.update(); + } + this.submitLocalPresenceDoc(range); } From b9589b085581429f699e0814f58d44bdbcd3839b Mon Sep 17 00:00:00 2001 From: siltomato Date: Tue, 12 Aug 2025 11:24:42 -0400 Subject: [PATCH 2/4] add tests --- .../app/shared/text/text.component.spec.ts | 80 +++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/shared/text/text.component.spec.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/shared/text/text.component.spec.ts index 214e872f034..a9a98d08991 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/shared/text/text.component.spec.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/shared/text/text.component.spec.ts @@ -1557,6 +1557,86 @@ describe('TextComponent', () => { TestEnvironment.waitForPresenceTimer(); })); + + describe('Text selection behavior', () => { + it('should track shift key state correctly', fakeAsync(() => { + const env: TestEnvironment = new TestEnvironment(); + + // Initially shift key should be false + expect((env.component as any).isShiftDown).toBe(false); + + // Simulate shift key down + const keyDownEvent = new KeyboardEvent('keydown', { shiftKey: true }); + document.dispatchEvent(keyDownEvent); + tick(); + + expect((env.component as any).isShiftDown).toBe(true); + + // Simulate shift key up + const keyUpEvent = new KeyboardEvent('keyup', { shiftKey: false }); + document.dispatchEvent(keyUpEvent); + tick(); + + expect((env.component as any).isShiftDown).toBe(false); + })); + + it('should not call update() during selection expansion (shift down)', fakeAsync(() => { + const env: TestEnvironment = new TestEnvironment(); + env.component.onEditorCreated(new MockQuill('quill-editor')); + env.waitForEditor(); + + spyOn(env.component, 'update' as any); + + // Simulate shift key down + (env.component as any).isShiftDown = true; + + // Call onSelectionChanged with a selection (length > 0) + const range: QuillRange = { index: 5, length: 3 }; + env.component.onSelectionChanged(range); + tick(); + + // update() should not have been called + expect(env.component['update']).not.toHaveBeenCalled(); + })); + + it('should call update() when shift key is released', fakeAsync(() => { + const env: TestEnvironment = new TestEnvironment(); + env.component.onEditorCreated(new MockQuill('quill-editor')); + env.waitForEditor(); + + spyOn(env.component, 'update' as any); + + // Set shift key down initially + (env.component as any).isShiftDown = true; + + // Simulate shift key release + const keyUpEvent = new KeyboardEvent('keyup', { shiftKey: false }); + document.dispatchEvent(keyUpEvent); + tick(); + + // update() should have been called once + expect(env.component['update']).toHaveBeenCalledTimes(1); + })); + + it('should call update() when no selection is active (cursor only)', fakeAsync(() => { + const env: TestEnvironment = new TestEnvironment(); + env.component.onEditorCreated(new MockQuill('quill-editor')); + env.waitForEditor(); + + spyOn(env.component, 'update' as any); + + // Simulate shift key not pressed + (env.component as any).isShiftDown = false; + + // Call onSelectionChanged with cursor only (length = 0) + const range: QuillRange = { index: 5, length: 0 }; + env.component.onSelectionChanged(range); + tick(); + + // update() should have been called + expect(env.component['update']).toHaveBeenCalledTimes(1); + })); + }); }); class MockDragEvent extends DragEvent { From 1de547cb4ab9b8d8e496099e9656db0cbf9ee377 Mon Sep 17 00:00:00 2001 From: siltomato Date: Wed, 13 Aug 2025 12:58:53 -0400 Subject: [PATCH 3/4] fix case of tracking modifiers when window loses focus --- .../src/app/shared/text/text.component.ts | 31 ++++++++++++++----- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/shared/text/text.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/shared/text/text.component.ts index b4eda2e0e73..1508c2373d3 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/shared/text/text.component.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/shared/text/text.component.ts @@ -1,9 +1,11 @@ +import { DOCUMENT } from '@angular/common'; import { AfterViewInit, ChangeDetectorRef, Component, DestroyRef, EventEmitter, + Inject, Input, OnDestroy, Output @@ -21,6 +23,7 @@ import { fromEvent, Subject, Subscription, timer } from 'rxjs'; import { takeUntil } from 'rxjs/operators'; import { LocalPresence, Presence } from 'sharedb/lib/sharedb'; import tinyColor from 'tinycolor2'; +import { WINDOW } from 'xforge-common/browser-globals'; import { DialogService } from 'xforge-common/dialog.service'; import { LocaleDirection } from 'xforge-common/models/i18n-locale'; import { UserDoc } from 'xforge-common/models/user-doc'; @@ -275,7 +278,9 @@ export class TextComponent implements AfterViewInit, OnDestroy { private readonly userService: UserService, readonly viewModel: TextViewModel, private readonly textDocService: TextDocService, - private readonly quillFormatRegistry: QuillFormatRegistryService + private readonly quillFormatRegistry: QuillFormatRegistryService, + @Inject(DOCUMENT) private document: Document, + @Inject(WINDOW) private window: Window ) { let localCursorColor = localStorage.getItem(this.cursorColorStorageKey); if (localCursorColor == null) { @@ -521,19 +526,19 @@ export class TextComponent implements AfterViewInit, OnDestroy { // Listening to document 'selectionchange' event allows local cursor to change position on mousedown, // as opposed to quill 'onSelectionChange' event that doesn't fire until mouseup. - fromEvent(document, 'selectionchange') + fromEvent(this.document, 'selectionchange') .pipe(quietTakeUntilDestroyed(this.destroyRef)) .subscribe(() => { this.updateLocalCursor(); }); - fromEvent(document, 'keydown') + fromEvent(this.document, 'keydown') .pipe(quietTakeUntilDestroyed(this.destroyRef)) .subscribe(event => { this.isShiftDown = event.shiftKey; }); - fromEvent(document, 'keyup') + fromEvent(this.document, 'keyup') .pipe(quietTakeUntilDestroyed(this.destroyRef)) .subscribe(event => { // Call 'update()' when shift key is released, as update is disabled while shift is down @@ -544,6 +549,18 @@ export class TextComponent implements AfterViewInit, OnDestroy { this.isShiftDown = event.shiftKey; }); + + fromEvent(this.window, 'blur') + .pipe(quietTakeUntilDestroyed(this.destroyRef)) + .subscribe(() => { + // Treat window blur as releasing all modifiers + const wasShiftDown: boolean = this.isShiftDown === true; + if (wasShiftDown) { + this.update(); + } + + this.isShiftDown = false; + }); } ngOnDestroy(): void { @@ -564,7 +581,7 @@ export class TextComponent implements AfterViewInit, OnDestroy { fromEvent(this._editor.root, 'scroll') .pipe(quietTakeUntilDestroyed(this.destroyRef)) .subscribe(() => this.updateHighlightMarkerVisibility()); - fromEvent(window, 'resize') + fromEvent(this.window, 'resize') .pipe(quietTakeUntilDestroyed(this.destroyRef)) .subscribe(() => this.setHighlightMarkerPosition()); this.viewModel.editor = editor; @@ -1144,7 +1161,7 @@ export class TextComponent implements AfterViewInit, OnDestroy { const cursors: QuillCursors = this.editor.getModule('cursors') as QuillCursors; cursors.createCursor(this.presenceId, '', ''); - this.localCursorElement = document.querySelector(`#ql-cursor-${this.presenceId}`); + this.localCursorElement = this.document.querySelector(`#ql-cursor-${this.presenceId}`); // Add a specific class to the local cursor if (this.localCursorElement != null) { @@ -1158,7 +1175,7 @@ export class TextComponent implements AfterViewInit, OnDestroy { return; } - const sel: Selection | null = window.getSelection(); + const sel: Selection | null = this.window.getSelection(); if (sel == null) { return; } From fdab86747d5c231c29a16b1670fad606a4ff2bfe Mon Sep 17 00:00:00 2001 From: siltomato Date: Fri, 29 Aug 2025 16:42:19 -0400 Subject: [PATCH 4/4] fix press and hold cursor lag when lynx enabled --- .../src/app/shared/text/text.component.html | 2 +- .../src/app/shared/text/text.component.scss | 2 +- .../src/app/shared/text/text.component.ts | 82 ++++++++++++++++--- 3 files changed, 74 insertions(+), 12 deletions(-) diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/shared/text/text.component.html b/src/SIL.XForge.Scripture/ClientApp/src/app/shared/text/text.component.html index aaebfe247ff..050613cfffd 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/shared/text/text.component.html +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/shared/text/text.component.html @@ -21,7 +21,7 @@ rtl: isRtl, 'mark-invalid': markInvalid, 'selectable-verses': selectableVerses, - 'custom-local-cursor': showInsights + 'custom-local-cursor': showInsights && !isCursorMoveKeyDown }" [dir]="$any(textDirection)" [lang]="lang" diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/shared/text/text.component.scss b/src/SIL.XForge.Scripture/ClientApp/src/app/shared/text/text.component.scss index 12e3ec88974..d5d50d228cd 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/shared/text/text.component.scss +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/shared/text/text.component.scss @@ -13,7 +13,7 @@ quill-editor { } .ql-cursor-caret { - width: 1px; + width: 1.5px; background-color: $local-cursor-color; } diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/shared/text/text.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/shared/text/text.component.ts index 1508c2373d3..1b69c11528a 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/shared/text/text.component.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/shared/text/text.component.ts @@ -20,7 +20,7 @@ import { SFProjectRole } from 'realtime-server/lib/esm/scriptureforge/models/sf- import { TextAnchor } from 'realtime-server/lib/esm/scriptureforge/models/text-anchor'; import { StringMap } from 'rich-text'; import { fromEvent, Subject, Subscription, timer } from 'rxjs'; -import { takeUntil } from 'rxjs/operators'; +import { takeUntil, tap } from 'rxjs/operators'; import { LocalPresence, Presence } from 'sharedb/lib/sharedb'; import tinyColor from 'tinycolor2'; import { WINDOW } from 'xforge-common/browser-globals'; @@ -113,12 +113,39 @@ export class TextComponent implements AfterViewInit, OnDestroy { @Output() editorCreated = new EventEmitter(); lang: string = ''; + + /** + * Flag activated when user presses and holds keys that cause the cursor to move. + * A true value will cause the system cursor to be used instead of + * Quill custom local cursor in order to avoid cursor lag. + */ + isCursorMoveKeyDown = false; + // only use USX formats and not default Quill formats readonly allowedFormats: string[] = this.quillFormatRegistry.getRegisteredFormats(); // allow for different CSS based on the browser engine readonly browserEngine: string = getBrowserEngine(); readonly cursorColor: string; + /** Set of currently pressed keys that move the cursor. */ + private readonly pressedCursorMoveKeys = new Set(); + + /** Set of non-printable keys that move the cursor. */ + private readonly nonPrintableCursorMoveKeys = new Set([ + 'ArrowLeft', + 'ArrowRight', + 'ArrowUp', + 'ArrowDown', + 'Home', + 'End', + 'PageUp', + 'PageDown', + 'Tab' + ]); + + private cursorMoveKeyHoldTimeout?: any; + private cursorMoveKeyHoldDelay: number = 500; // Press and hold ms delay before switching to system cursor + private clickSubs: Map = new Map(); private _isReadOnly: boolean = true; private _editorStyles: any = { fontSize: '1rem' }; @@ -533,21 +560,56 @@ export class TextComponent implements AfterViewInit, OnDestroy { }); fromEvent(this.document, 'keydown') - .pipe(quietTakeUntilDestroyed(this.destroyRef)) + .pipe( + quietTakeUntilDestroyed(this.destroyRef), + tap(event => (this.isShiftDown = event.shiftKey)) + ) .subscribe(event => { - this.isShiftDown = event.shiftKey; + // Set flag to use system cursor when any key is down that would move the cursor (avoids cursor lag issue) + if (this.nonPrintableCursorMoveKeys.has(event.key) || event.key.length === 1) { + this.pressedCursorMoveKeys.add(event.key); + + // Only set the flag when the user presses and holds (detect with a short timeout delay) + if (!this.isCursorMoveKeyDown && this.cursorMoveKeyHoldTimeout == null) { + this.cursorMoveKeyHoldTimeout = setTimeout(() => { + if (this.pressedCursorMoveKeys.size > 0) { + this.isCursorMoveKeyDown = true; + } + + this.cursorMoveKeyHoldTimeout = undefined; + }, this.cursorMoveKeyHoldDelay); + } + } }); fromEvent(this.document, 'keyup') - .pipe(quietTakeUntilDestroyed(this.destroyRef)) + .pipe( + quietTakeUntilDestroyed(this.destroyRef), + tap(event => { + // Call 'update()' when shift key is released, as update is disabled while shift is down + // to prevent incorrect cursor position updates while selecting text. + if (this.isShiftDown && !event.shiftKey) { + this.update(); + } + + this.isShiftDown = event.shiftKey; + }) + ) .subscribe(event => { - // Call 'update()' when shift key is released, as update is disabled while shift is down - // to prevent incorrect cursor position updates while selecting text. - if (this.isShiftDown && !event.shiftKey) { - this.update(); - } + this.pressedCursorMoveKeys.delete(event.key); - this.isShiftDown = event.shiftKey; + // If set is empty, all cursor movement keys are released + if (this.pressedCursorMoveKeys.size === 0) { + if (this.cursorMoveKeyHoldTimeout) { + clearTimeout(this.cursorMoveKeyHoldTimeout); + this.cursorMoveKeyHoldTimeout = undefined; + } + + // Helps to not yet show custom local cursor until it has caught up to system cursor that was just visible + requestAnimationFrame(() => { + this.isCursorMoveKeyDown = false; + }); + } }); fromEvent(this.window, 'blur')