From 39f30e4385febc8a64aa6e0ff60e201875c1d8e1 Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Tue, 6 Jun 2023 10:15:16 +0700 Subject: [PATCH] fix(web): restores element text scrolling --- .../app/browser/src/context/focusAssistant.ts | 22 ++--- .../src/context/pageIntegrationHandlers.ts | 6 +- web/src/app/browser/src/contextManager.ts | 12 +-- web/src/engine/element-wrappers/src/input.ts | 79 ++++++++--------- .../element-wrappers/src/outputTarget.ts | 10 +++ .../engine/element-wrappers/src/textarea.ts | 86 +++++++------------ 6 files changed, 101 insertions(+), 114 deletions(-) diff --git a/web/src/app/browser/src/context/focusAssistant.ts b/web/src/app/browser/src/context/focusAssistant.ts index bda60dee5d..c0ae8a6bdb 100644 --- a/web/src/app/browser/src/context/focusAssistant.ts +++ b/web/src/app/browser/src/context/focusAssistant.ts @@ -41,6 +41,18 @@ interface EventMap { export class FocusAssistant extends EventEmitter { private _maintainingFocus: boolean = false; // ActivatingKeymanWebUI - Does the OSK have active focus / an active interaction? + /** + * Returns `true` only when the active target has an active `forceScroll` method/state, which deliberately + * blurs and then refocuses the same element in order to force a browser-default page scroll to keep the + * element and text-caret visible. + */ + readonly isTargetForcingScroll: () => boolean; + + constructor(isTargetForcingScroll: () => boolean) { + super(); + this.isTargetForcingScroll = isTargetForcingScroll; + } + /* * Long-term idea here: about all of the relevant OSK events that would interact with this have "enter" and * "leave" variants - we could take a stack of `Promise`s. On a `Promise` fulfillment, remove it from the @@ -109,16 +121,6 @@ export class FocusAssistant extends EventEmitter { */ _IgnoreNextSelChange = 0; - /** - * JH (2023-04-24): Set only by the OutputTarget `forceScroll` method, which deliberately blurs and - * then refocuses the same element in order to force a browser-default page scroll to keep the element - * visible. - * - * While it feels like this should be possible to merge with the other class fields in some form... it - * doesn't seem as safe to do on first glance. - */ - _IgnoreBlurFocus: boolean = false; - /** * Is used as a time-delayed async `restoringFocus` or `maintainingFocus` - could be modeled decently as a Promise. * Probably more the latter, as it's a touch-OSK interaction like the other `maintainingFocus` cases. diff --git a/web/src/app/browser/src/context/pageIntegrationHandlers.ts b/web/src/app/browser/src/context/pageIntegrationHandlers.ts index a2cd64f156..30b91cb102 100644 --- a/web/src/app/browser/src/context/pageIntegrationHandlers.ts +++ b/web/src/app/browser/src/context/pageIntegrationHandlers.ts @@ -67,8 +67,10 @@ export class PageIntegrationHandlers { } private suppressFocusCheck: (e: FocusEvent) => boolean = (e) => { - if(this.focusAssistant._IgnoreBlurFocus) { - // Prevent triggering other blur-handling events (as possible) + if(this.focusAssistant.isTargetForcingScroll()) { + // Prevent triggering other blur-handling events (as possible) - this blur + // is programmatic in order to force a browser scroll-position update. + // All focus changes should be prevented at this time. e.stopPropagation(); e.cancelBubble = true; } diff --git a/web/src/app/browser/src/contextManager.ts b/web/src/app/browser/src/contextManager.ts index e0f3cd76a2..42e7edd729 100644 --- a/web/src/app/browser/src/contextManager.ts +++ b/web/src/app/browser/src/contextManager.ts @@ -43,7 +43,7 @@ function _SetTargDir(Ptarg: HTMLElement, activeKeyboard: Keyboard) { export default class ContextManager extends ContextManagerBase { private _activeKeyboard: {keyboard: Keyboard, metadata: KeyboardStub}; private cookieManager = new CookieSerializer('KeymanWeb_Keyboard'); - readonly focusAssistant = new FocusAssistant(); + readonly focusAssistant = new FocusAssistant(() => this.activeTarget?.isForcingScroll()); readonly page: PageContextAttachment; private mostRecentTarget: OutputTarget; private currentTarget: OutputTarget; @@ -231,6 +231,11 @@ export default class ContextManager extends ContextManagerBase void, - /** * This event will be raised when a newline is received by wrapped elements not of * the 'search' or 'submit' types. @@ -74,23 +41,19 @@ export default class Input extends OutputTarget { */ private processedSelectionEnd: number; + /** + * Set, then unset within the `forceScroll` method in order to facilitate the + * `isForcingScroll` flag. + */ + private _activeForcedScroll: boolean; + constructor(ele: HTMLInputElement) { super(); this.root = ele; this._cachedSelectionStart = -1; - - // Intended to facilitate reimplmentation of the old `forceScroll` as an event handler - // defined externally, but automatically set on class construction. - Input.constructorExtensions(this); } - /** - * This may be set to define additional construction behaviors to perform, such as - * automatically setting handlers for defined events. - */ - public static constructorExtensions: (constructingInstance: Input) => void = () => {}; - get isSynthetic(): boolean { return false; } @@ -146,11 +109,39 @@ export default class Input extends OutputTarget { this.processedSelectionStart = start; this.processedSelectionEnd = end; - this.events.emit('scrollfocusrequest', this.root); + this.forceScroll(); this.root.setSelectionRange(domStart, domEnd, direction); } + forceScroll() { + // Only executes when com.keyman.DOMEventHandlers is defined. + // + // We bypass this whenever operating in the embedded format. + const element = this.getElement(); + + let selectionStart = element.selectionStart; + let selectionEnd = element.selectionEnd; + + this._activeForcedScroll = true; + + try { + //Forces scrolling; the re-focus triggers the scroll, at least. + element.blur(); + element.focus(); + } finally { + // On Edge, it appears that the blur/focus combination will reset the caret position + // under certain scenarios during unit tests. So, we re-set it afterward. + element.selectionStart = selectionStart; + element.selectionEnd = selectionEnd; + this._activeForcedScroll = false; + } + } + + isForcingScroll(): boolean { + return this._activeForcedScroll; + } + getSelectionDirection(): "forward" | "backward" | "none" { return this.root.selectionDirection; } diff --git a/web/src/engine/element-wrappers/src/outputTarget.ts b/web/src/engine/element-wrappers/src/outputTarget.ts index e7640b36d5..fcc35da1b7 100644 --- a/web/src/engine/element-wrappers/src/outputTarget.ts +++ b/web/src/engine/element-wrappers/src/outputTarget.ts @@ -23,6 +23,16 @@ export default abstract class OutputTarget void, -} - -export default class TextArea extends OutputTarget { +export default class TextArea extends OutputTarget<{}> { root: HTMLTextAreaElement; /** @@ -56,31 +21,18 @@ export default class TextArea extends OutputTarget { private processedSelectionEnd: number; /** - * Used to temporarily store the y-axis scroll coordinate. + * Set, then unset within the `forceScroll` method in order to facilitate the + * `isForcingScroll` flag. */ - private scrollTop?: number; - - /** - * Used to temporarily store the x-axis scroll coordinate. - */ - private scrollLeft?: number; + private _activeForcedScroll: boolean; constructor(ele: HTMLTextAreaElement) { super(); this.root = ele; this._cachedSelectionStart = -1; - // Intended to facilitate reimplmentation of the old `forceScroll` as an event handler - // defined externally, but automatically set on class construction. - TextArea.constructorExtensions(this); } - /** - * This may be set to define additional construction behaviors to perform, such as - * automatically setting handlers for defined events. - */ - public static constructorExtensions: (constructingInstance: TextArea) => void = () => {}; - get isSynthetic(): boolean { return false; } @@ -136,11 +88,39 @@ export default class TextArea extends OutputTarget { this.processedSelectionStart = start; this.processedSelectionEnd = end; - this.events.emit('scrollfocusrequest', this.root); + this.forceScroll(); this.root.setSelectionRange(domStart, domEnd, direction); } + forceScroll() { + // Only executes when com.keyman.DOMEventHandlers is defined. + // + // We bypass this whenever operating in the embedded format. + const element = this.getElement(); + + let selectionStart = element.selectionStart; + let selectionEnd = element.selectionEnd; + + this._activeForcedScroll = true; + + try { + //Forces scrolling; the re-focus triggers the scroll, at least. + element.blur(); + element.focus(); + } finally { + // On Edge, it appears that the blur/focus combination will reset the caret position + // under certain scenarios during unit tests. So, we re-set it afterward. + element.selectionStart = selectionStart; + element.selectionEnd = selectionEnd; + this._activeForcedScroll = false; + } + } + + isForcingScroll(): boolean { + return this._activeForcedScroll; + } + getSelectionDirection(): "forward" | "backward" | "none" { return this.root.selectionDirection; }