From 79ad53f9cf8960449a90512e0b986536c069194c Mon Sep 17 00:00:00 2001 From: Eberhard Beilharz Date: Mon, 8 Dec 2025 19:40:24 +0100 Subject: [PATCH 1/4] refactor(web): document and cleanup parameters of `Keyboard.notify()` Document what values are possible for the `eventCode` parameter and what these values mean. Also change `data` parameter to a `boolean`. Test-bot: skip --- web/src/app/browser/src/contextManager.ts | 4 ++-- .../engine/src/js-processor/jsKeyboardProcessor.ts | 4 ++-- web/src/engine/src/keyboard/keyboards/jsKeyboard.ts | 6 +++--- web/src/engine/src/keyboard/keyboards/kmxKeyboard.ts | 11 +++++++---- 4 files changed, 14 insertions(+), 11 deletions(-) diff --git a/web/src/app/browser/src/contextManager.ts b/web/src/app/browser/src/contextManager.ts index 30729b34df..8004e743d8 100644 --- a/web/src/app/browser/src/contextManager.ts +++ b/web/src/app/browser/src/contextManager.ts @@ -613,7 +613,7 @@ export class ContextManager extends ContextManagerBase { const activeKeyboard = this.activeKeyboard?.keyboard; if(!focusAssistant.restoringFocus) { textStore?.deadkeys().clear(); - activeKeyboard?.notify(0, textStore, 1); // I2187 + activeKeyboard?.notify(0, textStore, true); // I2187 } if(!focusAssistant.restoringFocus && this.mostRecentTextStore != textStore) { @@ -711,7 +711,7 @@ export class ContextManager extends ContextManagerBase { const {activeKeyboard} = this; const {maintainingFocus} = this.focusAssistant; if(!maintainingFocus && activeKeyboard) { - activeKeyboard.keyboard.notify(0, textStore, 0); // I2187 + activeKeyboard.keyboard.notify(0, textStore, false); // I2187 } if(previousTextStore && !this.activeTextStore) { this.emit('textstorechange', null); diff --git a/web/src/engine/src/js-processor/jsKeyboardProcessor.ts b/web/src/engine/src/js-processor/jsKeyboardProcessor.ts index a42ecc7067..5b2bd705ac 100644 --- a/web/src/engine/src/js-processor/jsKeyboardProcessor.ts +++ b/web/src/engine/src/js-processor/jsKeyboardProcessor.ts @@ -539,7 +539,7 @@ export class JSKeyboardProcessor extends EventEmitter implements Keybo } if(Levent.isModifier) { - this.activeKeyboard.notify(Levent.Lcode, textStore, isKeyDown ? 1 : 0); + this.activeKeyboard.notify(Levent.Lcode, textStore, isKeyDown); // For eventual integration - we bypass an OSK update for physical keystrokes when in touch mode. if(!Levent.device.touchable) { return this._UpdateVKShift(Levent); // I2187 @@ -549,7 +549,7 @@ export class JSKeyboardProcessor extends EventEmitter implements Keybo } if(Levent.LmodifierChange) { - this.activeKeyboard.notify(0, textStore, 1); + this.activeKeyboard.notify(0, textStore, true); if(!Levent.device.touchable) { this._UpdateVKShift(Levent); } diff --git a/web/src/engine/src/keyboard/keyboards/jsKeyboard.ts b/web/src/engine/src/keyboard/keyboards/jsKeyboard.ts index 90b45f3ba5..4c6631a1a8 100644 --- a/web/src/engine/src/keyboard/keyboards/jsKeyboard.ts +++ b/web/src/engine/src/keyboard/keyboards/jsKeyboard.ts @@ -358,12 +358,12 @@ export class JSKeyboard { * * @param {number} command event code (16,17,18) or 0 * @param {TextStore} textStore textStore - * @param {number} data 1 or 0 + * @param {boolean} data 1 or 0 */ - public notify(command: number, textStore: TextStore, data: number): void { // I2187 + public notify(command: number, textStore: TextStore, data: boolean): void { // I2187 // Good example use case - the Japanese CJK-picker keyboard if(typeof(this.scriptObject['KNS']) == 'function') { - this.scriptObject['KNS'](command, textStore, data); + this.scriptObject['KNS'](command, textStore, data ? 1 : 0); } } diff --git a/web/src/engine/src/keyboard/keyboards/kmxKeyboard.ts b/web/src/engine/src/keyboard/keyboards/kmxKeyboard.ts index 73b62558dc..ba78dc4e01 100644 --- a/web/src/engine/src/keyboard/keyboards/kmxKeyboard.ts +++ b/web/src/engine/src/keyboard/keyboards/kmxKeyboard.ts @@ -99,12 +99,15 @@ export class KMXKeyboard { } /** - * @param {number} eventCode event code (16,17,18) or 0 // TODO-web-core: document meaning of these! (#15290) - * @param {TextStore} textStore textStore - * @param {number} data 1 or 0 * Notifies keyboard of keystroke or other event + * + * @param {number} eventCode key code (16-18: Shift, Control or Alt), + * or 0 for focus + * @param {TextStore} textStore textStore + * @param {number} boolean true for KeyDown or FocusReceived, + * false for KeyUp or FocusLost */ - public notify(eventCode: 16|17|18|0, textStore: TextStore, data: number) { // I2187 + public notify(eventCode: number, textStore: TextStore, data: boolean): void { // I2187 // TODO-web-core: do we need to support this? (#15290) } From e4e76caea76006d853d18711c97805eaa5b0395b Mon Sep 17 00:00:00 2001 From: Eberhard Beilharz Date: Wed, 17 Dec 2025 15:55:52 +0100 Subject: [PATCH 2/4] refactor(web): address code review comment Build-bot: skip Test-bot: skip --- web/src/engine/src/keyboard/keyboards/jsKeyboard.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/src/engine/src/keyboard/keyboards/jsKeyboard.ts b/web/src/engine/src/keyboard/keyboards/jsKeyboard.ts index 4c6631a1a8..50cc7846b9 100644 --- a/web/src/engine/src/keyboard/keyboards/jsKeyboard.ts +++ b/web/src/engine/src/keyboard/keyboards/jsKeyboard.ts @@ -358,7 +358,7 @@ export class JSKeyboard { * * @param {number} command event code (16,17,18) or 0 * @param {TextStore} textStore textStore - * @param {boolean} data 1 or 0 + * @param {boolean} data true o false */ public notify(command: number, textStore: TextStore, data: boolean): void { // I2187 // Good example use case - the Japanese CJK-picker keyboard From 1895a944a2b27747ce14e8879d3b1885c215b072 Mon Sep 17 00:00:00 2001 From: Eberhard Beilharz Date: Thu, 18 Dec 2025 12:48:45 +0100 Subject: [PATCH 3/4] Apply suggestions from code review Co-authored-by: Marc Durdin --- web/src/engine/src/keyboard/keyboards/kmxKeyboard.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/src/engine/src/keyboard/keyboards/kmxKeyboard.ts b/web/src/engine/src/keyboard/keyboards/kmxKeyboard.ts index ba78dc4e01..ff69acfa51 100644 --- a/web/src/engine/src/keyboard/keyboards/kmxKeyboard.ts +++ b/web/src/engine/src/keyboard/keyboards/kmxKeyboard.ts @@ -104,7 +104,7 @@ export class KMXKeyboard { * @param {number} eventCode key code (16-18: Shift, Control or Alt), * or 0 for focus * @param {TextStore} textStore textStore - * @param {number} boolean true for KeyDown or FocusReceived, + * @param {boolean} data true for KeyDown or FocusReceived, * false for KeyUp or FocusLost */ public notify(eventCode: number, textStore: TextStore, data: boolean): void { // I2187 From 72d12622fff76308ec9124396440b9dd1547dee0 Mon Sep 17 00:00:00 2001 From: Eberhard Beilharz Date: Fri, 19 Dec 2025 11:59:32 +0100 Subject: [PATCH 4/4] refactor(web): address code review comment --- web/src/app/browser/src/contextManager.ts | 4 ++-- .../engine/src/js-processor/jsKeyboardProcessor.ts | 4 ++-- web/src/engine/src/keyboard/index.ts | 2 +- web/src/engine/src/keyboard/keyboards/jsKeyboard.ts | 12 +++++++----- .../src/keyboard/keyboards/keyboardLoaderBase.ts | 8 ++++++++ .../engine/src/keyboard/keyboards/kmxKeyboard.ts | 13 +++++++------ 6 files changed, 27 insertions(+), 16 deletions(-) diff --git a/web/src/app/browser/src/contextManager.ts b/web/src/app/browser/src/contextManager.ts index 8004e743d8..30729b34df 100644 --- a/web/src/app/browser/src/contextManager.ts +++ b/web/src/app/browser/src/contextManager.ts @@ -613,7 +613,7 @@ export class ContextManager extends ContextManagerBase { const activeKeyboard = this.activeKeyboard?.keyboard; if(!focusAssistant.restoringFocus) { textStore?.deadkeys().clear(); - activeKeyboard?.notify(0, textStore, true); // I2187 + activeKeyboard?.notify(0, textStore, 1); // I2187 } if(!focusAssistant.restoringFocus && this.mostRecentTextStore != textStore) { @@ -711,7 +711,7 @@ export class ContextManager extends ContextManagerBase { const {activeKeyboard} = this; const {maintainingFocus} = this.focusAssistant; if(!maintainingFocus && activeKeyboard) { - activeKeyboard.keyboard.notify(0, textStore, false); // I2187 + activeKeyboard.keyboard.notify(0, textStore, 0); // I2187 } if(previousTextStore && !this.activeTextStore) { this.emit('textstorechange', null); diff --git a/web/src/engine/src/js-processor/jsKeyboardProcessor.ts b/web/src/engine/src/js-processor/jsKeyboardProcessor.ts index 5b2bd705ac..a42ecc7067 100644 --- a/web/src/engine/src/js-processor/jsKeyboardProcessor.ts +++ b/web/src/engine/src/js-processor/jsKeyboardProcessor.ts @@ -539,7 +539,7 @@ export class JSKeyboardProcessor extends EventEmitter implements Keybo } if(Levent.isModifier) { - this.activeKeyboard.notify(Levent.Lcode, textStore, isKeyDown); + this.activeKeyboard.notify(Levent.Lcode, textStore, isKeyDown ? 1 : 0); // For eventual integration - we bypass an OSK update for physical keystrokes when in touch mode. if(!Levent.device.touchable) { return this._UpdateVKShift(Levent); // I2187 @@ -549,7 +549,7 @@ export class JSKeyboardProcessor extends EventEmitter implements Keybo } if(Levent.LmodifierChange) { - this.activeKeyboard.notify(0, textStore, true); + this.activeKeyboard.notify(0, textStore, 1); if(!Levent.device.touchable) { this._UpdateVKShift(Levent); } diff --git a/web/src/engine/src/keyboard/index.ts b/web/src/engine/src/keyboard/index.ts index c8ea67593f..0e8fc53d3e 100644 --- a/web/src/engine/src/keyboard/index.ts +++ b/web/src/engine/src/keyboard/index.ts @@ -4,7 +4,7 @@ export { JSKeyboard, LayoutState } from "./keyboards/jsKeyboard.js"; export { KeyboardMinimalInterface } from './keyboards/keyboardMinimalInterface.js'; export { KMXKeyboard } from './keyboards/kmxKeyboard.js'; export { KeyboardHarness, KeyboardKeymanGlobal, MinimalCodesInterface, MinimalKeymanGlobal } from "./keyboards/keyboardHarness.js"; -export { Keyboard, KeyboardLoaderBase } from "./keyboards/keyboardLoaderBase.js"; +export { NotifyEventCode, Keyboard, KeyboardLoaderBase } from "./keyboards/keyboardLoaderBase.js"; export { KeyboardLoadErrorBuilder, KeyboardMissingError, KeyboardScriptError, KeyboardDownloadError, InvalidKeyboardError } from './keyboards/keyboardLoadError.js' export { BeepHandler, EventMap, KeyboardProcessor } from "./keyboards/keyboardProcessor.js"; export { diff --git a/web/src/engine/src/keyboard/keyboards/jsKeyboard.ts b/web/src/engine/src/keyboard/keyboards/jsKeyboard.ts index 50cc7846b9..5fd6e06530 100644 --- a/web/src/engine/src/keyboard/keyboards/jsKeyboard.ts +++ b/web/src/engine/src/keyboard/keyboards/jsKeyboard.ts @@ -14,6 +14,7 @@ type TouchLayoutSpec = TouchLayout.TouchLayoutPlatform & { isDefault?: boolean}; import { Version, DeviceSpec } from "keyman/common/web-utils"; import { StateKeyMap } from "./stateKeyMap.js"; +import { NotifyEventCode } from './keyboardLoaderBase.js'; /** * Stores preprocessed properties of a keyboard for quick retrieval later. @@ -356,14 +357,15 @@ export class JSKeyboard { /** * Notifies keyboard of keystroke or other event * - * @param {number} command event code (16,17,18) or 0 - * @param {TextStore} textStore textStore - * @param {boolean} data true o false + * @param {NotifyEventCode} command event code (16,17,18) or 0 + * @param {TextStore} textStore textStore + * @param {number} data 1 for KeyDown or FocusReceived, + * 0 for KeyUp or FocusLost */ - public notify(command: number, textStore: TextStore, data: boolean): void { // I2187 + public notify(command: NotifyEventCode, textStore: TextStore, data: number): void { // I2187 // Good example use case - the Japanese CJK-picker keyboard if(typeof(this.scriptObject['KNS']) == 'function') { - this.scriptObject['KNS'](command, textStore, data ? 1 : 0); + this.scriptObject['KNS'](command, textStore, data); } } diff --git a/web/src/engine/src/keyboard/keyboards/keyboardLoaderBase.ts b/web/src/engine/src/keyboard/keyboards/keyboardLoaderBase.ts index b7fe16dd1d..2d938e159e 100644 --- a/web/src/engine/src/keyboard/keyboards/keyboardLoaderBase.ts +++ b/web/src/engine/src/keyboard/keyboards/keyboardLoaderBase.ts @@ -4,6 +4,14 @@ import { KMXKeyboard } from './kmxKeyboard.js'; import { KeyboardHarness } from "./keyboardHarness.js"; import KeyboardProperties from "./keyboardProperties.js"; import { KeyboardLoadErrorBuilder, StubBasedErrorBuilder, UriBasedErrorBuilder } from './keyboardLoadError.js'; +import { Codes } from '../codes.js'; + +export enum NotifyEventCode { + FocusEvent = 0, + ShiftKey = Codes.keyCodes.K_SHIFT, + ControlKey = Codes.keyCodes.K_CONTROL, + AltKey = Codes.keyCodes.K_ALT, +}; export type KeyboardStub = KeyboardProperties & { filename: string }; export type Keyboard = JSKeyboard | KMXKeyboard; diff --git a/web/src/engine/src/keyboard/keyboards/kmxKeyboard.ts b/web/src/engine/src/keyboard/keyboards/kmxKeyboard.ts index ff69acfa51..1c8123192f 100644 --- a/web/src/engine/src/keyboard/keyboards/kmxKeyboard.ts +++ b/web/src/engine/src/keyboard/keyboards/kmxKeyboard.ts @@ -7,6 +7,7 @@ import { ActiveKey, ActiveSubKey } from './activeLayout.js'; import { StateKeyMap } from './stateKeyMap.js'; import { KeyEvent } from '../keyEvent.js'; import { TextStore } from '../textStore.js'; +import { NotifyEventCode } from './keyboardLoaderBase.js'; /** * Acts as a wrapper class for KMX(+) Keyman keyboards @@ -101,13 +102,13 @@ export class KMXKeyboard { /** * Notifies keyboard of keystroke or other event * - * @param {number} eventCode key code (16-18: Shift, Control or Alt), - * or 0 for focus - * @param {TextStore} textStore textStore - * @param {boolean} data true for KeyDown or FocusReceived, - * false for KeyUp or FocusLost + * @param {NotifyEventCode} eventCode key code (16-18: Shift, Control or Alt), + * or 0 for focus + * @param {TextStore} textStore textStore + * @param {number} data 1 for KeyDown or FocusReceived, + * 0 for KeyUp or FocusLost */ - public notify(eventCode: number, textStore: TextStore, data: boolean): void { // I2187 + public notify(eventCode: NotifyEventCode, textStore: TextStore, data: number): void { // I2187 // TODO-web-core: do we need to support this? (#15290) }