From a54053bec6c079ee8ad7d54c83d179f9b40615e9 Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Thu, 21 Mar 2024 10:08:23 +0700 Subject: [PATCH 1/5] fix(web): prevent layer switch key from erasing selection Fixes #7866. When the transform generated by a key event results in no changes to the text, this will no longer trigger a change event in the apps. This means that layer switch keys will no longer erase the selection. Fix proposed by @jahorton. --- .../web/keyboard-processor/src/text/outputTarget.ts | 12 +++++++----- web/src/app/webview/src/contextManager.ts | 4 +++- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/common/web/keyboard-processor/src/text/outputTarget.ts b/common/web/keyboard-processor/src/text/outputTarget.ts index ab9656b699..6ad579d1fa 100644 --- a/common/web/keyboard-processor/src/text/outputTarget.ts +++ b/common/web/keyboard-processor/src/text/outputTarget.ts @@ -21,15 +21,17 @@ export function isEmptyTransform(transform: Transform) { export class TextTransform implements Transform { readonly insert: string; readonly deleteLeft: number; - readonly deleteRight?: number; + readonly deleteRight: number; + readonly erasedSelection: boolean; - constructor(insert: string, deleteLeft: number, deleteRight?: number) { + constructor(insert: string, deleteLeft: number, deleteRight: number, erasedSelection: boolean) { this.insert = insert; this.deleteLeft = deleteLeft; - this.deleteRight = deleteRight || 0; + this.deleteRight = deleteRight; + this.erasedSelection = erasedSelection; } - public static readonly nil = new TextTransform('', 0, 0); + public static readonly nil = new TextTransform('', 0, 0, false); } export class Transcription { @@ -138,7 +140,7 @@ export default abstract class OutputTarget { // caret mid-word.. const deletedRight = fromRight.substring(0, rightDivergenceIndex + 1)._kmwLength(); - return new TextTransform(insertedText, deletedLeft, deletedRight); + return new TextTransform(insertedText, deletedLeft, deletedRight, original.getSelectedText() && !this.getSelectedText()); } buildTranscriptionFrom(original: OutputTarget, keyEvent: KeyEvent, readonly: boolean, alternates?: Alternate[]): Transcription { diff --git a/web/src/app/webview/src/contextManager.ts b/web/src/app/webview/src/contextManager.ts index d9ef2c6e52..04fda6fe77 100644 --- a/web/src/app/webview/src/contextManager.ts +++ b/web/src/app/webview/src/contextManager.ts @@ -41,7 +41,9 @@ export class ContextHost extends Mock { // Signal the necessary text changes to the embedding app, if it exists. if(this.oninserttext) { - this.oninserttext(transform.deleteLeft, transform.insert, transform.deleteRight); + if(transform.deleteLeft > 0 || transform.insert != '' || transform.deleteRight > 0 || transform.erasedSelection) { + this.oninserttext(transform.deleteLeft, transform.insert, transform.deleteRight); + } } } From 8fbb0c3281f60012396366613b272d4cc4f91843 Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Thu, 21 Mar 2024 10:38:43 +0700 Subject: [PATCH 2/5] chore(web): update tests --- common/web/keyboard-processor/tests/node/transcriptions.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/common/web/keyboard-processor/tests/node/transcriptions.js b/common/web/keyboard-processor/tests/node/transcriptions.js index 6d6be7b540..7fc940d63e 100644 --- a/common/web/keyboard-processor/tests/node/transcriptions.js +++ b/common/web/keyboard-processor/tests/node/transcriptions.js @@ -448,7 +448,8 @@ but not himself.`; // Sheev Palpatine, in the Star Wars prequels. assert.deepEqual(transform, { insert: '', deleteLeft: 0, - deleteRight: 0 + deleteRight: 0, + erasedSelection: false }); }); @@ -459,7 +460,8 @@ but not himself.`; // Sheev Palpatine, in the Star Wars prequels. const transform = { insert: '', deleteLeft: 0, - deleteRight: 0 + deleteRight: 0, + erasedSelection: false }; target.apply(transform); From ea8366455cba1d91e7ce63c616ad513cf5a949b4 Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Thu, 21 Mar 2024 14:03:40 +0700 Subject: [PATCH 3/5] chore(web): update test fixture --- common/web/keyboard-processor/tests/node/transcriptions.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/common/web/keyboard-processor/tests/node/transcriptions.js b/common/web/keyboard-processor/tests/node/transcriptions.js index 7fc940d63e..b071532bf2 100644 --- a/common/web/keyboard-processor/tests/node/transcriptions.js +++ b/common/web/keyboard-processor/tests/node/transcriptions.js @@ -449,7 +449,7 @@ but not himself.`; // Sheev Palpatine, in the Star Wars prequels. insert: '', deleteLeft: 0, deleteRight: 0, - erasedSelection: false + erasedSelection: true }); }); @@ -461,7 +461,7 @@ but not himself.`; // Sheev Palpatine, in the Star Wars prequels. insert: '', deleteLeft: 0, deleteRight: 0, - erasedSelection: false + erasedSelection: true }; target.apply(transform); From 70836b40cfafe1aea673f956838b174c65f5cfb1 Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Wed, 27 Mar 2024 13:03:35 +1100 Subject: [PATCH 4/5] chore: Apply suggestions from code review Co-authored-by: Joshua Horton --- web/src/app/webview/src/contextManager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/src/app/webview/src/contextManager.ts b/web/src/app/webview/src/contextManager.ts index 04fda6fe77..a9b0a1c252 100644 --- a/web/src/app/webview/src/contextManager.ts +++ b/web/src/app/webview/src/contextManager.ts @@ -41,7 +41,7 @@ export class ContextHost extends Mock { // Signal the necessary text changes to the embedding app, if it exists. if(this.oninserttext) { - if(transform.deleteLeft > 0 || transform.insert != '' || transform.deleteRight > 0 || transform.erasedSelection) { + if(!isEmptyTransform(transform) || transform.erasedSelection) { this.oninserttext(transform.deleteLeft, transform.insert, transform.deleteRight); } } From edfed74c62159716e6a896b7ced051e954faefba Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Wed, 27 Mar 2024 11:48:26 +0700 Subject: [PATCH 5/5] fix(web): update import for isEmptyTransform --- web/src/app/webview/src/contextManager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/src/app/webview/src/contextManager.ts b/web/src/app/webview/src/contextManager.ts index a9b0a1c252..a2450ad58c 100644 --- a/web/src/app/webview/src/contextManager.ts +++ b/web/src/app/webview/src/contextManager.ts @@ -1,4 +1,4 @@ -import { type Keyboard, Mock, OutputTarget, Transcription, findCommonSubstringEndIndex } from '@keymanapp/keyboard-processor'; +import { type Keyboard, Mock, OutputTarget, Transcription, findCommonSubstringEndIndex, isEmptyTransform } from '@keymanapp/keyboard-processor'; import { KeyboardStub } from 'keyman/engine/package-cache'; import { ContextManagerBase, ContextManagerConfiguration } from 'keyman/engine/main'; import { WebviewConfiguration } from './configuration.js';