From 257262972382f87da305ca07a55eaeafdf8401a6 Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Wed, 21 Dec 2022 10:12:28 +0700 Subject: [PATCH 01/60] feat(web): suggestion banner itself scrolls --- web/src/engine/osk/src/banner/banner.ts | 12 +++- web/src/engine/osk/src/banner/bannerView.ts | 47 ++++-------- .../engine/osk/src/banner/suggestionBanner.ts | 71 +++++++++++++------ .../event-interpreter/uiTouchHandlerBase.ts | 17 +++-- web/src/engine/osk/src/views/oskView.ts | 1 + web/src/resources/osk/kmwosk.css | 29 ++++++++ 6 files changed, 116 insertions(+), 61 deletions(-) diff --git a/web/src/engine/osk/src/banner/banner.ts b/web/src/engine/osk/src/banner/banner.ts index 3394efc438..b63262211b 100644 --- a/web/src/engine/osk/src/banner/banner.ts +++ b/web/src/engine/osk/src/banner/banner.ts @@ -5,6 +5,7 @@ import { createUnselectableElement } from 'keyman/engine/dom-utils'; export abstract class Banner { private _height: number; // pixels + private _width: number; // pixels private div: HTMLDivElement; public static DEFAULT_HEIGHT: number = 37; // pixels; embedded apps can modify @@ -35,6 +36,15 @@ export abstract class Banner { this.update(); } + public get width(): number { + return this._width; + } + + public set width(width: number) { + this._width = width; + this.update(); + } + /** * Function update * @return {boolean} true if the banner styling changed @@ -76,7 +86,7 @@ export abstract class Banner { * Function getDiv * Scope Public * @returns {HTMLElement} Base element of the banner - * Description Returns the HTMLElelemnt of the banner + * Description Returns the HTMLElement of the banner */ public getDiv(): HTMLElement { return this.div; diff --git a/web/src/engine/osk/src/banner/bannerView.ts b/web/src/engine/osk/src/banner/bannerView.ts index b291f7454e..de7d25a7e8 100644 --- a/web/src/engine/osk/src/banner/bannerView.ts +++ b/web/src/engine/osk/src/banner/bannerView.ts @@ -24,38 +24,8 @@ interface BannerViewEventMap { } /** - * The `BannerManager` module is designed to serve as a manager for the - * different `Banner` types. - * To facilitate this, it will provide a root element property that serves - * as a container for any active `Banner`, helping KMW to avoid needless - * DOM element shuffling. - * - * Goals for the `BannerManager`: - * - * * It will be exposed as `keyman.osk.banner` and will provide the following API: - * * `getOptions`, `setOptions` - refer to the `BannerOptions` class for details. - * * This provides a persistent point that the web page designers and our - * model apps can utilize and can communicate with. - * * These API functions are designed for live use and will allow - * _hot-swapping_ the `Banner` instance; they're not initialization-only. - * * Disabling the `Banner` (even for suggestions) outright with - * `enablePredictions == false` will auto-unload any loaded predictive model - * from `ModelManager` and setting it to `true` will revert this. - * * This should help to avoid wasting computational resources. - * * It will listen to ModelManager events and automatically swap Banner - * instances as appropriate: - * * The option `persistentBanner == true` is designed to replicate current - * iOS system keyboard behavior. - * * When true, an `ImageBanner` will be displayed. - * * If false, it will be replaced with a `BlankBanner` of zero height, - * corresponding to our current default lack of banner. - * * It will not automatically set `persistentBanner == true`; - * this must be set by the iOS app, and only under the following conditions: - * * `keyman.isEmbedded == true` - * * `device.OS == 'ios'` - * * Keyman is being used as the system keyboard within an app that - * needs to reserve this space (i.e: Keyman for iOS), - * rather than as its standalone app. + * The `BannerView` module is designed to serve as the hot-swap container for the + * different `Banner` types, helping KMW to avoid needless DOM element shuffling. */ export class BannerView implements OSKViewComponent { private bannerContainer: HTMLDivElement; @@ -161,5 +131,16 @@ export class BannerView implements OSKViewComponent { return ParsedLengthStyle.inPixels(this.height); } - public refreshLayout() {}; + public get width(): number | undefined { + return this.currentBanner?.width; + } + + public set width(w: number) { + if(this.currentBanner) { + this.currentBanner.width = w; + } + } + + public refreshLayout() { + } } \ No newline at end of file diff --git a/web/src/engine/osk/src/banner/suggestionBanner.ts b/web/src/engine/osk/src/banner/suggestionBanner.ts index c5ff725556..9f97617489 100644 --- a/web/src/engine/osk/src/banner/suggestionBanner.ts +++ b/web/src/engine/osk/src/banner/suggestionBanner.ts @@ -12,6 +12,7 @@ import EventEmitter from 'eventemitter3'; export class BannerSuggestion { div: HTMLDivElement; + container: HTMLDivElement; private display: HTMLSpanElement; private fontFamily?: string; private rtl: boolean = false; @@ -31,7 +32,8 @@ export class BannerSuggestion { // Provides an empty, base SPAN for text display. We'll swap these out regularly; // `Suggestion`s will have varying length and may need different styling. let display = this.display = createUnselectableElement('span'); - this.div.appendChild(display); + display.className = 'kmw-suggestion-text'; + this.container.appendChild(display); } private constructRoot() { @@ -40,13 +42,19 @@ export class BannerSuggestion { div.className = "kmw-suggest-option"; div.id = BannerSuggestion.BASE_ID + this.index; - // Ensures that a reasonable width % is set. - let usableWidth = 100 - SuggestionBanner.MARGIN * (SuggestionBanner.SUGGESTION_LIMIT - 1); - let widthpc = usableWidth / SuggestionBanner.SUGGESTION_LIMIT; - - ds.width = widthpc + '%'; - this.div['suggestion'] = this; + + let container = this.container = document.createElement('div'); + container.className = "kmw-suggestion-container"; + + // Ensures that a reasonable default width, based on % is set. (Since it's not yet in the DOM, we may not yet have actual width info.) + let usableWidth = 100 - SuggestionBanner.MARGIN * (SuggestionBanner.SUGGESTION_LIMIT - 1); + + // The `/ 2` part: Ensures that the full banner is double-wide, which is useful for demoing scrolling. + let widthpc = usableWidth / (SuggestionBanner.SUGGESTION_LIMIT / 2); + container.style.minWidth = widthpc + '%'; + + div.appendChild(container); } public matchKeyboardProperties(keyboardProperties: KeyboardProperties) { @@ -82,7 +90,7 @@ export class BannerSuggestion { private updateText() { let display = this.generateSuggestionText(this.rtl); - this.div.replaceChild(display, this.display); + this.container.replaceChild(display, this.display); this.display = display; } @@ -130,7 +138,7 @@ export class BannerSuggestion { * Description Display lexical model suggestions in the banner */ export class SuggestionBanner extends Banner { - public static readonly SUGGESTION_LIMIT: number = 3; + public static readonly SUGGESTION_LIMIT: number = 6; public static readonly MARGIN = 1; public readonly events: EventEmitter; @@ -141,6 +149,7 @@ export class SuggestionBanner extends Banner { private hostDevice: DeviceSpec; private manager: SuggestionInputManager; + private readonly container: HTMLElement; readonly type = 'suggestion'; @@ -148,6 +157,7 @@ export class SuggestionBanner extends Banner { static readonly TOUCHED_CLASS: string = 'kmw-suggest-touched'; static readonly BANNER_CLASS: string = 'kmw-suggest-banner'; + static readonly BANNER_SCROLLER_CLASS = 'kmw-suggest-banner-scroller'; constructor(hostDevice: DeviceSpec, height?: number) { super(height || Banner.DEFAULT_HEIGHT); @@ -155,9 +165,14 @@ export class SuggestionBanner extends Banner { this.getDiv().className = this.getDiv().className + ' ' + SuggestionBanner.BANNER_CLASS; + this.container = document.createElement('div'); + this.container.className = SuggestionBanner.BANNER_SCROLLER_CLASS; + this.getDiv().appendChild(this.container); + // TODO: additional styling for the banner scroll container? + this.buildInternals(false); - this.manager = new SuggestionInputManager(this.getDiv()); + this.manager = new SuggestionInputManager(this.container, this.container); this.events = this.manager.events; this.setupInputHandling(); @@ -181,18 +196,18 @@ export class SuggestionBanner extends Banner { */ for (var i=0; i { findTargetFrom(e: HTMLElement): HTMLDivElement { try { if(e) { - if(e.classList.contains('kmw-suggest-option')) { - return e as HTMLDivElement; + const parent = e.parentElement; + if(!parent) { + return null; } - if(e.parentElement && e.parentElement.classList.contains('kmw-suggest-option')) { - return e.parentElement as HTMLDivElement; + + if(parent.classList.contains('kmw-suggest-option')) { + return parent as HTMLDivElement; } + + const grandparent = parent.parentElement; + if(!grandparent) { + return null; + } + + if(grandparent.classList.contains('kmw-suggest-option')) { + return grandparent as HTMLDivElement; + } + // if(e.firstChild && util.hasClass( e.firstChild,'kmw-suggest-option')) { // return e.firstChild as HTMLDivElement; // } @@ -418,8 +445,8 @@ class SuggestionInputManager extends UITouchHandlerBase { }) } - constructor(div: HTMLElement) { + constructor(div: HTMLElement, scroller: HTMLElement) { // TODO: Determine appropriate CSS styling names, etc. - super(div, Banner.BANNER_CLASS, SuggestionBanner.TOUCHED_CLASS); + super(div, scroller, Banner.BANNER_CLASS, SuggestionBanner.TOUCHED_CLASS); } } diff --git a/web/src/engine/osk/src/input/event-interpreter/uiTouchHandlerBase.ts b/web/src/engine/osk/src/input/event-interpreter/uiTouchHandlerBase.ts index 1fea7fab11..0b4956280d 100644 --- a/web/src/engine/osk/src/input/event-interpreter/uiTouchHandlerBase.ts +++ b/web/src/engine/osk/src/input/event-interpreter/uiTouchHandlerBase.ts @@ -43,7 +43,9 @@ class ScrollState { export default abstract class UITouchHandlerBase { private rowClassMatch: string; private selectedTargetMatch: string; + private baseElement: HTMLElement; + private scroller: HTMLElement; private touchX: number; private touchY: number; @@ -54,10 +56,12 @@ export default abstract class UITouchHandlerBase { private scrollTouchState: ScrollState; private pendingTarget: Target; - constructor(baseElement: HTMLElement, rowClassMatch: string, selectedTargetMatch: string) { + constructor(baseElement: HTMLElement, scroller: HTMLElement, rowClassMatch: string, selectedTargetMatch: string) { this.baseElement = baseElement; this.rowClassMatch = rowClassMatch; this.selectedTargetMatch = selectedTargetMatch; + + this.scroller = scroller || null; } /** @@ -241,8 +245,7 @@ export default abstract class UITouchHandlerBase { } // Establish scroll tracking. - let shouldScroll = (this.currentTarget.clientWidth < this.currentTarget.scrollWidth); - this.scrollTouchState = shouldScroll ? new ScrollState(coord) : null; + this.scrollTouchState = new ScrollState(coord); // Alright, Target acquired! Now to use it: @@ -334,9 +337,13 @@ export default abstract class UITouchHandlerBase { return; } - if(this.currentTarget && this.scrollTouchState != null) { + if(this.scrollTouchState != null) { + // TODO: Work on smoothing this out; looks like subpixel scroll info gets rounded out, + // and this results in a mild desync. let deltaX = this.scrollTouchState.updateTo(coord).deltaX; - this.currentTarget.scrollLeft -= window.devicePixelRatio * deltaX; + if(this.scroller) { + this.scroller.scrollLeft -= deltaX; + } return; } diff --git a/web/src/engine/osk/src/views/oskView.ts b/web/src/engine/osk/src/views/oskView.ts index 0399991fe0..068bb35bd2 100644 --- a/web/src/engine/osk/src/views/oskView.ts +++ b/web/src/engine/osk/src/views/oskView.ts @@ -625,6 +625,7 @@ export default abstract class OSKView extends EventEmitter implements if(!pending) { this.headerView?.refreshLayout(); this.bannerView.refreshLayout(); + this.bannerView.width = this.computedWidth; this.footerView?.refreshLayout(); } diff --git a/web/src/resources/osk/kmwosk.css b/web/src/resources/osk/kmwosk.css index 029a0f7466..fe8990f8fe 100644 --- a/web/src/resources/osk/kmwosk.css +++ b/web/src/resources/osk/kmwosk.css @@ -66,6 +66,35 @@ font-size: 0.75em; } +.kmw-suggestion-text { + padding-left: 4px; + padding-right: 4px; +} + +.kmw-suggest-banner-scroller { + overflow-x: hidden; + width: 100%; + height: 100%; + scrollbar-width: none; /* Firefox scrollbar prevention */ +} + +.kmw-suggest-banner-scroller::-webkit-scrollbar { + display: none; /* Safari + Chrome scrollbar prevention */ +} + +.kmw-suggest-option { + overflow: hidden; +} + +.kmw-suggestion-container { + height: 100%; + transition: all 0.25s; +} + +.kmw-suggest-option.kmw-suggest-touched .kmw-suggestion-container { + margin-left: 0px !important /* Overrides 'collapse' styling, which is accomplished via negative margin-left */ +} + .phone.windows .kmw-key-row{max-width:80%;} .phone .kmw-5rows {padding-top: 0;} From b99ca9e7724401e449b0795f6eaf75c7af6517d5 Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Wed, 21 Dec 2022 10:20:56 +0700 Subject: [PATCH 02/60] feat(web): spins getTextMetrics off as importable method, implements suggestion expansion structures --- .../engine/osk/src/banner/suggestionBanner.ts | 58 +++++-- .../osk/src/keyboard-layout/getTextMetrics.ts | 50 ++++++ .../engine/osk/src/keyboard-layout/oskKey.ts | 52 +----- web/src/resources/osk/kmwosk.css | 148 +++++++++++++++--- 4 files changed, 231 insertions(+), 77 deletions(-) create mode 100644 web/src/engine/osk/src/keyboard-layout/getTextMetrics.ts diff --git a/web/src/engine/osk/src/banner/suggestionBanner.ts b/web/src/engine/osk/src/banner/suggestionBanner.ts index 9f97617489..fd2530735e 100644 --- a/web/src/engine/osk/src/banner/suggestionBanner.ts +++ b/web/src/engine/osk/src/banner/suggestionBanner.ts @@ -9,6 +9,9 @@ import UITouchHandlerBase from '../input/event-interpreter/uiTouchHandlerBase.js import { DeviceSpec, Keyboard, KeyboardProperties } from '@keymanapp/keyboard-processor'; import { Banner } from './banner.js'; import EventEmitter from 'eventemitter3'; +import { ParsedLengthStyle } from '../lengthStyle.js'; +import { getFontSizeStyle } from '../fontSizeUtils.js'; +import { getTextMetrics } from '../keyboard-layout/getTextMetrics.js'; export class BannerSuggestion { div: HTMLDivElement; @@ -81,17 +84,45 @@ export class BannerSuggestion { * Function update * @param {string} id Element ID for the suggestion span * @param {Suggestion} suggestion Suggestion from the lexical model + * @param fontStyle The CSS styling expected for the suggestion text + * @param emSize The font size represented by 1em (in px, as from getComputedStyle on document.body) + * @param targetWidth + * @param collapsedTargetWidth * Description Update the ID and text of the BannerSuggestionSpec */ - public update(suggestion: Suggestion) { + public update( + suggestion: Suggestion, + fontStyle: CSSStyleDeclaration, + emSize: number, + targetWidth: number, + collapsedTargetWidth: number + ) { this._suggestion = suggestion; - this.updateText(); - } - private updateText() { + // TODO: if the option is highlighted, maybe don't disable transitions? + this.container.style.transition = 'none'; // temporarily disable transition effects. + let display = this.generateSuggestionText(this.rtl); this.container.replaceChild(display, this.display); this.display = display; + + // Compute the raw text-width of the suggestion and determine specs for the default (collapsed) styling. + const optionCollapseStyle = this.container.style; + + const rawMetrics = getTextMetrics(suggestion.displayAs, emSize, fontStyle); + const rawTextWidth = rawMetrics.width; + optionCollapseStyle.minWidth = `${targetWidth}px`; + + if(rawTextWidth > collapsedTargetWidth) { + optionCollapseStyle.marginLeft = `${collapsedTargetWidth - rawTextWidth}px`; + } else { + optionCollapseStyle.marginLeft = '0px'; + } + + this.container.offsetWidth; // To 'flush' the changes before re-enabling transition animations. + this.container.offsetLeft; + + this.container.style.transition = ''; // Re-enable them (it's set on the element's class) } public isEmpty(): boolean { @@ -310,12 +341,21 @@ export class SuggestionBanner extends Banner { public onSuggestionUpdate = (suggestions: Suggestion[]): void => { this.currentSuggestions = suggestions; + const fontStyle = getComputedStyle(this.options[0].div); + const emSizeStr = getComputedStyle(document.body).fontSize; + const emSize = getFontSizeStyle(emSizeStr).val; + + const textStyle = getComputedStyle(this.options[0].container.firstChild as HTMLSpanElement); + + // TODO: polish up; do a calculation that leaves perfect, clean edges when displaying exactly three options. + const targetWidth = this.width / 3; // Not fancy; it'll leave rough edges. But... it'll do for a demo. + const textLeftPad = new ParsedLengthStyle(textStyle.paddingLeft || '2px'); // computedStyle will fail if the element's not in the DOM yet. + const textRightPad = new ParsedLengthStyle(textStyle.paddingRight || '2px'); + + const collapsedTargetWidth = targetWidth - textLeftPad.val - textRightPad.val; // Assumes fixed px padding. + this.options.forEach((option: BannerSuggestion, i: number) => { - if(i < suggestions.length) { - option.update(suggestions[i]); - } else { - option.update(null); - } + option.update(i < suggestions.length ? suggestions[i] : null, fontStyle, emSize, targetWidth, collapsedTargetWidth); }); } } diff --git a/web/src/engine/osk/src/keyboard-layout/getTextMetrics.ts b/web/src/engine/osk/src/keyboard-layout/getTextMetrics.ts new file mode 100644 index 0000000000..d7856ec232 --- /dev/null +++ b/web/src/engine/osk/src/keyboard-layout/getTextMetrics.ts @@ -0,0 +1,50 @@ +import { getFontSizeStyle } from "../fontSizeUtils.js"; + +let metricsCanvas: HTMLCanvasElement; + +/** + * Uses canvas.measureText to compute and return the width of the given text of given font in pixels. + * + * @param {String} text The text to be rendered. + * @param {String} style The CSSStyleDeclaration for an element to measure against, without modification. + * + * @see https://stackoverflow.com/questions/118241/calculate-text-width-with-javascript/21015393#21015393 + * This version has been substantially modified to work for this particular application. + */ +export function getTextMetrics(text: string, emScale: number, style: {fontFamily?: string, fontSize: string}): TextMetrics { + // Since we may mutate the incoming style, let's make sure to copy it first. + // Only the relevant properties, though. + style = { + fontFamily: style.fontFamily, + fontSize: style.fontSize + }; + + // A final fallback - having the right font selected makes a world of difference. + if(!style.fontFamily) { + style.fontFamily = getComputedStyle(document.body).fontFamily; + } + + if(!style.fontSize || style.fontSize == "") { + style.fontSize = '1em'; + } + + let fontFamily = style.fontFamily; + let fontSpec = getFontSizeStyle(style.fontSize); + + var fontSize: string; + if(fontSpec.absolute) { + // We've already got an exact size - use it! + fontSize = fontSpec.val + 'px'; + } else { + fontSize = fontSpec.val * emScale + 'px'; + } + + // re-use canvas object for better performance + metricsCanvas = metricsCanvas ?? document.createElement("canvas"); + + var context = metricsCanvas.getContext("2d"); + context.font = fontSize + " " + fontFamily; + var metrics = context.measureText(text); + + return metrics; +} \ No newline at end of file diff --git a/web/src/engine/osk/src/keyboard-layout/oskKey.ts b/web/src/engine/osk/src/keyboard-layout/oskKey.ts index 77d8927b60..56bed368d1 100644 --- a/web/src/engine/osk/src/keyboard-layout/oskKey.ts +++ b/web/src/engine/osk/src/keyboard-layout/oskKey.ts @@ -9,6 +9,7 @@ import buttonClassNames from '../buttonClassNames.js'; import { KeyElement } from '../keyElement.js'; import VisualKeyboard from '../visualKeyboard.js'; +import { getTextMetrics } from './getTextMetrics.js'; export class OSKKeySpec implements LayoutKey { id: string; @@ -158,53 +159,6 @@ export default abstract class OSKKey { } } - /** - * Uses canvas.measureText to compute and return the width of the given text of given font in pixels. - * - * @param {String} text The text to be rendered. - * @param {String} style The CSSStyleDeclaration for an element to measure against, without modification. - * - * @see https://stackoverflow.com/questions/118241/calculate-text-width-with-javascript/21015393#21015393 - * This version has been substantially modified to work for this particular application. - */ - static getTextMetrics(text: string, emScale: number, style: {fontFamily?: string, fontSize: string}): TextMetrics { - // Since we may mutate the incoming style, let's make sure to copy it first. - // Only the relevant properties, though. - style = { - fontFamily: style.fontFamily, - fontSize: style.fontSize - }; - - // A final fallback - having the right font selected makes a world of difference. - if(!style.fontFamily) { - style.fontFamily = getComputedStyle(document.body).fontFamily; - } - - if(!style.fontSize || style.fontSize == "") { - style.fontSize = '1em'; - } - - let fontFamily = style.fontFamily; - let fontSpec = getFontSizeStyle(style.fontSize); - - var fontSize: string; - if(fontSpec.absolute) { - // We've already got an exact size - use it! - fontSize = fontSpec.val + 'px'; - } else { - fontSize = fontSpec.val * emScale + 'px'; - } - - // re-use canvas object for better performance - var canvas: HTMLCanvasElement = OSKKey.getTextMetrics['canvas'] || - (OSKKey.getTextMetrics['canvas'] = document.createElement("canvas")); - var context = canvas.getContext("2d"); - context.font = fontSize + " " + fontFamily; - var metrics = context.measureText(text); - - return metrics; - } - /** * Calculate the font size required for a key cap, scaling to fit longer text * @param vkbd @@ -234,7 +188,7 @@ export default abstract class OSKKey { } let fontSpec = getFontSizeStyle(style.fontSize || '1em'); - let metrics = OSKKey.getTextMetrics(text, emScale, style); + let metrics = getTextMetrics(text, emScale, style); const MAX_X_PROPORTION = 0.90; const MAX_Y_PROPORTION = 0.90; @@ -378,7 +332,7 @@ export default abstract class OSKKey { // Check the key's display width - does the key visualize well? let emScale = vkbd.getKeyEmFontSize(); - var width: number = OSKKey.getTextMetrics(keyText, emScale, styleSpec).width; + var width: number = getTextMetrics(keyText, emScale, styleSpec).width; if(width == 0 && keyText != '' && keyText != '\xa0') { // Add the Unicode 'empty circle' as a base support for needy diacritics. diff --git a/web/src/resources/osk/kmwosk.css b/web/src/resources/osk/kmwosk.css index fe8990f8fe..8a0e597095 100644 --- a/web/src/resources/osk/kmwosk.css +++ b/web/src/resources/osk/kmwosk.css @@ -66,11 +66,6 @@ font-size: 0.75em; } -.kmw-suggestion-text { - padding-left: 4px; - padding-right: 4px; -} - .kmw-suggest-banner-scroller { overflow-x: hidden; width: 100%; @@ -110,7 +105,6 @@ .phone.ios .kmw-key.kmw-key-shift-on, .phone.ios .kmw-key.kmw-key-special-on {color:#fff;background-color:#88f;} .phone.ios .kmw-key.kmw-key-touched {background-color:#447;} -.phone.ios .kmw-suggest-option.kmw-suggest-touched {background-color:#88f;} .phone.ios .kmw-key-deadkey{color:#048204;background-color:#fdfdfe;} /* Probably best to make this its own CSS that can be optionally included? */ @@ -133,6 +127,14 @@ width: 100%; } +.ios .kmw-suggest-option::before { + background: linear-gradient(90deg, #cfd3d9 0%, transparent 100%); +} + +.ios .kmw-suggest-option::after { + background: linear-gradient(90deg, transparent 0%, #cfd3d9 100%); +} + .ios .kmw-banner-bar .kmw-suggest-option { display:inline-block; text-align: center; @@ -147,7 +149,18 @@ color: #000; } -.phone.ios .kmw-suggest-option.kmw-suggest-touched {background-color:#88f;} +.phone.ios .kmw-suggest-option.kmw-suggest-touched, +.tablet.ios .kmw-suggest-option.kmw-suggest-touched {background-color:#88f;} + +.phone.ios .kmw-suggest-option.kmw-suggest-touched::before, +.tablet.ios .kmw-suggest-option.kmw-suggest-touched::before { + background: linear-gradient(90deg, #88f 0%, transparent 100%); +} + +.phone.ios .kmw-suggest-option.kmw-suggest-touched::after, +.tablet.ios .kmw-suggest-option.kmw-suggest-touched::after { + background: linear-gradient(90deg, transparent 0%, #88f 100%); +} .phone.ios.kmw-osk-frame, .tablet.ios.kmw-osk-frame { @@ -165,6 +178,14 @@ background-color: #0f1319; } + .ios .kmw-suggest-option::before { + background: linear-gradient(90deg, #0f1319 0%, transparent 100%); + } + + .ios .kmw-suggest-option::after { + background: linear-gradient(90deg, transparent 0%, #0f1319 100%); + } + .ios .kmw-banner-bar .kmw-banner-separator { border-left: solid 1px #8a8d90 } @@ -203,6 +224,14 @@ width: 100%; } +.phone.android .kmw-suggest-option::before { + background: linear-gradient(90deg, #222 0%, transparent 100%); +} + +.phone.android .kmw-suggest-option::after { + background: linear-gradient(90deg, transparent 0%, #222 100%); +} + .phone.android .kmw-banner-bar .kmw-suggest-option { display:inline-block; text-align: center; @@ -215,6 +244,14 @@ .phone.android .kmw-suggest-option.kmw-suggest-touched {background-color:#bbb;} +.phone.android .kmw-suggest-option.kmw-suggest-touched::before { + background: linear-gradient(90deg, #bbb 0%, transparent 100%); +} + +.phone.android .kmw-suggest-option.kmw-suggest-touched::after { + background: linear-gradient(90deg, transparent 0%, #bbb 100%); +} + .tablet.kmw-osk-frame{left:0;bottom:0;width:100%;height:144px;overflow-y:visible; background-color:rgba(0,0,0,0.8);-webkit-user-select:none;} .tablet .kmw-osk-inner-frame{margin:0;background:transparent;} @@ -235,7 +272,6 @@ .tablet.ios .kmw-key.kmw-key-shift-on, .tablet.ios .kmw-key.kmw-key-special-on {color:#fff;background-color:#88f;} .tablet.ios .kmw-key.kmw-key-touched {background-color:#447;} -.tablet.ios .kmw-suggest-option.kmw-suggest-touched {background-color:#88f;} .tablet.ios .kmw-key-deadkey{color:#048204;background-color:#fdfdfe;} /* Probably best to make this its own CSS that can be optionally included? */ @@ -273,6 +309,14 @@ width: 100%; } +.tablet.android .kmw-suggest-option::before { + background: linear-gradient(90deg, #b4b4b8 0px, transparent 100%); +} + +.tablet.android .kmw-suggest-option::after { + background: linear-gradient(90deg, transparent 0%, #b4b4b8 100%); +} + .tablet.android .kmw-banner-bar .kmw-suggest-option { display:inline-block; text-align: center; @@ -282,11 +326,20 @@ .tablet.android .kmw-suggestion-text { color:#77f; } + .tablet.android .kmw-suggest-option.kmw-suggest-touched {background-color:#447;} +.tablet.android .kmw-suggest-option.kmw-suggest-touched::before { + background: linear-gradient(90deg, #447 0px, transparent 100%); +} + +.tablet.android .kmw-suggest-option.kmw-suggest-touched::after { + background: linear-gradient(90deg, transparent 0%, #447 100%); +} + /* Vertical centering of text labels on keys */ .kmw-key {text-align:center; white-space:nowrap;} -.kmw-key:before {content:'.'; display:inline-block; height:100%; vertical-align:middle; max-width:0px; visibility:hidden;} +.kmw-key::before {content:'.'; display:inline-block; height:100%; vertical-align:middle; max-width:0px; visibility:hidden;} .kmw-key span {display:inline-block} @@ -300,7 +353,7 @@ .desktop .kmw-key-label{position:absolute;left:2px;top:2px;font:0.5em Arial;color:#888;background-color:transparent;} /* Popup icon style (and content)*/ -.kmw-key-popup-icon:before{content:'\2022';} +.kmw-key-popup-icon::before{content:'\2022';} .kmw-key-popup-icon{position:absolute;display:block;visibility:visible;right:4%;top:1%;/*width:8px;height:8px;*/ font:bold 0.5em Arial;color:#aaa;} @@ -321,22 +374,79 @@ .kmw-footer-caption{color:#fff;font:0.7em Arial;margin:0 0 0 4px;} .kmw-banner-bar{height:100%; width:100%; margin:0; background-color:darkorange; display: inline-block; white-space: nowrap;} + +/* Creates a gradient to fade text at the borders, providing visual indication of overflow */ +/* Make sure the non-transparent color of the gradient matches .kmw-banner-bar's background-color. */ +.kmw-suggest-option::before, +.kmw-suggest-option::after { + position:absolute; + + /* Set scrollable-suggestion fade width here. Make sure to also set .kmw-suggestion-text + * padding-left and padding-right accordingly! + */ + width: 4px; + height: 100%; + content: ''; + top: 0; + z-index:10999; /* z-indexes this _behind_ the 'option' element that hosts the scrollable zone. */ + user-select: none; + pointer-events: none; /* Ensures click-through! But apparently not touch-through. */ + touch-action: none; /* Doesn't seem to allow touch-through, though - even with touch-action: none */ + /* https://stackoverflow.com/q/21474722 - poster never could find a solution, and settled*/ + /* on the same workaround: a 'before' and 'after' piece instead of a single overlay.*/ +} + +.kmw-suggest-option::before { + background: linear-gradient(90deg, darkorange 0%, transparent 100%); + left: 0; +} + +.kmw-suggest-option::after { + background: linear-gradient(90deg, transparent 0%, darkorange 100%); + right: 0; +} + +/* Fallback suggestion-selection highlighting */ +.kmw-suggest-option.kmw-suggest-touched { + background: #bbb; +} + +/* Creates a gradient to fade text at the borders, providing visual indication of overflow */ +/* Make sure the non-transparent color of the gradient matches .kmw-banner-bar's background-color. */ +.kmw-suggest-option.kmw-suggest-touched::before { + background: linear-gradient(90deg, #bbb 0%, transparent 100%); +} + +.kmw-suggest-option.kmw-suggest-touched::after { + background: linear-gradient(90deg, transparent 0%, #bbb 100%); +} + .kmw-banner-bar .kmw-banner-separator {border-left: solid 1px #8a8d90; width: 0px; vertical-align: middle; height: 45%; display: inline-block;} -.kmw-banner-bar .kmw-suggest-option {display:inline-block; text-align: center; height: 85%; overflow-x:hidden} -.kmw-suggestion-text{color:#fff; line-height: normal; position: relative; vertical-align: middle;} +.kmw-banner-bar .kmw-suggest-option {display:inline-block; text-align: center; height: 85%; position: relative; z-index: 11000} +.kmw-suggestion-text { + color:#fff; + line-height: normal; + position: relative; + vertical-align: middle; + padding-left: 4px; /* To prevent start & end of suggestion from being affected by gradient effects */ + padding-right: 4px; /* Set these to match scrollable-suggestion fade width set above. */ + width: max-content; /* Ensure the text span acts like it contains its text */ + min-width: calc(100% - 8px); /* To ensure the span stays centered; also adjusts for scrollable-suggestion fade width. */ + white-space: nowrap; +} .kmw-footer-resize{cursor:se-resize;position:absolute;right:2px;bottom:2px;width:16px;height:16px;overflow:hidden; font-family:SpecialOSK;color:white;} .kmw-footer-resize:hover{font-weight:bold;} -.kmw-footer-resize:before {content:'\e023';} +.kmw-footer-resize::before {content:'\e023';} .kmw-title-bar-image {cursor: default; float:right; padding: 2px 2px 0 0; width:16px; height:16px; font-family:SpecialOSK; color:white;} .kmw-title-bar-image:hover{font-weight:bold;} -#kmw-pin-image:before{content:'\e024';} -#kmw-config-image:before{content:'\e030';} -#kmw-help-image:before{content:'\e042';} -#kmw-close-button:before {content:'\e025';} +#kmw-pin-image::before{content:'\e024';} +#kmw-config-image::before{content:'\e030';} +#kmw-help-image::before{content:'\e042';} +#kmw-close-button::before {content:'\e025';} /* Common key appearance styles (can override with form-factor styles if necessary) */ .kmw-key-default{color:#000;background-color:#eee;} @@ -461,7 +571,7 @@ div.android div.kmw-keytip-cap { /* Box styles for keyboard-specific OSK (e.g. EuroLatin) and if no keyboard active (desktop only) */ .kmw-osk-static, .kmw-osk-none{text-align:left;font:12px sans-serif;border:solid 1px #ad4a28;color:blue;background-color:white;} .kmw-osk-none{padding:4px 6px 6px} -.kmw-osk-none:before{content:'Installing keyboard...';} +.kmw-osk-none::before{content:'Installing keyboard...';} /* OSK language menu styles */ #kmw-language-menu{position:fixed;left:0;width:232px;max-width:232px;z-index:10004;background-color:rgba(128,128,128,1); @@ -549,7 +659,7 @@ div.android div.kmw-keytip-cap { border:3px solid #ad4a28;border-radius:8px;text-align:center;padding:0px;background:white;} .kmw-alert-close{float:right; height:24px; width:24px; font:1em bold Arial,sans-serif;color:#ad4a28;} /*.kmw-alert-close{float:right; height:24px; width:24px; font:2em bold Arial,sans-serif;color:#ad4a28;} */ -.kmw-alert-close:before{content:'\00d7'} +.kmw-alert-close::before{content:'\00d7'} /*.kmw-alert-close{float:right;background:url('icons.gif') no-repeat -30px 0; height:13px; width:15px;}*/ .kmw-wait-text{clear:both; margin:4px;white-space:nowrap;} .kmw-wait-graphic{width:100%;min-height:19px;background:url('ajax-loader.gif') no-repeat;background-position:center top;} From 7177a495cf67bbf4e6a0d6cef424f0a0b8aa07cd Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Thu, 22 Dec 2022 11:06:30 +0700 Subject: [PATCH 03/60] feat(web): variable-width suggestions, padding-fill if total width too narrow --- .../engine/osk/src/banner/suggestionBanner.ts | 165 +++++++++++++++--- 1 file changed, 139 insertions(+), 26 deletions(-) diff --git a/web/src/engine/osk/src/banner/suggestionBanner.ts b/web/src/engine/osk/src/banner/suggestionBanner.ts index fd2530735e..89004b584d 100644 --- a/web/src/engine/osk/src/banner/suggestionBanner.ts +++ b/web/src/engine/osk/src/banner/suggestionBanner.ts @@ -13,10 +13,25 @@ import { ParsedLengthStyle } from '../lengthStyle.js'; import { getFontSizeStyle } from '../fontSizeUtils.js'; import { getTextMetrics } from '../keyboard-layout/getTextMetrics.js'; +// TODO: finalize + document +interface OptionFormatSpec { + minWidth?: number; + paddingWidth: number, + emSize: number, + styleForFont: CSSStyleDeclaration + + collapsedWidth?: number +} export class BannerSuggestion { div: HTMLDivElement; container: HTMLDivElement; private display: HTMLSpanElement; + + private _collapsedWidth: number; + private _textWidth: number; + private _minWidth: number; + private _paddingWidth: number; + private fontFamily?: string; private rtl: boolean = false; @@ -90,41 +105,95 @@ export class BannerSuggestion { * @param collapsedTargetWidth * Description Update the ID and text of the BannerSuggestionSpec */ - public update( - suggestion: Suggestion, - fontStyle: CSSStyleDeclaration, - emSize: number, - targetWidth: number, - collapsedTargetWidth: number - ) { + public update(suggestion: Suggestion, format: OptionFormatSpec) { this._suggestion = suggestion; - // TODO: if the option is highlighted, maybe don't disable transitions? - this.container.style.transition = 'none'; // temporarily disable transition effects. - let display = this.generateSuggestionText(this.rtl); this.container.replaceChild(display, this.display); this.display = display; - // Compute the raw text-width of the suggestion and determine specs for the default (collapsed) styling. - const optionCollapseStyle = this.container.style; - - const rawMetrics = getTextMetrics(suggestion.displayAs, emSize, fontStyle); - const rawTextWidth = rawMetrics.width; - optionCollapseStyle.minWidth = `${targetWidth}px`; - - if(rawTextWidth > collapsedTargetWidth) { - optionCollapseStyle.marginLeft = `${collapsedTargetWidth - rawTextWidth}px`; - } else { - optionCollapseStyle.marginLeft = '0px'; + // Set internal properties for use in format calculations. + if(format.minWidth !== undefined) { + this._minWidth = format.minWidth; } + this._paddingWidth = format.paddingWidth; + this._collapsedWidth = format.collapsedWidth; + + if(suggestion && suggestion.displayAs) { + const rawMetrics = getTextMetrics(suggestion.displayAs, format.emSize, format.styleForFont); + this._textWidth = rawMetrics.width; + } else { + this._textWidth = 0; + } + + this.updateLayout(); + } + + public updateLayout() { + if(!this.suggestion && this.index != 0) { + this.div.style.width='0px'; + return; + } else { + this.div.style.width=''; + } + + // TODO: if the option is highlighted, maybe don't disable transitions? + this.container.style.transition = 'none'; // temporarily disable transition effects. + + const collapserStyle = this.container.style; + collapserStyle.minWidth = this.collapsedWidth + 'px'; + collapserStyle.marginLeft = (this.collapsedWidth - this.expandedWidth) + 'px'; + this.container.offsetWidth; // To 'flush' the changes before re-enabling transition animations. this.container.offsetLeft; this.container.style.transition = ''; // Re-enable them (it's set on the element's class) } + + + public get targetCollapsedWidth(): number { + return this._collapsedWidth; + } + + public get textWidth(): number { + return this._textWidth; + } + + public get paddingWidth(): number { + return this._paddingWidth; + } + + public get minWidth(): number { + return this._minWidth; + } + + public set minWidth(val: number) { + this._minWidth = val; + } + + public get expandedWidth(): number { + // minWidth must be defined AND greater for the conditional to return this.minWidth. + return this.minWidth > this.spanWidth ? this.minWidth : this.spanWidth; + } + + public get spanWidth(): number { + let spanWidth = this.textWidth ?? 0; + if(spanWidth) { + spanWidth += this.paddingWidth ?? 0; + } + + return spanWidth; + } + + public get collapsedWidth(): number { + let maxWidth = this.targetCollapsedWidth < this.expandedWidth ? this.targetCollapsedWidth : this.expandedWidth; + + // Will return maxWidth if this.minWidth is undefined. + return (this.minWidth > maxWidth ? this.minWidth : maxWidth); + } + public isEmpty(): boolean { return !this._suggestion; } @@ -177,6 +246,8 @@ export class SuggestionBanner extends Banner { private currentSuggestions: Suggestion[] = []; private options : BannerSuggestion[] = []; + private separators: HTMLElement[] = []; + private hostDevice: DeviceSpec; private manager: SuggestionInputManager; @@ -211,8 +282,10 @@ export class SuggestionBanner extends Banner { buildInternals(rtl: boolean) { if(this.options.length > 0) { - this.options.splice(0, this.options.length); // Clear the array. + this.options = []; + this.separators = []; } + for (var i=0; i { - option.update(i < suggestions.length ? suggestions[i] : null, fontStyle, emSize, targetWidth, collapsedTargetWidth); - }); + let totalWidth = 0; + let displayCount = 0; + + for (let i=0; i i) { + const suggestion = suggestions[i]; + d.update(suggestion, optionFormat); + + totalWidth += d.collapsedWidth; + displayCount++; + } else { + d.update(null, optionFormat); + } + } + + // Ensure one suggestion is always displayed, even if empty. (Keep the separators out) + displayCount = displayCount || 1; + + if(totalWidth < this.width) { + let separatorWidth = (this.width * 0.01 * (displayCount-1)); + let fillPadding = (this.width - totalWidth - separatorWidth) / displayCount; + + for(let i=0; i < displayCount; i++) { + const d = this.options[i]; + + d.minWidth = d.collapsedWidth + fillPadding; + d.updateLayout(); + } + } + + // Hide any separators beyond the final displayed suggestion + for(let i=0; i < SuggestionBanner.SUGGESTION_LIMIT - 1; i++) { + this.separators[i].style.display = i < displayCount - 1 ? '' : 'none'; + } } } From 870e7341c0bf4d66e480ad9f85601120a73c3cbc Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Fri, 23 Dec 2022 13:41:08 +0700 Subject: [PATCH 04/60] fix(web): basic rtl handling for new features --- web/src/engine/osk/src/banner/suggestionBanner.ts | 7 ++++++- web/src/resources/osk/kmwosk.css | 3 ++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/web/src/engine/osk/src/banner/suggestionBanner.ts b/web/src/engine/osk/src/banner/suggestionBanner.ts index 89004b584d..357bc11b33 100644 --- a/web/src/engine/osk/src/banner/suggestionBanner.ts +++ b/web/src/engine/osk/src/banner/suggestionBanner.ts @@ -143,7 +143,12 @@ export class BannerSuggestion { const collapserStyle = this.container.style; collapserStyle.minWidth = this.collapsedWidth + 'px'; - collapserStyle.marginLeft = (this.collapsedWidth - this.expandedWidth) + 'px'; + + if(this.rtl) { + collapserStyle.marginRight = (this.collapsedWidth - this.expandedWidth) + 'px'; + } else { + collapserStyle.marginLeft = (this.collapsedWidth - this.expandedWidth) + 'px'; + } this.container.offsetWidth; // To 'flush' the changes before re-enabling transition animations. this.container.offsetLeft; diff --git a/web/src/resources/osk/kmwosk.css b/web/src/resources/osk/kmwosk.css index 8a0e597095..ccec96d798 100644 --- a/web/src/resources/osk/kmwosk.css +++ b/web/src/resources/osk/kmwosk.css @@ -87,7 +87,8 @@ } .kmw-suggest-option.kmw-suggest-touched .kmw-suggestion-container { - margin-left: 0px !important /* Overrides 'collapse' styling, which is accomplished via negative margin-left */ + margin-left: 0px !important; /* Overrides 'collapse' styling, which is accomplished via negative margin-left */ + margin-right: 0px !important; /* The same, but for RTL languages */ } .phone.windows .kmw-key-row{max-width:80%;} From b76b8714760996d24c03a7a3d21e7954a5843bb1 Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Wed, 4 Jan 2023 14:40:41 +0700 Subject: [PATCH 05/60] change(web): js-side animation, LTR counterscrolling on suggestion expansion --- .../engine/osk/src/banner/suggestionBanner.ts | 207 +++++++++++++++++- .../event-interpreter/uiTouchHandlerBase.ts | 40 ++-- web/src/resources/osk/kmwosk.css | 6 - 3 files changed, 230 insertions(+), 23 deletions(-) diff --git a/web/src/engine/osk/src/banner/suggestionBanner.ts b/web/src/engine/osk/src/banner/suggestionBanner.ts index 357bc11b33..123b5201c3 100644 --- a/web/src/engine/osk/src/banner/suggestionBanner.ts +++ b/web/src/engine/osk/src/banner/suggestionBanner.ts @@ -156,8 +156,6 @@ export class BannerSuggestion { this.container.style.transition = ''; // Re-enable them (it's set on the element's class) } - - public get targetCollapsedWidth(): number { return this._collapsedWidth; } @@ -199,6 +197,20 @@ export class BannerSuggestion { return (this.minWidth > maxWidth ? this.minWidth : maxWidth); } + public get currentWidth(): number { + return this.div.offsetWidth; + } + + public set currentWidth(val: number) { + // TODO: probably should set up errors or something here... + if(val < this.collapsedWidth) { + val = this.collapsedWidth; + } else if(val > this.expandedWidth) { + val = this.expandedWidth; + } + this.container.style.marginLeft = `${val - this.expandedWidth}px`; + } + public isEmpty(): boolean { return !this._suggestion; } @@ -257,6 +269,7 @@ export class SuggestionBanner extends Banner { private manager: SuggestionInputManager; private readonly container: HTMLElement; + private highlightAnimation: SuggestionExpandContractAnimation; readonly type = 'suggestion'; @@ -307,6 +320,11 @@ export class SuggestionBanner extends Banner { let indexToInsert = rtl ? SuggestionBanner.SUGGESTION_LIMIT - i -1 : i; this.container.appendChild(this.options[indexToInsert].div); + // RTL should start right-aligned, thus @ max scroll. + if(rtl) { + this.container.scrollLeft = this.container.scrollWidth; + } + if(i != SuggestionBanner.SUGGESTION_LIMIT - 1) { // Adds a 'separator' div element for UI purposes. let separatorDiv = createUnselectableElement('div'); @@ -340,8 +358,24 @@ export class SuggestionBanner extends Banner { if(on && classes.indexOf(cs) < 0) { elem.className=classes+cs; + if(this.highlightAnimation) { + this.highlightAnimation.decouple(); + } + + this.highlightAnimation = new SuggestionExpandContractAnimation(this.container, suggestion, false); + this.highlightAnimation.expand(); } else { elem.className=classes.replace(cs,''); + if(!this.highlightAnimation) { + this.highlightAnimation = new SuggestionExpandContractAnimation(this.container, suggestion, false); + } + this.highlightAnimation.collapse(); + } + }); + + this.manager.events.on('scrollLeft', (val) => { + if(this.highlightAnimation) { + this.highlightAnimation.setBaseScroll(val); } }); @@ -482,6 +516,171 @@ interface SuggestionInputEventMap { highlight: (bannerSuggestion: BannerSuggestion, state: boolean) => void, apply: (bannerSuggestion: BannerSuggestion) => void; hold: (bannerSuggestion: BannerSuggestion) => void; + scrollLeft: (val: number) => void; +} + + +class SuggestionExpandContractAnimation { + private scrollContainer: HTMLElement | null; + private option: BannerSuggestion; + + private collapsedScrollLeft: number; + + private startTimestamp: number; + private pendingAnimation: number; + + private static TRANSITION_TIME = 250; // in ms. + + constructor(scrollContainer: HTMLElement, option: BannerSuggestion, forRTL: boolean) { + this.scrollContainer = scrollContainer; + this.option = option; + this.collapsedScrollLeft = scrollContainer.scrollLeft; + } + + public setBaseScroll(val: number) { + this.collapsedScrollLeft = val; + + // Attempt to sync the banner-scroller's offset update with that of the + // animation for expansion and collapsing. + window.requestAnimationFrame(this.setOffsetScroll); + + // this.setOffsetScroll(); + } + + // the "fun", top-level banner part. + private setOffsetScroll = () => { + // If we've been 'decoupled', a different instance (likely for a different suggestion) + // is responsible for counter-scrolling. + if(!this.scrollContainer) { + return; + } + const baseScrollOffset = this.option.currentWidth - this.option.collapsedWidth; + + // TODO: clamping logic + + let finalTargetScrollLeft = this.collapsedScrollLeft + baseScrollOffset; + this.scrollContainer.scrollLeft = finalTargetScrollLeft; + + // Prevent "jitters" during counterscroll that occur on expansion / collapse animation. + // A one-frame "error correction" effect at the end of animation is far less jarring. + if(this.pendingAnimation) { + // scrollLeft doesn't work well with fractional values, unlike marginLeft / marginRight + let fractionalOffset = this.scrollContainer.scrollLeft - finalTargetScrollLeft + // So we put the fractional difference into marginLeft to force it to sync. + this.option.currentWidth += fractionalOffset; + } + } + + public decouple() { + this.scrollContainer = null; + } + + private clear() { + this.startTimestamp = null; + window.cancelAnimationFrame(this.pendingAnimation); + this.pendingAnimation = null; + } + + public expand() { + // Cancel any prior iterating animation-frame commands. + this.clear(); + + // set timestamp, adjusting the current time based on intermediate progress + this.startTimestamp = performance.now(); + + let progress = this.option.currentWidth - this.option.collapsedWidth; + let expansionDiff = this.option.expandedWidth - this.option.collapsedWidth; + + if(progress != 0) { + // Offset the timestamp by noting what start time would have given rise to + // the current position, keeping related animations smooth. + this.startTimestamp -= (progress / expansionDiff) * SuggestionExpandContractAnimation.TRANSITION_TIME; + } + + this.pendingAnimation = window.requestAnimationFrame(this._expand); + } + + private _expand = (timestamp: number) => { + if(this.startTimestamp === undefined) { + return; // No active expand op exists. May have been cancelled via `clear`. + } + + let progressTime = timestamp - this.startTimestamp; + let fin = progressTime > SuggestionExpandContractAnimation.TRANSITION_TIME; + + if(fin) { + progressTime = SuggestionExpandContractAnimation.TRANSITION_TIME; + } + + // -- Part 1: handle option expand / collapse state -- + let expansionDiff = this.option.expandedWidth - this.option.collapsedWidth; + let expansionRatio = progressTime / SuggestionExpandContractAnimation.TRANSITION_TIME; + + // expansionDiff * expansionRatio: the total adjustment from 'collapsed' width, in px. + const expansionPx = expansionDiff * expansionRatio; + this.option.currentWidth = expansionPx + this.option.collapsedWidth; + + // Part 2: trigger the next animation frame. + if(!fin) { + this.pendingAnimation = window.requestAnimationFrame(this._expand); + } else { + this.clear(); + } + + // Part 3: perform any needed counter-scrolling, scroll clamping, etc + // Existence of a followup animation frame is part of the logic, so keep this 'after'! + this.setOffsetScroll(); + }; + + public collapse() { + // Cancel any prior iterating animation-frame commands. + this.clear(); + + // set timestamp, adjusting the current time based on intermediate progress + this.startTimestamp = performance.now(); + + let progress = this.option.expandedWidth - this.option.currentWidth; + let expansionDiff = this.option.expandedWidth - this.option.collapsedWidth; + + if(progress != 0) { + // Offset the timestamp by noting what start time would have given rise to + // the current position, keeping related animations smooth. + this.startTimestamp -= (progress / expansionDiff) * SuggestionExpandContractAnimation.TRANSITION_TIME; + } + + this.pendingAnimation = window.requestAnimationFrame(this._collapse); + } + + private _collapse = (timestamp: number) => { + if(this.startTimestamp === undefined) { + return; // No active collapse op exists. May have been cancelled via `clear`. + } + + let progressTime = timestamp - this.startTimestamp; + let fin = progressTime > SuggestionExpandContractAnimation.TRANSITION_TIME; + if(fin) { + progressTime = SuggestionExpandContractAnimation.TRANSITION_TIME; + } + + // -- Part 1: handle option expand / collapse state -- + let expansionDiff = this.option.expandedWidth - this.option.collapsedWidth; + let expansionRatio = 1 - progressTime / SuggestionExpandContractAnimation.TRANSITION_TIME; + + // expansionDiff * expansionRatio: the total adjustment from 'collapsed' width, in px. + const expansionPx = expansionDiff * expansionRatio; + this.option.currentWidth = expansionPx + this.option.collapsedWidth; + + // Part 2: trigger the next animation frame. + if(!fin) { + this.pendingAnimation = window.requestAnimationFrame(this._collapse); + } else { + this.clear(); + } + + // Part 3: perform any needed counter-scrolling, scroll clamping, etc + // Existence of a followup animation frame is part of the logic, so keep this 'after'! + this.setOffsetScroll(); + }; } class SuggestionInputManager extends UITouchHandlerBase { @@ -521,6 +720,10 @@ class SuggestionInputManager extends UITouchHandlerBase { return null; } + protected onScrollLeftUpdate(val: number): void { + this.events.emit('scrollLeft', val); + } + protected highlight(t: HTMLDivElement, on: boolean): void { let suggestion = t['suggestion'] as BannerSuggestion; diff --git a/web/src/engine/osk/src/input/event-interpreter/uiTouchHandlerBase.ts b/web/src/engine/osk/src/input/event-interpreter/uiTouchHandlerBase.ts index 0b4956280d..c1e73c6248 100644 --- a/web/src/engine/osk/src/input/event-interpreter/uiTouchHandlerBase.ts +++ b/web/src/engine/osk/src/input/event-interpreter/uiTouchHandlerBase.ts @@ -9,27 +9,33 @@ import { getAbsoluteY } from 'keyman/engine/dom-utils'; * same method blocks native handling of overflow scrolling for touch browsers. */ class ScrollState { - // While we don't currently track y-coordinates here, the class is designed - // to permit tracking them with minimal extra effort if we ever decide to do so. - x: number; totalLength = 0; + baseCoord: InputEventCoordinate; + curCoord: InputEventCoordinate; + baseScrollLeft: number; + // The amount of coordinate 'noise' allowed during a scroll-enabled touch allowed // before interpreting the currently-ongoing touch command as having scrolled. static readonly HAS_SCROLLED_FUDGE_FACTOR = 10; - constructor(coord: InputEventCoordinate) { - this.x = coord.x; + constructor(coord: InputEventCoordinate, baseScrollLeft: number) { + this.baseCoord = coord; + this.curCoord = coord; + this.baseScrollLeft = baseScrollLeft; this.totalLength = 0; } - updateTo(coord: InputEventCoordinate): {deltaX: number} { - let x = this.x; - this.x = coord.x; + updateTo(coord: InputEventCoordinate): {scrollLeft: number} { + let prevCoord = this.curCoord; + this.curCoord = coord; - let deltas = {deltaX: this.x - x}; - this.totalLength += Math.abs(deltas.deltaX); + let deltas = { + scrollLeft: this.baseCoord.x - this.curCoord.x + this.baseScrollLeft + }; + // Track the total amount of scrolling used, even if just a pixel-wide back and forth wiggle. + this.totalLength += Math.abs(this.curCoord.x - prevCoord.x); return deltas; } @@ -230,6 +236,10 @@ export default abstract class UITouchHandlerBase { return false; } + protected onScrollLeftUpdate(val: number) { + this.scroller.scrollLeft = val; + } + touchStart(coord: InputEventCoordinate) { // Determine the selected Target, manage state. this.currentTarget = this.findBestTarget(coord); @@ -245,7 +255,9 @@ export default abstract class UITouchHandlerBase { } // Establish scroll tracking. - this.scrollTouchState = new ScrollState(coord); + if(this.scroller) { + this.scrollTouchState = new ScrollState(coord, this.scroller.scrollLeft); + } // Alright, Target acquired! Now to use it: @@ -338,11 +350,9 @@ export default abstract class UITouchHandlerBase { } if(this.scrollTouchState != null) { - // TODO: Work on smoothing this out; looks like subpixel scroll info gets rounded out, - // and this results in a mild desync. - let deltaX = this.scrollTouchState.updateTo(coord).deltaX; if(this.scroller) { - this.scroller.scrollLeft -= deltaX; + const scrollUpdate = this.scrollTouchState.updateTo(coord); + this.onScrollLeftUpdate(scrollUpdate.scrollLeft); } return; diff --git a/web/src/resources/osk/kmwosk.css b/web/src/resources/osk/kmwosk.css index ccec96d798..c608030fe5 100644 --- a/web/src/resources/osk/kmwosk.css +++ b/web/src/resources/osk/kmwosk.css @@ -83,12 +83,6 @@ .kmw-suggestion-container { height: 100%; - transition: all 0.25s; -} - -.kmw-suggest-option.kmw-suggest-touched .kmw-suggestion-container { - margin-left: 0px !important; /* Overrides 'collapse' styling, which is accomplished via negative margin-left */ - margin-right: 0px !important; /* The same, but for RTL languages */ } .phone.windows .kmw-key-row{max-width:80%;} From a46b203e3d70ed6180bb946d0e199b0d85c5a26a Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Fri, 6 Jan 2023 08:56:51 +0700 Subject: [PATCH 06/60] feat(web): scroll offset to promote expanded option visibility (LTR only) --- .../engine/osk/src/banner/suggestionBanner.ts | 57 +++++++++++++++++-- 1 file changed, 51 insertions(+), 6 deletions(-) diff --git a/web/src/engine/osk/src/banner/suggestionBanner.ts b/web/src/engine/osk/src/banner/suggestionBanner.ts index 123b5201c3..08cb0dc34e 100644 --- a/web/src/engine/osk/src/banner/suggestionBanner.ts +++ b/web/src/engine/osk/src/banner/suggestionBanner.ts @@ -525,6 +525,7 @@ class SuggestionExpandContractAnimation { private option: BannerSuggestion; private collapsedScrollLeft: number; + private originalScrollLeft: number; private startTimestamp: number; private pendingAnimation: number; @@ -535,6 +536,7 @@ class SuggestionExpandContractAnimation { this.scrollContainer = scrollContainer; this.option = option; this.collapsedScrollLeft = scrollContainer.scrollLeft; + this.originalScrollLeft = scrollContainer.scrollLeft; } public setBaseScroll(val: number) { @@ -554,18 +556,61 @@ class SuggestionExpandContractAnimation { if(!this.scrollContainer) { return; } - const baseScrollOffset = this.option.currentWidth - this.option.collapsedWidth; - // TODO: clamping logic + // -- Clamping logic -- - let finalTargetScrollLeft = this.collapsedScrollLeft + baseScrollOffset; - this.scrollContainer.scrollLeft = finalTargetScrollLeft; + // As currently written / defined below, "clamping" refers to alterations to scroll-positioned mapping designed + // to keep as much of the expanded option visible as possible via the offsets below while not pushing + // already-obscured parts of the expanded option into visible range. + // + // In essence, it's an extra offset we apply that is dynamically adjusted depending on scroll position as it changes. + // This offset may be decreased when it is no longer needed to make parts of the element visible. - // Prevent "jitters" during counterscroll that occur on expansion / collapse animation. + // The amount of extra space being taken by a partially or completely expanded suggestion. + const maxWidthToCounterscroll = this.option.currentWidth - this.option.collapsedWidth; + + // How much space existed to the left of the collapsed option in its original position. May be negative. + const originalCounterscrollBuffer = this.option.div.offsetLeft - this.originalScrollLeft; + // TODO: RTL version + + // Only allow a negative buffer in the final positioning if it already existed. + // And only as much as originally existed. + const srcCounterscrollOverflow = -Math.min(originalCounterscrollBuffer, 0); // positive offset into overflow-land. + + // Base position for scrollLeft clamped within std element scroll bounds, including: + // - an adjustment to cover the extra width from expansion + // - preserving the base expected overflow levels + const unclampedExpandingScrollOffset = Math.max(this.collapsedScrollLeft + maxWidthToCounterscroll, 0) - srcCounterscrollOverflow; + const srcUnclampedExpandingScrollOffset = Math.max(this.originalScrollLeft + maxWidthToCounterscroll, 0) - srcCounterscrollOverflow; + // TODO: RTL versions / calculations + logic + + // // Huh - first bugless version didn't actually end up using this. + // const collapsedScrollLeftDelta = this.originalScrollLeft - (unclampedExpandingScrollOffset - maxWidthToCounterscroll + srcCounterscrollOverflow); // neg if touchpoint moving left, + // // pos if touchpoint moving right + // // - scroll moves opposite ("natural") + // // TODO: May need a similar thing for RTL handling. + + // Do not shift an element clipped by the screen border further than its original scroll starting point. + const elementLeftOffsetForClamping = Math.min(unclampedExpandingScrollOffset, srcUnclampedExpandingScrollOffset); + + // Based on the scroll point selected, determine how far to offset scrolls to keep the option in visible range. + // Higher .scrollLeft values make this non-zero and reflect when scroll has begun clipping the element. + const elementLeftOffsetFromBorder = Math.max(elementLeftOffsetForClamping - this.option.div.offsetLeft, 0); + + const clampedExpandingScrollOffset = Math.min(maxWidthToCounterscroll, elementLeftOffsetFromBorder); + + const clampedScrollLeft = unclampedExpandingScrollOffset // base scroll-coordinate transform mapping based on extra width from element expansion + - clampedExpandingScrollOffset // offset to scroll to put word-start border against the corresponding screen border, fully visible + + srcCounterscrollOverflow; // offset to maintain original overflow past that border if it existed + + // -- Final step: Apply & fine-tune the final scroll positioning -- + this.scrollContainer.scrollLeft = clampedScrollLeft; + + // Prevent "jitters" during counterscroll that occur on expansion / collapse animation. // A one-frame "error correction" effect at the end of animation is far less jarring. if(this.pendingAnimation) { // scrollLeft doesn't work well with fractional values, unlike marginLeft / marginRight - let fractionalOffset = this.scrollContainer.scrollLeft - finalTargetScrollLeft + const fractionalOffset = this.scrollContainer.scrollLeft - clampedScrollLeft; // So we put the fractional difference into marginLeft to force it to sync. this.option.currentWidth += fractionalOffset; } From 6d54e08d7baee8a2fef0da66ce4f4e99a0e1d495 Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Fri, 6 Jan 2023 09:06:40 +0700 Subject: [PATCH 07/60] feat(web): visibility offset cancellation after manual scroll to corresponding area --- .../engine/osk/src/banner/suggestionBanner.ts | 28 ++++++++++++------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/web/src/engine/osk/src/banner/suggestionBanner.ts b/web/src/engine/osk/src/banner/suggestionBanner.ts index 08cb0dc34e..dc5b02261e 100644 --- a/web/src/engine/osk/src/banner/suggestionBanner.ts +++ b/web/src/engine/osk/src/banner/suggestionBanner.ts @@ -33,7 +33,7 @@ export class BannerSuggestion { private _paddingWidth: number; private fontFamily?: string; - private rtl: boolean = false; + public readonly rtl: boolean; private _suggestion: Suggestion; @@ -43,7 +43,7 @@ export class BannerSuggestion { constructor(index: number, isRTL: boolean) { this.index = index; - this.rtl = isRTL; + this.rtl = isRTL ?? false; this.constructRoot(); @@ -524,8 +524,8 @@ class SuggestionExpandContractAnimation { private scrollContainer: HTMLElement | null; private option: BannerSuggestion; - private collapsedScrollLeft: number; - private originalScrollLeft: number; + private collapsedScrollOffset: number; + private rootScrollOffset: number; private startTimestamp: number; private pendingAnimation: number; @@ -535,12 +535,20 @@ class SuggestionExpandContractAnimation { constructor(scrollContainer: HTMLElement, option: BannerSuggestion, forRTL: boolean) { this.scrollContainer = scrollContainer; this.option = option; - this.collapsedScrollLeft = scrollContainer.scrollLeft; - this.originalScrollLeft = scrollContainer.scrollLeft; + this.collapsedScrollOffset = scrollContainer.scrollLeft; + this.rootScrollOffset = scrollContainer.scrollLeft; } public setBaseScroll(val: number) { - this.collapsedScrollLeft = val; + this.collapsedScrollOffset = val; + + // If the user has shifted right to make more of the element visible, we can remove part of the corresponding + // scrolling offset permanently; the user's taken action to view that area. + if(!this.option.rtl) { + if(val < this.rootScrollOffset) { + this.rootScrollOffset = val; + } + } // TODO: else for the RTL adjustment instead. // Attempt to sync the banner-scroller's offset update with that of the // animation for expansion and collapsing. @@ -570,7 +578,7 @@ class SuggestionExpandContractAnimation { const maxWidthToCounterscroll = this.option.currentWidth - this.option.collapsedWidth; // How much space existed to the left of the collapsed option in its original position. May be negative. - const originalCounterscrollBuffer = this.option.div.offsetLeft - this.originalScrollLeft; + const originalCounterscrollBuffer = this.option.div.offsetLeft - this.rootScrollOffset; // TODO: RTL version // Only allow a negative buffer in the final positioning if it already existed. @@ -580,8 +588,8 @@ class SuggestionExpandContractAnimation { // Base position for scrollLeft clamped within std element scroll bounds, including: // - an adjustment to cover the extra width from expansion // - preserving the base expected overflow levels - const unclampedExpandingScrollOffset = Math.max(this.collapsedScrollLeft + maxWidthToCounterscroll, 0) - srcCounterscrollOverflow; - const srcUnclampedExpandingScrollOffset = Math.max(this.originalScrollLeft + maxWidthToCounterscroll, 0) - srcCounterscrollOverflow; + const unclampedExpandingScrollOffset = Math.max(this.collapsedScrollOffset + maxWidthToCounterscroll, 0) - srcCounterscrollOverflow; + const srcUnclampedExpandingScrollOffset = Math.max(this.rootScrollOffset + maxWidthToCounterscroll, 0) - srcCounterscrollOverflow; // TODO: RTL versions / calculations + logic // // Huh - first bugless version didn't actually end up using this. From 40488693e8737115b6ff61c4871828a8ec98f6e9 Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Fri, 6 Jan 2023 11:11:12 +0700 Subject: [PATCH 08/60] feat(web): similar handling for RTL --- .../engine/osk/src/banner/suggestionBanner.ts | 57 +++++++++++-------- 1 file changed, 33 insertions(+), 24 deletions(-) diff --git a/web/src/engine/osk/src/banner/suggestionBanner.ts b/web/src/engine/osk/src/banner/suggestionBanner.ts index dc5b02261e..4c3271a050 100644 --- a/web/src/engine/osk/src/banner/suggestionBanner.ts +++ b/web/src/engine/osk/src/banner/suggestionBanner.ts @@ -208,7 +208,12 @@ export class BannerSuggestion { } else if(val > this.expandedWidth) { val = this.expandedWidth; } - this.container.style.marginLeft = `${val - this.expandedWidth}px`; + + if(this.rtl) { + this.container.style.marginRight = `${val - this.expandedWidth}px`; + } else { + this.container.style.marginLeft = `${val - this.expandedWidth}px`; + } } public isEmpty(): boolean { @@ -542,9 +547,15 @@ class SuggestionExpandContractAnimation { public setBaseScroll(val: number) { this.collapsedScrollOffset = val; - // If the user has shifted right to make more of the element visible, we can remove part of the corresponding - // scrolling offset permanently; the user's taken action to view that area. - if(!this.option.rtl) { + // If the user has shifted the scroll position to make more of the element visible, we can remove part + // of the corresponding scrolling offset permanently; the user's taken action to view that area. + if(this.option.rtl) { + // A higher scrollLeft (scrolling right) will reveal more of an initially-clipped suggestion. + if(val > this.rootScrollOffset) { + this.rootScrollOffset = val; + } + } else { + // Here, a lower scrollLeft (scrolling left). if(val < this.rootScrollOffset) { this.rootScrollOffset = val; } @@ -576,40 +587,38 @@ class SuggestionExpandContractAnimation { // The amount of extra space being taken by a partially or completely expanded suggestion. const maxWidthToCounterscroll = this.option.currentWidth - this.option.collapsedWidth; + const rtl = this.option.rtl; - // How much space existed to the left of the collapsed option in its original position. May be negative. - const originalCounterscrollBuffer = this.option.div.offsetLeft - this.rootScrollOffset; - // TODO: RTL version + // If non-zero, indicates the pixel-width of the collapsed form of the suggestion clipped by the relevant screen border. + const ltrOverflow = Math.max(this.rootScrollOffset - this.option.div.offsetLeft, 0); + const rtlOverflow = Math.max(this.option.div.offsetLeft + this.option.collapsedWidth - (this.rootScrollOffset + this.scrollContainer.offsetWidth)); - // Only allow a negative buffer in the final positioning if it already existed. - // And only as much as originally existed. - const srcCounterscrollOverflow = -Math.min(originalCounterscrollBuffer, 0); // positive offset into overflow-land. + const srcCounterscrollOverflow = Math.max(rtl ? rtlOverflow : ltrOverflow, 0); // positive offset into overflow-land. // Base position for scrollLeft clamped within std element scroll bounds, including: // - an adjustment to cover the extra width from expansion // - preserving the base expected overflow levels - const unclampedExpandingScrollOffset = Math.max(this.collapsedScrollOffset + maxWidthToCounterscroll, 0) - srcCounterscrollOverflow; - const srcUnclampedExpandingScrollOffset = Math.max(this.rootScrollOffset + maxWidthToCounterscroll, 0) - srcCounterscrollOverflow; - // TODO: RTL versions / calculations + logic - - // // Huh - first bugless version didn't actually end up using this. - // const collapsedScrollLeftDelta = this.originalScrollLeft - (unclampedExpandingScrollOffset - maxWidthToCounterscroll + srcCounterscrollOverflow); // neg if touchpoint moving left, - // // pos if touchpoint moving right - // // - scroll moves opposite ("natural") - // // TODO: May need a similar thing for RTL handling. + const unclampedExpandingScrollOffset = Math.max(this.collapsedScrollOffset + (rtl ? 0 : 1) * maxWidthToCounterscroll, 0) + (rtl ? 0 : -1) * srcCounterscrollOverflow; + const srcUnclampedExpandingScrollOffset = Math.max(this.rootScrollOffset + (rtl ? 0 : 1) * maxWidthToCounterscroll, 0) + (rtl ? 0 : -1) * srcCounterscrollOverflow; // Do not shift an element clipped by the screen border further than its original scroll starting point. - const elementLeftOffsetForClamping = Math.min(unclampedExpandingScrollOffset, srcUnclampedExpandingScrollOffset); + const elementOffsetForClamping = rtl + ? Math.max(unclampedExpandingScrollOffset, srcUnclampedExpandingScrollOffset) + : Math.min(unclampedExpandingScrollOffset, srcUnclampedExpandingScrollOffset); // Based on the scroll point selected, determine how far to offset scrolls to keep the option in visible range. // Higher .scrollLeft values make this non-zero and reflect when scroll has begun clipping the element. - const elementLeftOffsetFromBorder = Math.max(elementLeftOffsetForClamping - this.option.div.offsetLeft, 0); + const elementOffsetFromBorder = rtl + // RTL offset: "offsetRight" based on "scrollRight" + ? Math.max(this.option.div.offsetLeft + this.option.currentWidth - (elementOffsetForClamping + this.scrollContainer.offsetWidth), 0) // double-check this one. + // LTR: based on scrollLeft offsetLeft + : Math.max(elementOffsetForClamping - this.option.div.offsetLeft, 0); - const clampedExpandingScrollOffset = Math.min(maxWidthToCounterscroll, elementLeftOffsetFromBorder); + const clampedExpandingScrollOffset = Math.min(maxWidthToCounterscroll, elementOffsetFromBorder); const clampedScrollLeft = unclampedExpandingScrollOffset // base scroll-coordinate transform mapping based on extra width from element expansion - - clampedExpandingScrollOffset // offset to scroll to put word-start border against the corresponding screen border, fully visible - + srcCounterscrollOverflow; // offset to maintain original overflow past that border if it existed + + (rtl ? 1 : -1) * clampedExpandingScrollOffset // offset to scroll to put word-start border against the corresponding screen border, fully visible + + (rtl ? 0 : 1) * srcCounterscrollOverflow; // offset to maintain original overflow past that border if it existed // -- Final step: Apply & fine-tune the final scroll positioning -- this.scrollContainer.scrollLeft = clampedScrollLeft; From f592c5094ddf01ac38af3d4800e5d1d1ed099b6c Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Fri, 6 Jan 2023 13:53:45 +0700 Subject: [PATCH 09/60] feat(web): bonus round - variable-width suggestions --- .../engine/osk/src/banner/suggestionBanner.ts | 46 +++++++++++++++---- 1 file changed, 38 insertions(+), 8 deletions(-) diff --git a/web/src/engine/osk/src/banner/suggestionBanner.ts b/web/src/engine/osk/src/banner/suggestionBanner.ts index 4c3271a050..275bf4aad2 100644 --- a/web/src/engine/osk/src/banner/suggestionBanner.ts +++ b/web/src/engine/osk/src/banner/suggestionBanner.ts @@ -66,10 +66,9 @@ export class BannerSuggestion { container.className = "kmw-suggestion-container"; // Ensures that a reasonable default width, based on % is set. (Since it's not yet in the DOM, we may not yet have actual width info.) - let usableWidth = 100 - SuggestionBanner.MARGIN * (SuggestionBanner.SUGGESTION_LIMIT - 1); + let usableWidth = 100 - SuggestionBanner.MARGIN * (SuggestionBanner.LONG_SUGGESTION_DISPLAY_LIMIT - 1); - // The `/ 2` part: Ensures that the full banner is double-wide, which is useful for demoing scrolling. - let widthpc = usableWidth / (SuggestionBanner.SUGGESTION_LIMIT / 2); + let widthpc = usableWidth / (SuggestionBanner.LONG_SUGGESTION_DISPLAY_LIMIT); container.style.minWidth = widthpc + '%'; div.appendChild(container); @@ -191,7 +190,11 @@ export class BannerSuggestion { } public get collapsedWidth(): number { - let maxWidth = this.targetCollapsedWidth < this.expandedWidth ? this.targetCollapsedWidth : this.expandedWidth; + // Allow shrinking a suggestion's width if it has excess whitespace. + let utilizedWidth = this.spanWidth < this.targetCollapsedWidth ? this.spanWidth : this.targetCollapsedWidth; + // If a minimum width has been specified, enforce that minimum. + let maxWidth = utilizedWidth < this.expandedWidth ? utilizedWidth : this.expandedWidth; + // Will return maxWidth if this.minWidth is undefined. return (this.minWidth > maxWidth ? this.minWidth : maxWidth); @@ -260,7 +263,8 @@ export class BannerSuggestion { * Description Display lexical model suggestions in the banner */ export class SuggestionBanner extends Banner { - public static readonly SUGGESTION_LIMIT: number = 6; + public static readonly SUGGESTION_LIMIT: number = 8; + public static readonly LONG_SUGGESTION_DISPLAY_LIMIT: number = 3; public static readonly MARGIN = 1; public readonly events: EventEmitter; @@ -322,7 +326,7 @@ export class SuggestionBanner extends Banner { * for visuals/UI while still being internally LTR. */ for (var i=0; i i) { const suggestion = suggestions[i]; d.update(suggestion, optionFormat); + if(d.collapsedWidth < d.expandedWidth) { + collapsedOptions.push(d); + } totalWidth += d.collapsedWidth; displayCount++; @@ -500,6 +508,28 @@ export class SuggestionBanner extends Banner { if(totalWidth < this.width) { let separatorWidth = (this.width * 0.01 * (displayCount-1)); + // Prioritize adding padding to suggestions that actually need it. + // Use equal measure for each so long as it still could use extra display space. + while(totalWidth < this.width && collapsedOptions.length > 0) { + let maxFillPadding = (this.width - totalWidth - separatorWidth) / collapsedOptions.length; + collapsedOptions.sort((a, b) => a.expandedWidth - b.expandedWidth); + + let shortestCollapsed = collapsedOptions[0]; + let neededWidth = shortestCollapsed.expandedWidth - shortestCollapsed.collapsedWidth; + + let padding = Math.min(neededWidth, maxFillPadding); + + // Check: it is possible that two elements were matched for equal length, thus the second loop's takes no additional padding. + // No need to trigger re-layout ops for that case. + if(padding > 0) { + collapsedOptions.forEach((a) => a.minWidth = a.collapsedWidth + padding); + totalWidth += padding * collapsedOptions.length; // don't forget to record that we added the padding! + } + + collapsedOptions.splice(0, 1); // discard the element we based our judgment upon; we need not consider it any longer. + } + + // If there's STILL leftover padding to distribute, let's do that now. let fillPadding = (this.width - totalWidth - separatorWidth) / displayCount; for(let i=0; i < displayCount; i++) { From 9f7c6c9754f0f17740c3525e1ccf2de112456e35 Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Fri, 6 Jan 2023 14:37:27 +0700 Subject: [PATCH 10/60] chore(web): ez-pz bits of cleanup --- web/src/engine/osk/src/banner/suggestionBanner.ts | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/web/src/engine/osk/src/banner/suggestionBanner.ts b/web/src/engine/osk/src/banner/suggestionBanner.ts index 275bf4aad2..bb4b6cbcec 100644 --- a/web/src/engine/osk/src/banner/suggestionBanner.ts +++ b/web/src/engine/osk/src/banner/suggestionBanner.ts @@ -137,9 +137,6 @@ export class BannerSuggestion { this.div.style.width=''; } - // TODO: if the option is highlighted, maybe don't disable transitions? - this.container.style.transition = 'none'; // temporarily disable transition effects. - const collapserStyle = this.container.style; collapserStyle.minWidth = this.collapsedWidth + 'px'; @@ -148,11 +145,6 @@ export class BannerSuggestion { } else { collapserStyle.marginLeft = (this.collapsedWidth - this.expandedWidth) + 'px'; } - - this.container.offsetWidth; // To 'flush' the changes before re-enabling transition animations. - this.container.offsetLeft; - - this.container.style.transition = ''; // Re-enable them (it's set on the element's class) } public get targetCollapsedWidth(): number { @@ -297,7 +289,6 @@ export class SuggestionBanner extends Banner { this.container = document.createElement('div'); this.container.className = SuggestionBanner.BANNER_SCROLLER_CLASS; this.getDiv().appendChild(this.container); - // TODO: additional styling for the banner scroll container? this.buildInternals(false); @@ -589,7 +580,7 @@ class SuggestionExpandContractAnimation { if(val < this.rootScrollOffset) { this.rootScrollOffset = val; } - } // TODO: else for the RTL adjustment instead. + } // Attempt to sync the banner-scroller's offset update with that of the // animation for expansion and collapsing. From 1534c6516e817796ca5a66d0e961e4bc931e77da Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Fri, 6 Jan 2023 15:45:39 +0700 Subject: [PATCH 11/60] docs(web): documentation of new internal interface, clarifying renames --- .../engine/osk/src/banner/suggestionBanner.ts | 151 ++++++++++++++---- 1 file changed, 122 insertions(+), 29 deletions(-) diff --git a/web/src/engine/osk/src/banner/suggestionBanner.ts b/web/src/engine/osk/src/banner/suggestionBanner.ts index bb4b6cbcec..b09705609a 100644 --- a/web/src/engine/osk/src/banner/suggestionBanner.ts +++ b/web/src/engine/osk/src/banner/suggestionBanner.ts @@ -13,15 +13,41 @@ import { ParsedLengthStyle } from '../lengthStyle.js'; import { getFontSizeStyle } from '../fontSizeUtils.js'; import { getTextMetrics } from '../keyboard-layout/getTextMetrics.js'; -// TODO: finalize + document -interface OptionFormatSpec { +/** + * Defines various parameters used by `BannerSuggestion` instances for layout and formatting. + * This object is designed first and foremost for use with `BannerSuggestion.update()`. + */ +interface BannerSuggestionFormatSpec { + /** + * Sets a minimum width to use for the `BannerSuggestion`'s element; this overrides any + * and all settings that would otherwise result in a narrower final width. + */ minWidth?: number; - paddingWidth: number, - emSize: number, - styleForFont: CSSStyleDeclaration + /** + * Sets the width of padding around the text of each suggestion. This should generally match + * the 'width' of class = `.kmw-suggest-option::before` and class = `.kmw-suggest-option::after` + * elements as defined in kmwosk.css. + */ + paddingWidth: number, + + /** + * The default font size to use for calculations based on relative font-size specs + */ + emSize: number, + + /** + * The font style (font-size, font-family) to use for suggestion-banner display text. + */ + styleForFont: CSSStyleDeclaration, + + /** + * Sets a target width to use when 'collapsing' suggestions. Only affects those long + * enough to need said 'collapsing'. + */ collapsedWidth?: number } + export class BannerSuggestion { div: HTMLDivElement; container: HTMLDivElement; @@ -104,7 +130,7 @@ export class BannerSuggestion { * @param collapsedTargetWidth * Description Update the ID and text of the BannerSuggestionSpec */ - public update(suggestion: Suggestion, format: OptionFormatSpec) { + public update(suggestion: Suggestion, format: BannerSuggestionFormatSpec) { this._suggestion = suggestion; let display = this.generateSuggestionText(this.rtl); @@ -147,31 +173,55 @@ export class BannerSuggestion { } } + /** + * Denotes the threshold at which the banner suggestion will no longer gain width + * in its default form, resulting in two separate states: "collapsed" and "expanded". + */ public get targetCollapsedWidth(): number { return this._collapsedWidth; } + /** + * The raw width needed to display the suggestion's display text without triggering overflow. + */ public get textWidth(): number { return this._textWidth; } + /** + * Width of the padding to apply equally on both sides of the suggestion's display text. + * Is the sum of both, rather than the value applied to each side. + */ public get paddingWidth(): number { return this._paddingWidth; } + /** + * The absolute minimum width to allow for the represented suggestion's banner element. + */ public get minWidth(): number { return this._minWidth; } + /** + * The absolute minimum width to allow for the represented suggestion's banner element. + */ public set minWidth(val: number) { this._minWidth = val; } + /** + * The total width taken by the suggestion's banner element when fully expanded. + * This may equal the `collapsed` width for sufficiently short suggestions. + */ public get expandedWidth(): number { // minWidth must be defined AND greater for the conditional to return this.minWidth. return this.minWidth > this.spanWidth ? this.minWidth : this.spanWidth; } + /** + * The total width used by the internal contents of the suggestion's banner element when not obscured. + */ public get spanWidth(): number { let spanWidth = this.textWidth ?? 0; if(spanWidth) { @@ -181,21 +231,32 @@ export class BannerSuggestion { return spanWidth; } + /** + * The actual width to be used for the `BannerSuggestion`'s display element when in the 'collapsed' + * state and not transitioning. + */ public get collapsedWidth(): number { // Allow shrinking a suggestion's width if it has excess whitespace. let utilizedWidth = this.spanWidth < this.targetCollapsedWidth ? this.spanWidth : this.targetCollapsedWidth; // If a minimum width has been specified, enforce that minimum. let maxWidth = utilizedWidth < this.expandedWidth ? utilizedWidth : this.expandedWidth; - // Will return maxWidth if this.minWidth is undefined. return (this.minWidth > maxWidth ? this.minWidth : maxWidth); } + /** + * The actual width currently utilized by the `BannerSuggestion`'s display element, regardless of + * current state. + */ public get currentWidth(): number { return this.div.offsetWidth; } + /** + * The actual width currently utilized by the `BannerSuggestion`'s display element, regardless of + * current state. + */ public set currentWidth(val: number) { // TODO: probably should set up errors or something here... if(val < this.collapsedWidth) { @@ -451,6 +512,11 @@ export class SuggestionBanner extends Banner { } } + /** + * Produces a closure useful for updating the SuggestionBanner's UI to match newly-received + * suggestions, including optimization of the banner's layout. + * @param suggestions + */ public onSuggestionUpdate = (suggestions: Suggestion[]): void => { this.currentSuggestions = suggestions; @@ -464,7 +530,7 @@ export class SuggestionBanner extends Banner { const textLeftPad = new ParsedLengthStyle(textStyle.paddingLeft || '2px'); // computedStyle will fail if the element's not in the DOM yet. const textRightPad = new ParsedLengthStyle(textStyle.paddingRight || '2px'); - let optionFormat: OptionFormatSpec = { + let optionFormat: BannerSuggestionFormatSpec = { paddingWidth: textLeftPad.val + textRightPad.val, // Assumes fixed px padding. emSize: emSize, styleForFont: fontStyle, @@ -582,29 +648,51 @@ class SuggestionExpandContractAnimation { } } - // Attempt to sync the banner-scroller's offset update with that of the + // Synchronize the banner-scroller's offset update with that of the // animation for expansion and collapsing. - window.requestAnimationFrame(this.setOffsetScroll); - - // this.setOffsetScroll(); + window.requestAnimationFrame(this.setScrollOffset); } - // the "fun", top-level banner part. - private setOffsetScroll = () => { + /** + * Performs mapping of the user's touchpoint to properly-offset scroll coordinates based on + * the state of the ongoing scroll operation. + * + * First priority: this function aims to keep all currently-visible parts of a selected + * suggestion visible when first selected. Any currently-clipped parts will remain clipped. + * + * Second priority: all animations should be smooth and continuous; aesthetics do matter to + * users. + * + * Third priority: when possible without violating the first two priorities, this (in tandem with + * adjustments within `setBaseScroll`) will aim to sync the touchpoint with its original + * location on an expanded suggestion. + * - For LTR languages, this means that suggestions will "expand left" if possible. + * - While for RTL languages, they will "expand right" if possible. + * - However, if they would expand outside of the banner's effective viewport, a scroll offset + * will kick in to enforce the "first priority" mentioned above. + * - This "scroll offset" will be progressively removed (because second priority) if and as + * the user manually scrolls to reveal relevant space that was originally outside of the viewport. + * + * @returns + */ + private setScrollOffset = () => { // If we've been 'decoupled', a different instance (likely for a different suggestion) // is responsible for counter-scrolling. if(!this.scrollContainer) { return; } - // -- Clamping logic -- + // -- Clamping / "scroll offset" logic -- - // As currently written / defined below, "clamping" refers to alterations to scroll-positioned mapping designed - // to keep as much of the expanded option visible as possible via the offsets below while not pushing - // already-obscured parts of the expanded option into visible range. + // As currently written / defined below, and used internally within this function, "clamping" + // refers to alterations to scroll-positioned mapping designed to keep as much of the expanded + // option visible as possible via the offsets below (that is, "clamped" to the relevant border) + // while not adding extra discontinuity by pushing already-obscured parts of the expanded option + // into visible range. // - // In essence, it's an extra offset we apply that is dynamically adjusted depending on scroll position as it changes. - // This offset may be decreased when it is no longer needed to make parts of the element visible. + // In essence, it's an extra "scroll offset" we apply that is dynamically adjusted depending on + // scroll position as it changes. This offset may be decreased when it is no longer needed to + // make parts of the element visible. // The amount of extra space being taken by a partially or completely expanded suggestion. const maxWidthToCounterscroll = this.option.currentWidth - this.option.collapsedWidth; @@ -619,13 +707,15 @@ class SuggestionExpandContractAnimation { // Base position for scrollLeft clamped within std element scroll bounds, including: // - an adjustment to cover the extra width from expansion // - preserving the base expected overflow levels + // Does NOT make adjustments to force extra visibility on the element being highlighted/focused. const unclampedExpandingScrollOffset = Math.max(this.collapsedScrollOffset + (rtl ? 0 : 1) * maxWidthToCounterscroll, 0) + (rtl ? 0 : -1) * srcCounterscrollOverflow; - const srcUnclampedExpandingScrollOffset = Math.max(this.rootScrollOffset + (rtl ? 0 : 1) * maxWidthToCounterscroll, 0) + (rtl ? 0 : -1) * srcCounterscrollOverflow; + // The same, but for our 'root scroll coordinate'. + const rootUnclampedExpandingScrollOffset = Math.max(this.rootScrollOffset + (rtl ? 0 : 1) * maxWidthToCounterscroll, 0) + (rtl ? 0 : -1) * srcCounterscrollOverflow; // Do not shift an element clipped by the screen border further than its original scroll starting point. const elementOffsetForClamping = rtl - ? Math.max(unclampedExpandingScrollOffset, srcUnclampedExpandingScrollOffset) - : Math.min(unclampedExpandingScrollOffset, srcUnclampedExpandingScrollOffset); + ? Math.max(unclampedExpandingScrollOffset, rootUnclampedExpandingScrollOffset) + : Math.min(unclampedExpandingScrollOffset, rootUnclampedExpandingScrollOffset); // Based on the scroll point selected, determine how far to offset scrolls to keep the option in visible range. // Higher .scrollLeft values make this non-zero and reflect when scroll has begun clipping the element. @@ -635,20 +725,23 @@ class SuggestionExpandContractAnimation { // LTR: based on scrollLeft offsetLeft : Math.max(elementOffsetForClamping - this.option.div.offsetLeft, 0); + // If the element is close enough to the border, don't offset beyond the element! + // If it is further, do not add excess padding - it'd effectively break scrolling. + // Do maintain any remaining scroll offset that exists, though. const clampedExpandingScrollOffset = Math.min(maxWidthToCounterscroll, elementOffsetFromBorder); - const clampedScrollLeft = unclampedExpandingScrollOffset // base scroll-coordinate transform mapping based on extra width from element expansion + const finalScrollOffset = unclampedExpandingScrollOffset // base scroll-coordinate transform mapping based on extra width from element expansion + (rtl ? 1 : -1) * clampedExpandingScrollOffset // offset to scroll to put word-start border against the corresponding screen border, fully visible + (rtl ? 0 : 1) * srcCounterscrollOverflow; // offset to maintain original overflow past that border if it existed // -- Final step: Apply & fine-tune the final scroll positioning -- - this.scrollContainer.scrollLeft = clampedScrollLeft; + this.scrollContainer.scrollLeft = finalScrollOffset; - // Prevent "jitters" during counterscroll that occur on expansion / collapse animation. + // Prevent "jitters" during counterscroll that occur on expansion / collapse animation. // A one-frame "error correction" effect at the end of animation is far less jarring. if(this.pendingAnimation) { // scrollLeft doesn't work well with fractional values, unlike marginLeft / marginRight - const fractionalOffset = this.scrollContainer.scrollLeft - clampedScrollLeft; + const fractionalOffset = this.scrollContainer.scrollLeft - finalScrollOffset; // So we put the fractional difference into marginLeft to force it to sync. this.option.currentWidth += fractionalOffset; } @@ -712,7 +805,7 @@ class SuggestionExpandContractAnimation { // Part 3: perform any needed counter-scrolling, scroll clamping, etc // Existence of a followup animation frame is part of the logic, so keep this 'after'! - this.setOffsetScroll(); + this.setScrollOffset(); }; public collapse() { @@ -762,7 +855,7 @@ class SuggestionExpandContractAnimation { // Part 3: perform any needed counter-scrolling, scroll clamping, etc // Existence of a followup animation frame is part of the logic, so keep this 'after'! - this.setOffsetScroll(); + this.setScrollOffset(); }; } From 0ab094df439e98e3eeba7923ba546bc04aef1ca3 Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Wed, 15 Nov 2023 11:38:57 +0700 Subject: [PATCH 12/60] docs(web): adds comment --- web/src/engine/osk/src/banner/suggestionBanner.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/web/src/engine/osk/src/banner/suggestionBanner.ts b/web/src/engine/osk/src/banner/suggestionBanner.ts index b09705609a..fdbc8488bb 100644 --- a/web/src/engine/osk/src/banner/suggestionBanner.ts +++ b/web/src/engine/osk/src/banner/suggestionBanner.ts @@ -527,8 +527,11 @@ export class SuggestionBanner extends Banner { const textStyle = getComputedStyle(this.options[0].container.firstChild as HTMLSpanElement); const targetWidth = this.width / SuggestionBanner.LONG_SUGGESTION_DISPLAY_LIMIT; - const textLeftPad = new ParsedLengthStyle(textStyle.paddingLeft || '2px'); // computedStyle will fail if the element's not in the DOM yet. - const textRightPad = new ParsedLengthStyle(textStyle.paddingRight || '2px'); + + // computedStyle will fail if the element's not in the DOM yet. + // Seeks to get the values specified within kmwosk.css. + const textLeftPad = new ParsedLengthStyle(textStyle.paddingLeft || '4px'); + const textRightPad = new ParsedLengthStyle(textStyle.paddingRight || '4px'); let optionFormat: BannerSuggestionFormatSpec = { paddingWidth: textLeftPad.val + textRightPad.val, // Assumes fixed px padding. From 5fd0cebdcb2dc6bf70fc0a8c229e97c9ce055f68 Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Fri, 1 Dec 2023 09:42:52 +0700 Subject: [PATCH 13/60] fix(web): z-index issues - suggestions were above keytips and subkeys --- web/src/resources/osk/kmwosk.css | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/web/src/resources/osk/kmwosk.css b/web/src/resources/osk/kmwosk.css index f45660b958..79c9865a97 100644 --- a/web/src/resources/osk/kmwosk.css +++ b/web/src/resources/osk/kmwosk.css @@ -383,7 +383,7 @@ height: 100%; content: ''; top: 0; - z-index:10999; /* z-indexes this _behind_ the 'option' element that hosts the scrollable zone. */ + z-index:10000; /* z-indexes this _behind_ the 'option' element that hosts the scrollable zone. */ user-select: none; pointer-events: none; /* Ensures click-through! But apparently not touch-through. */ touch-action: none; /* Doesn't seem to allow touch-through, though - even with touch-action: none */ @@ -417,7 +417,7 @@ } .kmw-banner-bar .kmw-banner-separator {border-left: solid 1px #8a8d90; width: 0px; vertical-align: middle; height: 45%; display: inline-block;} -.kmw-banner-bar .kmw-suggest-option {display:inline-block; text-align: center; height: 85%; position: relative; z-index: 11000} +.kmw-banner-bar .kmw-suggest-option {display:inline-block; text-align: center; height: 85%; position: relative; z-index: 10001} .kmw-suggestion-text { color:#fff; line-height: normal; From d720bd9ade39420c09f5e9b13c10629078933369 Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Fri, 1 Dec 2023 10:59:45 +0700 Subject: [PATCH 14/60] fix(web): font style retrieval for suggestion-width calcs --- .../engine/osk/src/banner/suggestionBanner.ts | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/web/src/engine/osk/src/banner/suggestionBanner.ts b/web/src/engine/osk/src/banner/suggestionBanner.ts index fdbc8488bb..94419f444c 100644 --- a/web/src/engine/osk/src/banner/suggestionBanner.ts +++ b/web/src/engine/osk/src/banner/suggestionBanner.ts @@ -39,7 +39,10 @@ interface BannerSuggestionFormatSpec { /** * The font style (font-size, font-family) to use for suggestion-banner display text. */ - styleForFont: CSSStyleDeclaration, + styleForFont: { + fontSize: typeof CSSStyleDeclaration.prototype.fontSize, + fontFamily: typeof CSSStyleDeclaration.prototype.fontFamily + }, /** * Sets a target width to use when 'collapsing' suggestions. Only affects those long @@ -80,6 +83,10 @@ export class BannerSuggestion { this.container.appendChild(display); } + get computedStyle() { + return getComputedStyle(this.display); + } + private constructRoot() { // Add OSK suggestion labels let div = this.div = createUnselectableElement('div'), ds=div.style; @@ -520,7 +527,13 @@ export class SuggestionBanner extends Banner { public onSuggestionUpdate = (suggestions: Suggestion[]): void => { this.currentSuggestions = suggestions; - const fontStyle = getComputedStyle(this.options[0].div); + const fontStyleBase = this.options[0].computedStyle; + // Do NOT just re-use the returned object from the line above; it may spontaneously change + // (in a bad way) when the underlying span is replaced! + const fontStyle = { + fontSize: fontStyleBase.fontSize, + fontFamily: fontStyleBase.fontFamily + } const emSizeStr = getComputedStyle(document.body).fontSize; const emSize = getFontSizeStyle(emSizeStr).val; From 121e24a81e3ca5e07112cbaa8a2cf56e13fb06df Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Fri, 1 Dec 2023 13:24:04 +0700 Subject: [PATCH 15/60] fix(web): at min, mitigates non-collapsing option issue --- web/src/engine/osk/src/banner/suggestionBanner.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/web/src/engine/osk/src/banner/suggestionBanner.ts b/web/src/engine/osk/src/banner/suggestionBanner.ts index 94419f444c..aedd76e4a3 100644 --- a/web/src/engine/osk/src/banner/suggestionBanner.ts +++ b/web/src/engine/osk/src/banner/suggestionBanner.ts @@ -159,6 +159,7 @@ export class BannerSuggestion { this._textWidth = 0; } + this.currentWidth = this.collapsedWidth; this.updateLayout(); } @@ -427,6 +428,7 @@ export class SuggestionBanner extends Banner { if(on && classes.indexOf(cs) < 0) { elem.className=classes+cs; if(this.highlightAnimation) { + this.highlightAnimation.cancel(); this.highlightAnimation.decouple(); } @@ -526,6 +528,8 @@ export class SuggestionBanner extends Banner { */ public onSuggestionUpdate = (suggestions: Suggestion[]): void => { this.currentSuggestions = suggestions; + // Immediately stop all animations and reset options accordingly. + this.highlightAnimation?.cancel(); const fontStyleBase = this.options[0].computedStyle; // Do NOT just re-use the returned object from the line above; it may spontaneously change @@ -764,6 +768,7 @@ class SuggestionExpandContractAnimation { } public decouple() { + this.cancel(); this.scrollContainer = null; } @@ -773,6 +778,11 @@ class SuggestionExpandContractAnimation { this.pendingAnimation = null; } + cancel() { + this.clear(); + this.option.currentWidth = this.option.collapsedWidth; + } + public expand() { // Cancel any prior iterating animation-frame commands. this.clear(); From e5e04f1de2b29078d1993165226a1a51ef28b0b7 Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Mon, 4 Dec 2023 11:01:25 +0700 Subject: [PATCH 16/60] feat(web): restores scroll-state tracker --- .../osk/src/banner/bannerScrollState.ts | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 web/src/engine/osk/src/banner/bannerScrollState.ts diff --git a/web/src/engine/osk/src/banner/bannerScrollState.ts b/web/src/engine/osk/src/banner/bannerScrollState.ts new file mode 100644 index 0000000000..c819f77bb2 --- /dev/null +++ b/web/src/engine/osk/src/banner/bannerScrollState.ts @@ -0,0 +1,50 @@ +import { InputSample } from "@keymanapp/gesture-recognizer"; + +/** + * The amount of coordinate 'noise' allowed during a scroll-enabled touch allowed + * before interpreting the currently-ongoing touch command as having scrolled. + */ +const HAS_SCROLLED_FUDGE_FACTOR = 10; + +/** + * This class was added to facilitate scroll handling for overflow-x elements, though it could + * be extended in the future to accept overflow-y if needed. + * + * This is necessary because of the OSK's need to use `.preventDefault()` for stability; that + * same method blocks native handling of overflow scrolling for touch browsers. + */ +export class BannerScrollState { + totalLength = 0; + + baseCoord: InputSample; + curCoord: InputSample; + baseScrollLeft: number; + + // The amount of coordinate 'noise' allowed during a scroll-enabled touch allowed + // before interpreting the currently-ongoing touch command as having scrolled. + static readonly HAS_SCROLLED_FUDGE_FACTOR = 10; + + constructor(coord: InputSample, baseScrollLeft: number) { + this.baseCoord = coord; + this.curCoord = coord; + this.baseScrollLeft = baseScrollLeft; + + this.totalLength = 0; + } + + updateTo(coord: InputSample): number { + let prevCoord = this.curCoord; + this.curCoord = coord; + + let delta = this.baseCoord.targetX - this.curCoord.targetX + this.baseScrollLeft + // Track the total amount of scrolling used, even if just a pixel-wide back and forth wiggle. + this.totalLength += Math.abs(this.curCoord.targetX - prevCoord.targetX); + + return delta; + } + + public get hasScrolled(): boolean { + // Allow an accidental fudge-factor for overflow element noise during a touch, but not much. + return this.totalLength > HAS_SCROLLED_FUDGE_FACTOR; + } +} \ No newline at end of file From 8465f1f3e236e42f3cd8006c4d9db81fe7791dce Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Mon, 4 Dec 2023 12:46:52 +0700 Subject: [PATCH 17/60] chore(web): adjusts pre-gesture banner code to post-gesture --- .../osk/src/banner/bannerScrollState.ts | 2 +- .../engine/osk/src/banner/suggestionBanner.ts | 119 ++++++++++++------ 2 files changed, 85 insertions(+), 36 deletions(-) diff --git a/web/src/engine/osk/src/banner/bannerScrollState.ts b/web/src/engine/osk/src/banner/bannerScrollState.ts index c819f77bb2..bef4ba7d74 100644 --- a/web/src/engine/osk/src/banner/bannerScrollState.ts +++ b/web/src/engine/osk/src/banner/bannerScrollState.ts @@ -36,7 +36,7 @@ export class BannerScrollState { let prevCoord = this.curCoord; this.curCoord = coord; - let delta = this.baseCoord.targetX - this.curCoord.targetX + this.baseScrollLeft + let delta = this.baseCoord.targetX - this.curCoord.targetX + this.baseScrollLeft; // Track the total amount of scrolling used, even if just a pixel-wide back and forth wiggle. this.totalLength += Math.abs(this.curCoord.targetX - prevCoord.targetX); diff --git a/web/src/engine/osk/src/banner/suggestionBanner.ts b/web/src/engine/osk/src/banner/suggestionBanner.ts index 63fa765337..80b0f20ff3 100644 --- a/web/src/engine/osk/src/banner/suggestionBanner.ts +++ b/web/src/engine/osk/src/banner/suggestionBanner.ts @@ -7,7 +7,8 @@ import { GestureRecognizerConfiguration, GestureSource, InputSample, - PaddedZoneSource + PaddedZoneSource, + RecognitionZoneSource } from '@keymanapp/gesture-recognizer'; import { BANNER_GESTURE_SET } from './bannerGestureSet.js'; @@ -18,12 +19,14 @@ import EventEmitter from 'eventemitter3'; import { ParsedLengthStyle } from '../lengthStyle.js'; import { getFontSizeStyle } from '../fontSizeUtils.js'; import { getTextMetrics } from '../keyboard-layout/getTextMetrics.js'; - +import { BannerScrollState } from './bannerScrollState.js'; const TOUCHED_CLASS: string = 'kmw-suggest-touched'; const BANNER_CLASS: string = 'kmw-suggest-banner'; const BANNER_SCROLLER_CLASS = 'kmw-suggest-banner-scroller'; +const BANNER_VERT_ROAMING_HEIGHT_RATIO = 0.666; + /** * Defines various parameters used by `BannerSuggestion` instances for layout and formatting. * This object is designed first and foremost for use with `BannerSuggestion.update()`. @@ -293,13 +296,11 @@ export class BannerSuggestion { public highlight(on: boolean) { const elem = this.div; - let classes = elem.className; - let cs = ' ' + TOUCHED_CLASS; - if(on && classes.indexOf(cs) < 0) { - elem.className=classes+cs; + if(on) { + elem.classList.add(TOUCHED_CLASS); } else { - elem.className=classes.replace(cs,''); + elem.classList.remove(TOUCHED_CLASS); } } @@ -362,10 +363,15 @@ export class SuggestionBanner extends Banner { private hostDevice: DeviceSpec; + /** + * The banner 'container', which is also the root element for banner scrolling. + */ private readonly container: HTMLElement; private highlightAnimation: SuggestionExpandContractAnimation; private gestureEngine: GestureRecognizer; + private scrollState: BannerScrollState; + private selectionBounds: RecognitionZoneSource; private _predictionContext: PredictionContext; @@ -443,11 +449,32 @@ export class SuggestionBanner extends Banner { return null; } + // Auto-cancels suggestion-selection if the finger moves too far; having very generous + // safe-zone settings also helps keep scrolls active on demo pages, etc. + const safeBounds = new PaddedZoneSource(this.getDiv(), [-Number.MAX_SAFE_INTEGER]); + this.selectionBounds = new PaddedZoneSource( + this.getDiv(), + [-BANNER_VERT_ROAMING_HEIGHT_RATIO * this.height, -Number.MAX_SAFE_INTEGER] + ); + const config: GestureRecognizerConfiguration = { targetRoot: this.getDiv(), - maxRoamingBounds: new PaddedZoneSource(this.getDiv(), [-0.333 * this.height]), + maxRoamingBounds: safeBounds, + safeBounds: safeBounds, // touchEventRoot: this.element, // is the default itemIdentifier: (sample, target: HTMLElement) => { + const selBounds = this.selectionBounds.getBoundingClientRect(); + + // Step 1: is the coordinate within the range we permit for selecting _anything_? + if(sample.clientX < selBounds.left || sample.clientX > selBounds.right) { + return null; + } + if(sample.clientY < selBounds.top || sample.clientY > selBounds.bottom) { + return null; + } + + // Step 2: find the best-matching selection. + let bestMatch: BannerSuggestion = null; let bestDist = Number.MAX_VALUE; @@ -482,6 +509,25 @@ export class SuggestionBanner extends Banner { suggestion: null }; + const markSelection = (suggestion: BannerSuggestion) => { + suggestion.highlight(true); + if(this.highlightAnimation) { + this.highlightAnimation.cancel(); + this.highlightAnimation.decouple(); + } + + this.highlightAnimation = new SuggestionExpandContractAnimation(this.container, suggestion, false); + this.highlightAnimation.expand(); + } + + const clearSelection = (suggestion: BannerSuggestion) => { + suggestion.highlight(false); + if(!this.highlightAnimation) { + this.highlightAnimation = new SuggestionExpandContractAnimation(this.container, suggestion, false); + } + this.highlightAnimation.collapse(); + } + engine.on('inputstart', (source) => { // The banner does not support multi-touch - if one is still current, block all others. if(sourceTracker.source) { @@ -489,46 +535,45 @@ export class SuggestionBanner extends Banner { return; } + this.scrollState = new BannerScrollState(source.currentSample, this.container.scrollLeft); + const suggestion = source.baseItem; + sourceTracker.source = source; sourceTracker.scrollingHandler = (sample) => { - // Maintain highlighting - const suggestion = sample.item; + const newScrollLeft = this.scrollState.updateTo(sample); + this.highlightAnimation.setBaseScroll(newScrollLeft); - if(suggestion != sourceTracker.suggestion) { - sourceTracker.suggestion?.highlight(false); - sourceTracker.suggestion?.div.classList.remove(TOUCHED_CLASS); - suggestion.highlight(true); + // Only re-enable the original suggestion, even if the touchpoint finds + // itself over a different suggestion. Might happen if a scroll boundary + // is reached. + const incoming = sample.item ? suggestion : null; - const elem = suggestion.div; - if(!elem.classList.contains(TOUCHED_CLASS)) { - elem.classList.add(TOUCHED_CLASS); - if(this.highlightAnimation) { - this.highlightAnimation.cancel(); - this.highlightAnimation.decouple(); - } - - this.highlightAnimation = new SuggestionExpandContractAnimation(this.container, suggestion, false); - this.highlightAnimation.expand(); - } else { - elem.classList.remove(TOUCHED_CLASS); - if(!this.highlightAnimation) { - this.highlightAnimation = new SuggestionExpandContractAnimation(this.container, suggestion, false); - } - this.highlightAnimation.collapse(); + // It's possible to cancel selection while still scrolling. + if(incoming != sourceTracker.suggestion) { + if(sourceTracker.suggestion) { + clearSelection(sourceTracker.suggestion); } - sourceTracker.suggestion = suggestion; + sourceTracker.suggestion = incoming; + if(incoming) { + markSelection(incoming); + } } }; + sourceTracker.suggestion = source.currentSample.item; + markSelection(sourceTracker.suggestion); source.currentSample.item.highlight(true); const terminationHandler = () => { - sourceTracker.suggestion.highlight(false); + if(sourceTracker.suggestion) { + clearSelection(sourceTracker.suggestion); + sourceTracker.suggestion = null; + } + sourceTracker.source = null; sourceTracker.scrollingHandler = null; - sourceTracker.suggestion = null; } source.path.on('complete', terminationHandler); @@ -540,9 +585,11 @@ export class SuggestionBanner extends Banner { // The actual result comes in via the sequence's `stage` event. sequence.once('stage', (result) => { const suggestion = result.item; // Should also == sourceTracker.suggestion. - if(suggestion) { + if(suggestion && !this.scrollState.hasScrolled) { this.predictionContext.accept(suggestion.suggestion); } + + this.scrollState = null; }); }); @@ -555,7 +602,9 @@ export class SuggestionBanner extends Banner { // Ensure the banner's extended recognition zone is based on proper, up-to-date layout info. // Note: during banner init, `this.gestureEngine` may only be defined after // the first call to this setter! - (this.gestureEngine?.config.maxRoamingBounds as PaddedZoneSource)?.updatePadding([-0.333 * this.height]); + (this.selectionBounds as PaddedZoneSource)?.updatePadding( + [-BANNER_VERT_ROAMING_HEIGHT_RATIO * this.height, -Number.MAX_SAFE_INTEGER] + ); return result; } From 33ffad05ee3fdb3ca08d6d7e4e31bc4c613b4060 Mon Sep 17 00:00:00 2001 From: jahorton Date: Tue, 9 Jan 2024 13:46:03 +0700 Subject: [PATCH 18/60] fix(ios): banner image management --- .../Keyman.bundle/Contents/Resources/ios-host.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/ios/engine/KMEI/KeymanEngine/resources/Keyman.bundle/Contents/Resources/ios-host.js b/ios/engine/KMEI/KeymanEngine/resources/Keyman.bundle/Contents/Resources/ios-host.js index 3717b5e648..1b3effa9ee 100644 --- a/ios/engine/KMEI/KeymanEngine/resources/Keyman.bundle/Contents/Resources/ios-host.js +++ b/ios/engine/KMEI/KeymanEngine/resources/Keyman.bundle/Contents/Resources/ios-host.js @@ -81,6 +81,14 @@ function showBanner(flag) { function setBannerImage(path) { bannerImgPath = path; + + var bc = keyman && keyman.osk && keyman.osk.bannerController; + if(!bc) { + return; + } + + // If an inactive banner is set, update its image. + bc.inactiveBanner = bc.inactiveBanner ? new bc.ImageBanner(bannerImgPath) : null; } function setBannerHeight(h) { From a6c23d6a3b4e102bff37123f562f001a16e2c9f4 Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Wed, 10 Jan 2024 15:09:30 +0700 Subject: [PATCH 19/60] fix(web): cancels active gestures on globe-key use --- web/src/engine/osk/src/visualKeyboard.ts | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/web/src/engine/osk/src/visualKeyboard.ts b/web/src/engine/osk/src/visualKeyboard.ts index 79563beb75..41617c5ee0 100644 --- a/web/src/engine/osk/src/visualKeyboard.ts +++ b/web/src/engine/osk/src/visualKeyboard.ts @@ -612,8 +612,23 @@ export default class VisualKeyboard extends EventEmitter implements Ke // Merely constructing the instance is enough; it'll link into the sequence's events and // handle everything that remains for the backspace from here. handlers = [new HeldRepeater(gestureSequence, () => this.modelKeyClick(gestureKey, coord))]; - } else if(gestureKey.key.spec.baseKeyID == "K_LOPT") { + } else if(gestureKey.key.spec.baseKeyID == "K_LOPT") { // globe key gestureSequence.on('complete', () => this.emit('globekey', gestureKey, false)); + + for(const identifier of Object.keys(sourceTrackingMap)) { + if(identifier == coordSource.identifier) { + // No need to cancel the current gesture; let the globe-key gesture complete. + continue; + } + + // Any _other_ gesture, though - yeah, that should cancel out. + // Might be a _bit_ funky if there's an active modipress, but only momentarily. + const entry = sourceTrackingMap[identifier]; + + // Trigger cancellation of all other pending gestures - they're not valid after a keyboard-swap. + entry.source.terminate(true); + } + } } else if(gestureStage.matchedId.indexOf('longpress') > -1) { existingPreviewHost?.cancel(); From 5d509a77174c8c8552c5656ef5f1e2dbdda98240 Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Thu, 11 Jan 2024 08:09:26 +0700 Subject: [PATCH 20/60] fix(web): gesture-recognizer now clears gesture-tracking when 'shut down' --- .../web/gesture-recognizer/src/engine/gestureRecognizer.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/common/web/gesture-recognizer/src/engine/gestureRecognizer.ts b/common/web/gesture-recognizer/src/engine/gestureRecognizer.ts index 86fd645d0a..d172255122 100644 --- a/common/web/gesture-recognizer/src/engine/gestureRecognizer.ts +++ b/common/web/gesture-recognizer/src/engine/gestureRecognizer.ts @@ -33,6 +33,11 @@ export class GestureRecognizer extends Touchp } public destroy() { + // When shutting down the gesture engine, we should go ahead and clear out all related + // gesture-source tracking. + this.activeGestures.forEach((sequence) => sequence.cancel()); + this.activeSources.forEach((source) => source.terminate(true)); + this.mouseEngine.unregisterEventHandlers(); this.touchEngine.unregisterEventHandlers(); From 8534a1fc7404d2937ce68fc44bbf74516bc0bf7d Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Thu, 11 Jan 2024 08:14:39 +0700 Subject: [PATCH 21/60] change(web): puts new code in self-contained func --- web/src/engine/osk/src/visualKeyboard.ts | 32 +++++++++++++----------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/web/src/engine/osk/src/visualKeyboard.ts b/web/src/engine/osk/src/visualKeyboard.ts index 41617c5ee0..2aefc82d2b 100644 --- a/web/src/engine/osk/src/visualKeyboard.ts +++ b/web/src/engine/osk/src/visualKeyboard.ts @@ -407,6 +407,22 @@ export default class VisualKeyboard extends EventEmitter implements Ke previewHost: GesturePreviewHost }> = {}; + const clearActiveGestures = (exception?: string) => { + for(const identifier of Object.keys(sourceTrackingMap)) { + if(identifier == exception) { + // No need to cancel the current gesture; let the globe-key gesture complete. + continue; + } + + // Any _other_ gesture, though - yeah, that should cancel out. + // Might be a _bit_ funky if there's an active modipress, but only momentarily. + const entry = sourceTrackingMap[identifier]; + + // Trigger cancellation of all other pending gestures - they're not valid after a keyboard-swap. + entry.source.terminate(true); + } + } + const gestureHandlerMap = new Map, GestureHandler[]>(); // Now to set up event-handling links. @@ -614,21 +630,7 @@ export default class VisualKeyboard extends EventEmitter implements Ke handlers = [new HeldRepeater(gestureSequence, () => this.modelKeyClick(gestureKey, coord))]; } else if(gestureKey.key.spec.baseKeyID == "K_LOPT") { // globe key gestureSequence.on('complete', () => this.emit('globekey', gestureKey, false)); - - for(const identifier of Object.keys(sourceTrackingMap)) { - if(identifier == coordSource.identifier) { - // No need to cancel the current gesture; let the globe-key gesture complete. - continue; - } - - // Any _other_ gesture, though - yeah, that should cancel out. - // Might be a _bit_ funky if there's an active modipress, but only momentarily. - const entry = sourceTrackingMap[identifier]; - - // Trigger cancellation of all other pending gestures - they're not valid after a keyboard-swap. - entry.source.terminate(true); - } - + clearActiveGestures(coordSource.identifier); } } else if(gestureStage.matchedId.indexOf('longpress') > -1) { existingPreviewHost?.cancel(); From 97fab6fc3069889b182dbbb432573954ed967646 Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Thu, 11 Jan 2024 09:14:20 +0700 Subject: [PATCH 22/60] chore(web): tweaks per review --- web/src/engine/osk/src/visualKeyboard.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/web/src/engine/osk/src/visualKeyboard.ts b/web/src/engine/osk/src/visualKeyboard.ts index 2aefc82d2b..2132cdca1a 100644 --- a/web/src/engine/osk/src/visualKeyboard.ts +++ b/web/src/engine/osk/src/visualKeyboard.ts @@ -407,18 +407,16 @@ export default class VisualKeyboard extends EventEmitter implements Ke previewHost: GesturePreviewHost }> = {}; - const clearActiveGestures = (exception?: string) => { + const clearActiveGestures = (excludedTouchpointId?: string) => { for(const identifier of Object.keys(sourceTrackingMap)) { - if(identifier == exception) { - // No need to cancel the current gesture; let the globe-key gesture complete. + // Filter out the exclusion if one exists. + if(identifier == excludedTouchpointId) { continue; } // Any _other_ gesture, though - yeah, that should cancel out. - // Might be a _bit_ funky if there's an active modipress, but only momentarily. + // Note: this can cancel ongoing modipress gestures, which may trigger an unexpected layer shift. const entry = sourceTrackingMap[identifier]; - - // Trigger cancellation of all other pending gestures - they're not valid after a keyboard-swap. entry.source.terminate(true); } } @@ -630,6 +628,8 @@ export default class VisualKeyboard extends EventEmitter implements Ke handlers = [new HeldRepeater(gestureSequence, () => this.modelKeyClick(gestureKey, coord))]; } else if(gestureKey.key.spec.baseKeyID == "K_LOPT") { // globe key gestureSequence.on('complete', () => this.emit('globekey', gestureKey, false)); + // Cancel all other gesture sources; a language-menu interaction voids all previously-active + // gestures that haven't completed. clearActiveGestures(coordSource.identifier); } } else if(gestureStage.matchedId.indexOf('longpress') > -1) { From cf0b81ae3b4ef4f215e37621bde7293cb3ffefaa Mon Sep 17 00:00:00 2001 From: jahorton Date: Thu, 11 Jan 2024 11:33:24 +0700 Subject: [PATCH 23/60] fix(ios): prevent multiple kbd slide-in animations on app startt --- ios/engine/KMEI/KeymanEngine/Classes/TextView.swift | 5 ----- 1 file changed, 5 deletions(-) diff --git a/ios/engine/KMEI/KeymanEngine/Classes/TextView.swift b/ios/engine/KMEI/KeymanEngine/Classes/TextView.swift index 8f0ab0c639..43b735bc61 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/TextView.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/TextView.swift @@ -133,11 +133,6 @@ public class TextView: UITextView, KeymanResponder { font = UIFont.systemFont(ofSize: fontSize) } - if isFirstResponder { - resignFirstResponder() - becomeFirstResponder() - } - log.debug("TextView: \(self.hashValue) setFont: \(font?.familyName ?? "nil")") } From c49dab2103556c29b109d06bd7636cc1f71b9866 Mon Sep 17 00:00:00 2001 From: "Steven R. Loomis" Date: Wed, 10 Jan 2024 18:57:02 -0600 Subject: [PATCH 24/60] =?UTF-8?q?feat(core):=20unscape=20u=20=F0=9F=99=80?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - update unescaping from \u{…} to \uXXXX #10319 --- .../types/src/ldml-keyboard/pattern-parser.ts | 16 ++---- common/web/types/src/util/util.ts | 52 +++++++++++++++++++ common/web/types/test/util/test-unescape.ts | 14 ++++- developer/src/kmc-ldml/src/compiler/tran.ts | 7 ++- 4 files changed, 74 insertions(+), 15 deletions(-) diff --git a/common/web/types/src/ldml-keyboard/pattern-parser.ts b/common/web/types/src/ldml-keyboard/pattern-parser.ts index 055926960a..1eccd73e7d 100644 --- a/common/web/types/src/ldml-keyboard/pattern-parser.ts +++ b/common/web/types/src/ldml-keyboard/pattern-parser.ts @@ -3,7 +3,7 @@ */ import { constants } from "@keymanapp/ldml-keyboard-constants"; -import { MATCH_QUAD_ESCAPE, isOneChar, unescapeOneQuadString, unescapeString } from "../util/util.js"; +import { MATCH_QUAD_ESCAPE, isOneChar, unescapeOneQuadString, unescapeString, hexQuad } from "../util/util.js"; /** @@ -65,17 +65,9 @@ export class MarkerParser { /** Max count of markers */ public static readonly MAX_MARKER_COUNT = constants.marker_max_count; - /** 0000 … FFFF */ - private static hexQuad(n: number): string { - if (n < 0x000 || n > 0xFFFF) { - throw RangeError(`${n} not in [0x0000,0xFFFF]`); - } - return n.toString(16).padStart(4, '0'); - } - private static anyMarkerMatch() : string { - const start = MarkerParser.hexQuad(this.MIN_MARKER_INDEX); - const end = MarkerParser.hexQuad(this.MAX_MARKER_INDEX); + const start = hexQuad(this.MIN_MARKER_INDEX); + const end = hexQuad(this.MAX_MARKER_INDEX); return `${this.SENTINEL}${this.MARKER_CODE}[\\u${start}-\\u${end}]`; // TODO-LDML: #9121 wrong escape format } @@ -103,7 +95,7 @@ export class MarkerParser { if (!forMatch) { return String.fromCharCode(n); } else { - return `\\u${MarkerParser.hexQuad(n)}`; // TODO-LDML: #9121 wrong escape format + return `\\u${hexQuad(n)}`; // TODO-LDML: #9121 wrong escape format } } diff --git a/common/web/types/src/util/util.ts b/common/web/types/src/util/util.ts index 5c8eb70810..de5175e6a9 100644 --- a/common/web/types/src/util/util.ts +++ b/common/web/types/src/util/util.ts @@ -88,6 +88,58 @@ export function unescapeString(s: string): string { return s; } +/** 0000 … FFFF */ +export function hexQuad(n: number): string { + if (n < 0x000 || n > 0xFFFF) { + throw RangeError(`${n} not in [0x0000,0xFFFF]`); + } + return n.toString(16).padStart(4, '0'); +} + + +/** + * Unescape one codepoint to \u format + * @param hex one codepoint in hex, such as '0127' + * @returns the unescaped codepoint + */ +function regexOne(hex: string): string { + const unescaped = unescapeOne(hex); + // unescape as UTF-16 + return unescaped.split('').map(ch => '\\u' + hexQuad(ch.charCodeAt(0))).join(''); +} +/** + * Unescapes a string according to UTS#18§1.1, see + * @param s escaped string + * @returns + */ +export function unescapeStringToRegex(s: string): string { + if(!s) { + return s; + } + try { + /** + * process one regex match + * @param str ignored + * @param matched the entire match such as '0127' or '22 22' + * @returns the unescaped match + */ + function processMatch(str: string, matched: string) : string { + const codepoints = matched.split(' '); + const unescaped = codepoints.map(regexOne); + return unescaped.join(''); + } + s = s.replaceAll(MATCH_HEX_ESCAPE, processMatch); + } catch(e) { + if (e instanceof RangeError) { + throw new UnescapeError(`Out of range while unescaping '${s}': ${e.message}`, { cause: e }); + /* c8 ignore next 3 */ + } else { + throw e; // pass through some other error + } + } + return s; +} + /** True if this string *could* be a UTF-32 single char */ export function isOneChar(value: string) : boolean { diff --git a/common/web/types/test/util/test-unescape.ts b/common/web/types/test/util/test-unescape.ts index 732e9af2bb..d12ac6cfb0 100644 --- a/common/web/types/test/util/test-unescape.ts +++ b/common/web/types/test/util/test-unescape.ts @@ -1,6 +1,6 @@ import 'mocha'; import {assert} from 'chai'; -import {unescapeString, UnescapeError, isOneChar, toOneChar, unescapeOneQuadString, BadStringAnalyzer, isValidUnicode, describeCodepoint, isPUA, BadStringType} from '../../src/util/util.js'; +import {unescapeString, UnescapeError, isOneChar, toOneChar, unescapeOneQuadString, BadStringAnalyzer, isValidUnicode, describeCodepoint, isPUA, BadStringType, unescapeStringToRegex} from '../../src/util/util.js'; describe('test UTF32 functions()', function() { it('should properly categorize strings', () => { @@ -57,6 +57,18 @@ describe('test unescapeString()', function() { }); }); +describe('test unescapeRegex()', () => { + it("should correctly handle 1..6 char escapes", function() { + assert.equal(unescapeStringToRegex('\\u{9}'), '\\u0009'); // TAB + assert.equal(unescapeStringToRegex('\\u{4a}'), '\\u004a'); // J + assert.equal(unescapeStringToRegex('\\u{3c8}'), '\\u03c8'); // ψ + assert.equal(unescapeStringToRegex('\\u{304B}'), '\\u304b'); // か + // the following go to two UTF-16 escapes for ICU + assert.equal(unescapeStringToRegex('\\u{1e109}'), '\\ud838\\udd09'); // 𞄉 + assert.equal(unescapeStringToRegex('\\u{10fff0}'), '\\udbff\\udff0'); // Plane 16 Private Use + }); +}); + describe('test unescapeOneQuadString()', () => { it('should be able to convert', () => { // testing that `\u0127` is unescaped correctly (to U+0127: 'ħ') diff --git a/developer/src/kmc-ldml/src/compiler/tran.ts b/developer/src/kmc-ldml/src/compiler/tran.ts index 2e1944a7cc..d0251588d4 100644 --- a/developer/src/kmc-ldml/src/compiler/tran.ts +++ b/developer/src/kmc-ldml/src/compiler/tran.ts @@ -1,5 +1,5 @@ import { constants, SectionIdent } from "@keymanapp/ldml-keyboard-constants"; -import { KMXPlus, LDMLKeyboard, CompilerCallbacks, VariableParser, MarkerParser } from '@keymanapp/common-types'; +import { KMXPlus, LDMLKeyboard, CompilerCallbacks, VariableParser, MarkerParser, util } from '@keymanapp/common-types'; import { SectionCompiler } from "./section-compiler.js"; import Bksp = KMXPlus.Bksp; @@ -138,9 +138,12 @@ export abstract class TransformCompiler Date: Thu, 11 Jan 2024 13:07:26 -0600 Subject: [PATCH 25/60] =?UTF-8?q?feat(developer,common):=20all=20unescapin?= =?UTF-8?q?g=20as=20\uXXXX=20=F0=9F=99=80?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - update unescaping from \u{…} to \uXXXX in regex - re-unescape in the string analyzr (empty-compiler.ts) so that we catch PUA etc. - fix some XML files that had bad DTDs - common; updates to string util api #10319 --- common/web/types/src/util/util.ts | 20 ++++++++++++++++--- .../kmc-ldml/src/compiler/empty-compiler.ts | 6 ++++++ .../test/fixtures/sections/strs/hint-pua.xml | 4 ++-- .../sections/strs/invalid-illegal.xml | 2 +- .../sections/strs/warn-unassigned.xml | 2 +- 5 files changed, 27 insertions(+), 7 deletions(-) diff --git a/common/web/types/src/util/util.ts b/common/web/types/src/util/util.ts index de5175e6a9..91cb3bb532 100644 --- a/common/web/types/src/util/util.ts +++ b/common/web/types/src/util/util.ts @@ -23,7 +23,10 @@ export const MATCH_HEX_ESCAPE = /\\u{([0-9a-fA-F ]{1,})}/g; // const MATCH_HEX_ESCAPE = /\\u{((?:(?:[0-9a-fA-F]{1,5})|(?:10[0-9a-fA-F]{4})(?: (?!}))?)+)}/g; /** regex for single quad escape such as \u0127 */ -export const MATCH_QUAD_ESCAPE = /\\u([0-9a-fA-F]{4})/g; +export const CONTAINS_QUAD_ESCAPE = /\\u([0-9a-fA-F]{4})/; + +/** regex for single quad escape such as \u0127 */ +export const MATCH_QUAD_ESCAPE = new RegExp(CONTAINS_QUAD_ESCAPE, 'g'); export class UnescapeError extends Error { } @@ -55,6 +58,13 @@ export function unescapeOneQuadString(s: string): string { return s; } +/** unscape multiple occurences of \u0127 style strings */ +export function unescapeQuadString(s: string): string { + s = s.replaceAll(MATCH_QUAD_ESCAPE, (quad) => unescapeOneQuadString(quad)); + return s; +} + + /** * Unescapes a string according to UTS#18§1.1, see * @param s escaped string @@ -96,6 +106,10 @@ export function hexQuad(n: number): string { return n.toString(16).padStart(4, '0'); } +/** escape one char for regex in \uXXXX form */ +function escapeRegexChar(ch: string) { + return '\\u' + hexQuad(ch.charCodeAt(0)); +} /** * Unescape one codepoint to \u format @@ -104,8 +118,8 @@ export function hexQuad(n: number): string { */ function regexOne(hex: string): string { const unescaped = unescapeOne(hex); - // unescape as UTF-16 - return unescaped.split('').map(ch => '\\u' + hexQuad(ch.charCodeAt(0))).join(''); + // unescape as UTF-16 code units + return unescaped.split('').map(ch => escapeRegexChar(ch)).join(''); } /** * Unescapes a string according to UTS#18§1.1, see diff --git a/developer/src/kmc-ldml/src/compiler/empty-compiler.ts b/developer/src/kmc-ldml/src/compiler/empty-compiler.ts index ac45243734..911e56d896 100644 --- a/developer/src/kmc-ldml/src/compiler/empty-compiler.ts +++ b/developer/src/kmc-ldml/src/compiler/empty-compiler.ts @@ -36,6 +36,12 @@ export class StrsCompiler extends EmptyCompiler { const badStringAnalyzer = new util.BadStringAnalyzer(); const CONTAINS_MARKER_REGEX = new RegExp(MarkerParser.ANY_MARKER_MATCH); for (let s of strs.allProcessedStrings.values()) { + // replace all \\uXXXX with the actual code point. + // this lets us analyze whether there are PUA, unassigned, etc. + // the results might not be valid regex of course. + if (util.CONTAINS_QUAD_ESCAPE.test(s)) { + s = util.unescapeQuadString(s); + } // skip marker strings if (CONTAINS_MARKER_REGEX.test(s)) { // it had a marker, take out all marker strings, as the sentinel is illegal diff --git a/developer/src/kmc-ldml/test/fixtures/sections/strs/hint-pua.xml b/developer/src/kmc-ldml/test/fixtures/sections/strs/hint-pua.xml index 23c85d4023..355c5ef2b7 100644 --- a/developer/src/kmc-ldml/test/fixtures/sections/strs/hint-pua.xml +++ b/developer/src/kmc-ldml/test/fixtures/sections/strs/hint-pua.xml @@ -4,7 +4,7 @@ @@keys: [K_Q][K_W][K_Q] @@expected: \u0127\u1790\u17B6\u0127 --> - + @@ -46,7 +46,7 @@ - + diff --git a/developer/src/kmc-ldml/test/fixtures/sections/strs/invalid-illegal.xml b/developer/src/kmc-ldml/test/fixtures/sections/strs/invalid-illegal.xml index a21429d833..6362ac7fec 100644 --- a/developer/src/kmc-ldml/test/fixtures/sections/strs/invalid-illegal.xml +++ b/developer/src/kmc-ldml/test/fixtures/sections/strs/invalid-illegal.xml @@ -4,7 +4,7 @@ @@keys: [K_Q][K_W][K_Q] @@expected: \u0127\u1790\u17B6\u0127 --> - + diff --git a/developer/src/kmc-ldml/test/fixtures/sections/strs/warn-unassigned.xml b/developer/src/kmc-ldml/test/fixtures/sections/strs/warn-unassigned.xml index 79f3291dca..571fd2d4da 100644 --- a/developer/src/kmc-ldml/test/fixtures/sections/strs/warn-unassigned.xml +++ b/developer/src/kmc-ldml/test/fixtures/sections/strs/warn-unassigned.xml @@ -4,7 +4,7 @@ @@keys: [K_Q][K_W][K_Q] @@expected: \u0127\u1790\u17B6\u0127 --> - + From 04bce67d03b79cfa7d8830b12f7a8465b86ef5db Mon Sep 17 00:00:00 2001 From: "Steven R. Loomis" Date: Fri, 12 Jan 2024 23:21:38 -0600 Subject: [PATCH 26/60] =?UTF-8?q?feat(developer):=2032=20bit=20escapades?= =?UTF-8?q?=20=F0=9F=99=80?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - match sentinels as \\u… - support \Uxxxxxxxx for >0xffff - more escaping stuff #10319 --- .../types/src/ldml-keyboard/pattern-parser.ts | 10 ++++-- common/web/types/src/util/util.ts | 35 +++++++++++++------ .../test/ldml-keyboard/test-pattern-parser.ts | 4 +++ common/web/types/test/util/test-unescape.ts | 14 +++++--- 4 files changed, 47 insertions(+), 16 deletions(-) diff --git a/common/web/types/src/ldml-keyboard/pattern-parser.ts b/common/web/types/src/ldml-keyboard/pattern-parser.ts index 1eccd73e7d..a76831e670 100644 --- a/common/web/types/src/ldml-keyboard/pattern-parser.ts +++ b/common/web/types/src/ldml-keyboard/pattern-parser.ts @@ -51,10 +51,12 @@ export class MarkerParser { * Marker sentinel as a string - U+FFFF */ public static readonly SENTINEL = String.fromCodePoint(constants.uc_sentinel); + static readonly SENTINEL_MATCH = '\\u' + hexQuad(constants.uc_sentinel); /** * Marker code as a string - U+0008 */ public static readonly MARKER_CODE = String.fromCodePoint(constants.marker_code); + static readonly MARKER_CODE_MATCH = '\\u' + hexQuad(constants.marker_code); /** Minimum ID (trailing code unit) */ public static readonly MIN_MARKER_INDEX = constants.marker_min_index; @@ -68,7 +70,7 @@ export class MarkerParser { private static anyMarkerMatch() : string { const start = hexQuad(this.MIN_MARKER_INDEX); const end = hexQuad(this.MAX_MARKER_INDEX); - return `${this.SENTINEL}${this.MARKER_CODE}[\\u${start}-\\u${end}]`; // TODO-LDML: #9121 wrong escape format + return `${this.SENTINEL_MATCH}${this.MARKER_CODE_MATCH}[\\u${start}-\\u${end}]`; // TODO-LDML: #9121 wrong escape format } /** Expression that matches any marker */ @@ -104,7 +106,11 @@ export class MarkerParser { if (n < MarkerParser.MIN_MARKER_INDEX || n > MarkerParser.ANY_MARKER_INDEX) { throw RangeError(`Internal Error: marker index out of range ${n}`); } - return this.SENTINEL + this.MARKER_CODE + this.markerCodeToString(n, forMatch); + if (forMatch) { + return this.SENTINEL_MATCH + this.MARKER_CODE_MATCH + this.markerCodeToString(n, forMatch); + } else { + return this.SENTINEL + this.MARKER_CODE + this.markerCodeToString(n, forMatch); + } } /** @returns all marker strings as sentinel values */ diff --git a/common/web/types/src/util/util.ts b/common/web/types/src/util/util.ts index 91cb3bb532..de06216383 100644 --- a/common/web/types/src/util/util.ts +++ b/common/web/types/src/util/util.ts @@ -22,8 +22,8 @@ export function boxXmlArray(o: any, x: string): void { export const MATCH_HEX_ESCAPE = /\\u{([0-9a-fA-F ]{1,})}/g; // const MATCH_HEX_ESCAPE = /\\u{((?:(?:[0-9a-fA-F]{1,5})|(?:10[0-9a-fA-F]{4})(?: (?!}))?)+)}/g; -/** regex for single quad escape such as \u0127 */ -export const CONTAINS_QUAD_ESCAPE = /\\u([0-9a-fA-F]{4})/; +/** regex for single quad escape such as \u0127 or \U00000000 */ +export const CONTAINS_QUAD_ESCAPE = /(?:\\u([0-9a-fA-F]{4})|\\U([0-9a-fA-F]{8}))/; /** regex for single quad escape such as \u0127 */ export const MATCH_QUAD_ESCAPE = new RegExp(CONTAINS_QUAD_ESCAPE, 'g'); @@ -42,8 +42,10 @@ function unescapeOne(hex: string): string { } /** - * Unescape one single quad string such as \u0127. + * Unescape one single quad string such as \u0127 / \U00000000 * Throws exception if the string doesn't match MATCH_QUAD_ESCAPE + * Note this does not attempt to handle or reject surrogates. + * So, `\\uD838\\uDD09` will work but other combinations may not. * @param s input string * @returns output */ @@ -51,8 +53,8 @@ export function unescapeOneQuadString(s: string): string { if (!s || !s.match(MATCH_QUAD_ESCAPE)) { throw new UnescapeError(`Not a quad escape: ${s}`); } - function processMatch(str: string, matched: string): string { - return unescapeOne(matched); + function processMatch(str: string, m16: string, m32: string): string { + return unescapeOne(m16 || m32); // either \u or \U } s = s.replace(MATCH_QUAD_ESCAPE, processMatch); return s; @@ -100,26 +102,39 @@ export function unescapeString(s: string): string { /** 0000 … FFFF */ export function hexQuad(n: number): string { - if (n < 0x000 || n > 0xFFFF) { + if (n < 0x0000 || n > 0xFFFF) { throw RangeError(`${n} not in [0x0000,0xFFFF]`); } return n.toString(16).padStart(4, '0'); } +/** 00000000 … FFFFFFFF */ +export function hexOcts(n: number): string { + if (n < 0x0000 || n > 0xFFFFFFFF) { + throw RangeError(`${n} not in [0x00000000,0xFFFFFFFF]`); + } + return n.toString(16).padStart(8, '0'); +} + /** escape one char for regex in \uXXXX form */ function escapeRegexChar(ch: string) { - return '\\u' + hexQuad(ch.charCodeAt(0)); + const code = ch.codePointAt(0); + if (code <= 0xFFFF) { + return '\\u' + hexQuad(code); + } else { + return '\\U' + hexOcts(code); + } } /** - * Unescape one codepoint to \u format + * Unescape one codepoint to \u or \U format * @param hex one codepoint in hex, such as '0127' * @returns the unescaped codepoint */ function regexOne(hex: string): string { const unescaped = unescapeOne(hex); - // unescape as UTF-16 code units - return unescaped.split('').map(ch => escapeRegexChar(ch)).join(''); + // re-escape as 16 or 32 bit code units + return Array.from(unescaped).map(ch => escapeRegexChar(ch)).join(''); } /** * Unescapes a string according to UTS#18§1.1, see diff --git a/common/web/types/test/ldml-keyboard/test-pattern-parser.ts b/common/web/types/test/ldml-keyboard/test-pattern-parser.ts index 0274f2a5c2..4af2c29459 100644 --- a/common/web/types/test/ldml-keyboard/test-pattern-parser.ts +++ b/common/web/types/test/ldml-keyboard/test-pattern-parser.ts @@ -92,6 +92,10 @@ describe('Test of Pattern Parsers', () => { `Give me \\m{a} and \\m{c}, or \\m{.}.`, markers), `Give me \uFFFF\u0008\u0001 and \uFFFF\u0008\u0003, or \uFFFF\u0008\uD7FF.` ); + assert.equal(MarkerParser.toSentinelString( + `Give me \\m{a} and \\m{c}, or \\m{.}.`, markers, true), + `Give me \\uffff\\u0008\\u0001 and \\uffff\\u0008\\u0003, or ${MarkerParser.ANY_MARKER_MATCH}.` + ); assert.throws(() => MarkerParser.toSentinelString( `Want to see something funny? \\m{zzz}`, // out of range diff --git a/common/web/types/test/util/test-unescape.ts b/common/web/types/test/util/test-unescape.ts index d12ac6cfb0..089aca5371 100644 --- a/common/web/types/test/util/test-unescape.ts +++ b/common/web/types/test/util/test-unescape.ts @@ -1,6 +1,6 @@ import 'mocha'; import {assert} from 'chai'; -import {unescapeString, UnescapeError, isOneChar, toOneChar, unescapeOneQuadString, BadStringAnalyzer, isValidUnicode, describeCodepoint, isPUA, BadStringType, unescapeStringToRegex} from '../../src/util/util.js'; +import {unescapeString, UnescapeError, isOneChar, toOneChar, unescapeOneQuadString, BadStringAnalyzer, isValidUnicode, describeCodepoint, isPUA, BadStringType, unescapeStringToRegex, unescapeQuadString} from '../../src/util/util.js'; describe('test UTF32 functions()', function() { it('should properly categorize strings', () => { @@ -63,9 +63,8 @@ describe('test unescapeRegex()', () => { assert.equal(unescapeStringToRegex('\\u{4a}'), '\\u004a'); // J assert.equal(unescapeStringToRegex('\\u{3c8}'), '\\u03c8'); // ψ assert.equal(unescapeStringToRegex('\\u{304B}'), '\\u304b'); // か - // the following go to two UTF-16 escapes for ICU - assert.equal(unescapeStringToRegex('\\u{1e109}'), '\\ud838\\udd09'); // 𞄉 - assert.equal(unescapeStringToRegex('\\u{10fff0}'), '\\udbff\\udff0'); // Plane 16 Private Use + assert.equal(unescapeStringToRegex('\\u{1e109}'), '\\U0001e109'); // 𞄉 + assert.equal(unescapeStringToRegex('\\u{10fff0}'), '\\U0010fff0'); // Plane 16 Private Use }); }); @@ -73,12 +72,19 @@ describe('test unescapeOneQuadString()', () => { it('should be able to convert', () => { // testing that `\u0127` is unescaped correctly (to U+0127: 'ħ') assert.equal(unescapeOneQuadString('\\u0127'), '\u{0127}'); + assert.equal(unescapeOneQuadString('\\U0010FFF0'), '\u{10fff0}'); // test the fail cases }); it('should fail when it needs to fail', () => { assert.throws(() => unescapeOneQuadString(null), null); assert.throws(() => unescapeOneQuadString('\uFFFFFFFFFFFF')); }); + const PAIRED=`\\uD838\\uDD09`; + it('test of paired surrogates ${UNPAIRED}', () => { + const s = unescapeQuadString(PAIRED); + assert.equal(s, '\u{1e109}'); + assert.equal(s, '\u{d838}\u{dd09}'); + }); }); function titleize(o : any) { From c5d0e0525e47f3655bf1c4936f5062c53458c979 Mon Sep 17 00:00:00 2001 From: "Steven R. Loomis" Date: Sat, 13 Jan 2024 12:55:51 -0600 Subject: [PATCH 27/60] =?UTF-8?q?feat(developer):=2032=20bit=20escapades?= =?UTF-8?q?=20=F0=9F=99=80?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - escaping working, matching not yet #10319 --- core/src/kmx/kmx_processevent.cpp | 2 +- core/src/ldml/ldml_markers.cpp | 128 ++++++++--------------- core/src/ldml/ldml_processor.cpp | 72 ++++++++----- core/tests/unit/ldml/test_transforms.cpp | 27 +++-- 4 files changed, 107 insertions(+), 122 deletions(-) diff --git a/core/src/kmx/kmx_processevent.cpp b/core/src/kmx/kmx_processevent.cpp index 4b98312c36..f298d87cd8 100644 --- a/core/src/kmx/kmx_processevent.cpp +++ b/core/src/kmx/kmx_processevent.cpp @@ -11,7 +11,7 @@ using namespace kmx; /* Globals */ -KMX_BOOL km::core::kmx::g_debug_ToConsole = FALSE; +KMX_BOOL km::core::kmx::g_debug_ToConsole = TRUE; KMX_BOOL km::core::kmx::g_debug_KeymanLog = TRUE; KMX_BOOL km::core::kmx::g_silent = FALSE; diff --git a/core/src/ldml/ldml_markers.cpp b/core/src/ldml/ldml_markers.cpp index 3230614ab4..01ed9ab7f5 100644 --- a/core/src/ldml/ldml_markers.cpp +++ b/core/src/ldml/ldml_markers.cpp @@ -19,6 +19,10 @@ namespace core { namespace ldml { +const std::u32string REGEX_PREFIX = U"\\uFFFF\\u0008"; // does not include '\\' because it might be a range +const std::u32string RAW_PREFIX = U"\uFFFF\u0008"; +const std::u32string REGEX_ANY_MATCH = U"[\\u0001-\\uD7FE]"; + // string manipulation /** internal function to normalize with a specified mode */ @@ -176,23 +180,15 @@ prepend_marker(std::u32string &str, marker_num marker, marker_encoding encoding) assert(encoding == regex_sentinel); if (marker == LDML_MARKER_ANY_INDEX) { // recreate the regex from back to front - str.insert(0, 1, U']'); - prepend_hex_quad(str, LDML_MARKER_MAX_INDEX); - str.insert(0, 1, U'u'); - str.insert(0, 1, U'\\'); - str.insert(0, 1, U'-'); - prepend_hex_quad(str, LDML_MARKER_MIN_INDEX); - str.insert(0, 1, U'u'); - str.insert(0, 1, U'\\'); - str.insert(0, 1, U'['); - str.insert(0, 1, LDML_MARKER_CODE); - str.insert(0, 1, LDML_UC_SENTINEL); + str.insert(0, REGEX_ANY_MATCH); + str.insert(0, REGEX_PREFIX); } else { // add hex part prepend_hex_quad(str, marker); // add static part - km_core_usv markstr[] = {LDML_UC_SENTINEL, LDML_MARKER_CODE, u'\\', u'u'}; - str.insert(0, markstr, 4); + km_core_usv markstr[] = {u'\\', u'u'}; + str.insert(0, markstr, 2); + str.insert(0, REGEX_PREFIX); } } } @@ -256,27 +252,40 @@ std::u32string remove_markers(const std::u32string &str, marker_map *markers, ma auto i = str.begin(); auto last = i; marker_list last_markers; - for (i = find(i, str.end(), LDML_UC_SENTINEL); i != str.end(); i = find(i, str.end(), LDML_UC_SENTINEL)) { - // append any prefix (from prior pos'n to here) + const auto &lookfor_str = (encoding == regex_sentinel) ? REGEX_PREFIX : RAW_PREFIX; + auto lookfor = lookfor_str.at(0); + + for (i = find(i, str.end(), lookfor); i != str.end(); i = find(i, str.end(), lookfor)) { out.append(last, i); + assert(*i == lookfor); // assert that find() worked + last = i; // keep track of the last segment appendd. - // #1: LDML_UC_SENTINEL (what we searched for) - assert(*i == LDML_UC_SENTINEL); // assert that find() worked - i++; - last = i; - if (i == str.end()) { - break; // hit end + std::u32string rest(i, str.end()); + if (rest.length() <= lookfor_str.length()) { + // not enough left so it can't match, so continue + i = str.end(); // end of string + if (encoding == plain_sentinel) { + // in plain mode, delete any irregular sequences + last = i; + } + continue; } - // #2 LDML_MARKER_CODE - if (*i != LDML_MARKER_CODE) { - continue; // can't process this, get out + rest.resize(lookfor_str.length()); + + i += lookfor_str.length(); + + if (rest != lookfor_str) { + // no match - could be backslash something else + if (encoding == plain_sentinel) { + // in plain mode, delete any irregular sequences + last = i; + } + continue; } - i++; + + // matches. Skip over the prefix last = i; - if (i == str.end()) { - break; // hit end - } KMX_DWORD marker_no; if (encoding == plain_sentinel) { @@ -311,64 +320,13 @@ std::u32string remove_markers(const std::u32string &str, marker_map *markers, ma markno[3] = *(i++); marker_no = parse_hex_quad(markno); assert (marker_no != 0); // illegal marker number - } else if (*i == U'[') { - if (++i == str.end()) { - break; + } else if (*i == REGEX_ANY_MATCH.at(0)) { // '[' + std::u32string rest2(i, str.end()); + if (rest2.length() < REGEX_ANY_MATCH.length()) { + // not enough left so it can't match, so continue + continue; } - assert(*i == U'\\'); - if (++i == str.end()) { - break; - } - assert(*i == U'u'); - if (++i == str.end()) { - break; - } - assert(xdigitval(*i) != -1); - if (++i == str.end()) { - break; - } - assert(xdigitval(*i) != -1); - if (++i == str.end()) { - break; - } - assert(xdigitval(*i) != -1); - if (++i == str.end()) { - break; - } - assert(xdigitval(*i) != -1); - if (++i == str.end()) { - break; - } - assert(*i == U'-'); - if (++i == str.end()) { - break; - } - assert(*i == U'\\'); - if (++i == str.end()) { - break; - } - assert(*i == U'u'); - if (++i == str.end()) { - break; - } - assert(xdigitval(*i) != -1); - if (++i == str.end()) { - break; - } - assert(xdigitval(*i) != -1); - if (++i == str.end()) { - break; - } - assert(xdigitval(*i) != -1); - if (++i == str.end()) { - break; - } - assert(xdigitval(*i) != -1); - if (++i == str.end()) { - break; - } - assert(*i == U']'); - i++; + i += REGEX_ANY_MATCH.length(); marker_no = LDML_MARKER_ANY_INDEX; } else { assert(*i == U'\\' || *i == U'['); // error. diff --git a/core/src/ldml/ldml_processor.cpp b/core/src/ldml/ldml_processor.cpp index 42baaed0a4..6e2bd4b652 100644 --- a/core/src/ldml/ldml_processor.cpp +++ b/core/src/ldml/ldml_processor.cpp @@ -283,19 +283,23 @@ ldml_processor::process_key_string(km_core_state *state, const std::u16string &k size_t ldml_processor::process_output(km_core_state *state, const std::u32string &str, ldml::transforms *with_transforms) const { std::u32string nfd_str = str; - assert(ldml::normalize_nfd_markers(nfd_str)); // TODO-LDML: else fail? + // Note: + // The normalize functions have assert and Debuglog at the bottom. + // so we do not need to assert the status here unless we're going to do something + // different with control flow. + (void)ldml::normalize_nfd_markers(nfd_str); + // extract context string, in NFD std::u32string old_ctxtstr_nfd; (void)context_to_string(state, old_ctxtstr_nfd, true); - assert(ldml::normalize_nfd_markers(old_ctxtstr_nfd)); // TODO-LDML: else fail? + (void)ldml::normalize_nfd_markers(old_ctxtstr_nfd); // context string in NFD std::u32string ctxtstr; (void)context_to_string(state, ctxtstr, true); // with markers // add the newly added key output to ctxtstr ctxtstr.append(nfd_str); - assert(ldml::normalize_nfd_markers(ctxtstr)); // TODO-LDML: else fail? - + (void)ldml::normalize_nfd_markers(ctxtstr); /** transform output string */ std::u32string outputString; /** how many chars of the ctxtstr to replace */ @@ -305,9 +309,7 @@ size_t ldml_processor::process_output(km_core_state *state, const std::u32string if(with_transforms != nullptr) { matchedContext = with_transforms->apply(ctxtstr, outputString); - } else { - // no transforms, no output - } + } // else: no transforms, no output // Short Circuit: if no transforms matched, and no new text is being output, // just return. @@ -318,27 +320,13 @@ size_t ldml_processor::process_output(km_core_state *state, const std::u32string // drop last 'matchedContext': ctxtstr.resize(ctxtstr.length() - matchedContext); ctxtstr.append(outputString); // TODO-LDML: should be able to do a normalization-safe append here. - { - const auto normalize_ok = ldml::normalize_nfd_markers(ctxtstr); - assert(normalize_ok); - if(!normalize_ok) { - DebugLog("ldml_processor::process_output: failed ldml::normalize_nfd_markers(ctxtstr)"); - } - } + (void)ldml::normalize_nfd_markers(ctxtstr); // Ok. We've done all the happy manipulations. /** NFD w/ markers */ std::u32string ctxtstr_cleanedup = ctxtstr; - { - const auto normalize_ok = ldml::normalize_nfd_markers(ctxtstr_cleanedup); - assert(normalize_ok); - if(!normalize_ok) { - DebugLog("ldml_processor::process_output: failed ldml::normalize_nfd_markers(ctxtstr_cleanedup)"); - } - } - - assert(ldml::normalize_nfd_markers(ctxtstr_cleanedup)); + (void)ldml::normalize_nfd_markers(ctxtstr_cleanedup); // find common prefix. // For example, if the context previously had "aaBBBBB" and it is changing to "aaCCC" then we will have: @@ -346,6 +334,32 @@ size_t ldml_processor::process_output(km_core_state *state, const std::u32string // - new_ctxtstr_changed = "CCC" // So the BBBBB needs to be removed and then CCC added. auto ctxt_prefix = mismatch(old_ctxtstr_nfd.begin(), old_ctxtstr_nfd.end(), ctxtstr_cleanedup.begin(), ctxtstr_cleanedup.end()); + + // handle a special case where we're simply changing from one marker to another. + // Example: + // 0. old_ctxtstr_changed ends with … U+FFFF U+0008 | U+0001 … + // 1. ctxtstr_cleanedup ends with … U+FFFF U+0008 | U+0002 … + // Pipe symbol shows where the difference starts. + // As you can see, the different starts in the MIDDLE of a marker sequence. + // so, old_ctxtstr_changed will start with U+0001 + // and new_ctxtstr_changed will start with U+0002 + // remove_text will only be able to delete up to and through the U+0001 + // and it will need to emit a push_backspace(KM_CORE_BT_MARKER,…) due to the + // marker change. + // We can detect this because the unchanged_prefix will end with u+FFFF U+0008 + // + // Oh, and yes, test case 'regex-test-8a-0' hits this. + std::u32string common_prefix(old_ctxtstr_nfd.begin(), ctxt_prefix.first); + if (common_prefix.length() >= 2) { + auto iter = common_prefix.rbegin(); + if (*(iter++) == LDML_MARKER_CODE && *(iter++) == UC_SENTINEL) { + // adjust the iterator so that the "U+FFFF U+0008" is not a part of the common prefix. + ctxt_prefix.first -= 2; + ctxt_prefix.second += 2; + // Now, old_ctxtstr_changed and new_ctxtstr_changed will start with U+FFFF U+0008 … + } + } + /** The part of the old string to be removed */ std::u32string old_ctxtstr_changed(ctxt_prefix.first,old_ctxtstr_nfd.end()); /** The new context to be added */ @@ -367,6 +381,8 @@ size_t ldml_processor::process_output(km_core_state *state, const std::u32string void ldml_processor::remove_text(km_core_state *state, std::u32string &str, size_t length) { + // str is the string to remove, so it should be at least as long as length + assert(length <= str.length()); /** track how many context items have been removed, via push_backspace() */ size_t contextRemoved = 0; for (auto c = state->context().rbegin(); length > 0 && c != state->context().rend(); c++, contextRemoved++) { @@ -382,23 +398,23 @@ ldml_processor::remove_text(km_core_state *state, std::u32string &str, size_t le // Cause prior char to be removed state->actions().push_backspace(KM_CORE_BT_CHAR, c->character); } else if (type == KM_CORE_BT_MARKER) { - // It's a marker. - // need to be able to drop 3 chars assert(length >= 3); - length -= 3; + state->actions().push_backspace(KM_CORE_BT_MARKER, c->marker); // #3 - the marker. assert(lastCtx == c->marker); str.pop_back(); + length--; // #2 - the code assert(str.back() == LDML_MARKER_CODE); str.pop_back(); + length--; // #1 - the sentinel assert(str.back() == UC_SENTINEL); str.pop_back(); - // cause marker to be removed - state->actions().push_backspace(KM_CORE_BT_MARKER, c->marker); + length--; } } + assert(length == 0); // now, pop the context items for (size_t i = 0; i < contextRemoved; i++) { // we don't pop during the above loop because the iterator gets confused diff --git a/core/tests/unit/ldml/test_transforms.cpp b/core/tests/unit/ldml/test_transforms.cpp index c541f038e1..6e60f5c294 100644 --- a/core/tests/unit/ldml/test_transforms.cpp +++ b/core/tests/unit/ldml/test_transforms.cpp @@ -645,21 +645,30 @@ int test_strutils() { { marker_map map; std::cout << __FILE__ << ":" << __LINE__ << " - bad1" << std::endl; - const std::u32string src = U"6\U0000ffffq"; // missing code + const std::u32string src = U"6\U0000ffffq"; // missing sentinel subtype const std::u32string dst = remove_markers(src, map); - const std::u32string expect = U"6q"; + const std::u32string expect = U"6"; // 'q' removed zassert_string_equal(dst, expect); assert_equal(map.size(), 0); } { marker_map map; - std::cout << __FILE__ << ":" << __LINE__ << " - bad1" << std::endl; + std::cout << __FILE__ << ":" << __LINE__ << " - bad1b" << std::endl; const std::u32string src = U"6\U0000ffff"; // missing code const std::u32string dst = remove_markers(src, map); const std::u32string expect = U"6"; zassert_string_equal(dst, expect); assert_equal(map.size(), 0); } + { + marker_map map; + std::cout << __FILE__ << ":" << __LINE__ << " - bad1c" << std::endl; + const std::u32string src = U"6\U0000ffffzz"; // missing code + const std::u32string dst = remove_markers(src, map); + const std::u32string expect = U"6z"; + zassert_string_equal(dst, expect); + assert_equal(map.size(), 0); + } { marker_map map; std::cout << __FILE__ << ":" << __LINE__ << " - marker end test" << std::endl; @@ -836,13 +845,15 @@ int test_normalize() { // from tests - regex edition marker_map map; std::cout << __FILE__ << ":" << __LINE__ << " - complex test 9c+regex" << std::endl; - const std::u32string src = U"9ce\u0300\uFFFF\u0008\\u0002\u0320\uFFFF\u0008\\u0001"; - const std::u32string expect = U"9ce\uFFFF\u0008\\u0002\u0320\u0300\uFFFF\u0008\\u0001"; + const std::u32string src = U"9ce\u0300\\uFFFF\\u0008\\u0002\u0320\\uFFFF\\u0008\\u0001"; + const std::u32string expect = U"9ce\\uFFFF\\u0008\\u0002\u0320\u0300\\uFFFF\\u0008\\u0001"; std::u32string dst = src; - assert(normalize_nfd_markers_segment(dst, map, regex_sentinel)); // TODO-LDML: need regex flag + assert(normalize_nfd_markers_segment(dst, map, regex_sentinel)); if (dst != expect) { std::cout << "dst: " << Debug_UnicodeString(dst) << std::endl; + std::cout << " " << dst << std::endl; std::cout << "exp: " << Debug_UnicodeString(expect) << std::endl; + std::cout << " " << expect << std::endl; } zassert_string_equal(dst, expect); assert_equal(map.size(), 2); @@ -853,8 +864,8 @@ int test_normalize() { // from tests - regex edition marker_map map; std::cout << __FILE__ << ":" << __LINE__ << " - complex test \\m{.}" << std::endl; - const std::u32string src = U"9ce\u0300\uFFFF\u0008[\\u0001-\\uD7FE]\u0320\uFFFF\u0008\\u0001"; - const std::u32string expect = U"9ce\uFFFF\u0008[\\u0001-\\uD7FE]\u0320\u0300\uFFFF\u0008\\u0001"; + const std::u32string src = U"9ce\u0300\\uFFFF\\u0008[\\u0001-\\uD7FE]\u0320\\uFFFF\\u0008\\u0001"; + const std::u32string expect = U"9ce\\uFFFF\\u0008[\\u0001-\\uD7FE]\u0320\u0300\\uFFFF\\u0008\\u0001"; std::u32string dst = src; assert(normalize_nfd_markers_segment(dst, map, regex_sentinel)); if (dst != expect) { From e0a2ce22d726520a0b21cfbdfeb867dded28c472 Mon Sep 17 00:00:00 2001 From: "Steven R. Loomis" Date: Mon, 15 Jan 2024 14:06:50 -0600 Subject: [PATCH 28/60] =?UTF-8?q?feat(developer):=2032=20bit=20escapades?= =?UTF-8?q?=20=F0=9F=99=80?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - basic.txt fixup #10319 --- developer/src/kmc-ldml/test/fixtures/basic.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/developer/src/kmc-ldml/test/fixtures/basic.txt b/developer/src/kmc-ldml/test/fixtures/basic.txt index d90e762685..aefbc1ce92 100644 --- a/developer/src/kmc-ldml/test/fixtures/basic.txt +++ b/developer/src/kmc-ldml/test/fixtures/basic.txt @@ -154,7 +154,7 @@ block(bksp) 00 00 00 00 # KMX_DWORD index = 0 # transforms 0 - index(strNull,strElemTranFrom1b,2) # KMX_DWORD str from ^e + index(strNull,strElemTranFrom1b,2) # KMX_DWORD str from ^e ## << ?? index(strNull,strNull,2) # KMX_DWORD str to 0 index(strNull,strNull,2) # KMX_DWORD str mapFrom 0 index(strNull,strNull,2) # KMX_DWORD str mapTo 0 @@ -493,6 +493,7 @@ block(strs) # struct COMP_KMXPLUS_STRS { diff(strs,strFromSet) sizeof(strFromSet,2) diff(strs,strUSet) sizeof(strUSet,2) diff(strs,strAmarker) sizeof(strAmarker,2) + diff(strs,strSentinel0001r) sizeof(strSentinel0001r,2) diff(strs,strElemTranFrom1) sizeof(strElemTranFrom1,2) diff(strs,strElemTranFrom1a) sizeof(strElemTranFrom1a,2) diff(strs,strElemTranFrom1b) sizeof(strElemTranFrom1b,2) @@ -516,7 +517,6 @@ block(strs) # struct COMP_KMXPLUS_STRS { diff(strs,strKeys) sizeof(strKeys,2) diff(strs,strIndicator) sizeof(strIndicator,2) diff(strs,strSentinel0001) sizeof(strSentinel0001,2) - diff(strs,strSentinel0001r) sizeof(strSentinel0001r,2) # String table -- block(x) is used to store the null u16char at end of each string # without interfering with sizeof() calculation above @@ -528,6 +528,7 @@ block(strs) # struct COMP_KMXPLUS_STRS { block(strFromSet) 5B 00 5C 00 75 00 31 00 41 00 37 00 35 00 2D 00 5C 00 75 00 31 00 41 00 37 00 39 00 5D 00 block(x) 00 00 # [\u1a75-\u1a79] block(strUSet) 5b 00 61 00 62 00 63 00 5d 00 block(x) 00 00 # '[abc]' block(strAmarker) 5C 00 6D 00 7B 00 61 00 7D 00 block(x) 00 00 # '\m{a}' + block(strSentinel0001r) 5c 00 75 00 66 00 66 00 66 00 66 00 5c 00 75 00 30 00 30 00 30 00 38 00 5C 00 75 00 30 00 30 00 30 00 31 00 block(x) 00 00 # UC_SENTINEL CODE_DEADKEY \u0001 (regex form) block(strElemTranFrom1) 5E 00 block(x) 00 00 # '^' block(strElemTranFrom1a) 5E 00 61 00 block(x) 00 00 # '^a' block(strElemTranFrom1b) 5E 00 65 00 block(x) 00 00 # '^e' @@ -556,7 +557,6 @@ block(strs) # struct COMP_KMXPLUS_STRS { # block(strIndicator) 3d d8 40 de block(x) 00 00 # '🙀' block(strSentinel0001) FF FF 08 00 01 00 block(x) 00 00 # UC_SENTINEL CODE_DEADKEY U+0001 - block(strSentinel0001r) FF FF 08 00 5C 00 75 00 30 00 30 00 30 00 31 00 block(x) 00 00 # UC_SENTINEL CODE_DEADKEY \u0001 (regex form) block(endstrs) # end of strs block From ab67061843517445411039905c842aab9c35dc87 Mon Sep 17 00:00:00 2001 From: "Steven R. Loomis" Date: Mon, 15 Jan 2024 14:07:19 -0600 Subject: [PATCH 29/60] =?UTF-8?q?feat(core):=2032=20bit=20escapades=20?= =?UTF-8?q?=F0=9F=99=80?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - improved test code and marker parsing #10319 --- core/src/ldml/ldml_markers.cpp | 67 +++++++++++++++--------- core/tests/unit/ldml/test_transforms.cpp | 8 +-- 2 files changed, 45 insertions(+), 30 deletions(-) diff --git a/core/src/ldml/ldml_markers.cpp b/core/src/ldml/ldml_markers.cpp index 01ed9ab7f5..1eb3bcf743 100644 --- a/core/src/ldml/ldml_markers.cpp +++ b/core/src/ldml/ldml_markers.cpp @@ -19,9 +19,9 @@ namespace core { namespace ldml { -const std::u32string REGEX_PREFIX = U"\\uFFFF\\u0008"; // does not include '\\' because it might be a range -const std::u32string RAW_PREFIX = U"\uFFFF\u0008"; -const std::u32string REGEX_ANY_MATCH = U"[\\u0001-\\uD7FE]"; +const std::u32string REGEX_PREFIX = U"\\uffff\\u0008"; // does not include '\\' because it might be a range +const std::u32string RAW_PREFIX = U"\uffff\u0008"; +const std::u32string REGEX_ANY_MATCH = U"[\\u0001-\\ud7fe]"; // string manipulation @@ -235,7 +235,7 @@ KMX_DWORD parse_hex_quad(const km_core_usv hex_str[]) { } /** add the list to the map */ -void add_markers_to_map(marker_map &markers, char32_t marker_ch, const marker_list &list) { +static void add_markers_to_map(marker_map &markers, char32_t marker_ch, const marker_list &list) { auto rep = markers.emplace(marker_ch, list); if (!rep.second) { // already existed. @@ -247,15 +247,45 @@ void add_markers_to_map(marker_map &markers, char32_t marker_ch, const marker_li } } +/** + * Add any markers, if needed + * @param markers marker map or nullptr + * @param last the 'last' parameter past the prior parsing + * @param end end of the input string + */ +static void +add_pending_markers( + marker_map *markers, + marker_list &last_markers, + const std::u32string::const_iterator &last, + const std::u32string::const_iterator &end) { + if(markers == nullptr || last_markers.empty()) { + return; + } + char32_t marker_ch; + if (last == end) { + marker_ch = MARKER_BEFORE_EOT; + } else { + marker_ch = *last; + } + add_markers_to_map(*markers, marker_ch, last_markers); + last_markers.clear(); // mark as already recorded +} + std::u32string remove_markers(const std::u32string &str, marker_map *markers, marker_encoding encoding) { std::u32string out; - auto i = str.begin(); - auto last = i; + auto i = str.begin(); // current iterator + auto last = i; // points to the part of the string after the last matched marker marker_list last_markers; const auto &lookfor_str = (encoding == regex_sentinel) ? REGEX_PREFIX : RAW_PREFIX; auto lookfor = lookfor_str.at(0); for (i = find(i, str.end(), lookfor); i != str.end(); i = find(i, str.end(), lookfor)) { + if (last != i) { + // add any markers found before this entry, but only if there is intervening + // text. This prevents the sentinel or the '\u' from becoming the attachment char. + add_pending_markers(markers, last_markers, last, str.end()); + } out.append(last, i); assert(*i == lookfor); // assert that find() worked last = i; // keep track of the last segment appendd. @@ -336,32 +366,17 @@ std::u32string remove_markers(const std::u32string &str, marker_map *markers, ma assert(marker_no >= LDML_MARKER_MIN_INDEX && marker_no <= LDML_MARKER_ANY_INDEX); // The marker number is good, add it to the list last = i; - // record the marker if (marker_no >= LDML_MARKER_MIN_INDEX && markers != nullptr) { // add it to the list last_markers.emplace_back(marker_no); - char32_t marker_ch; - if (i == str.end()) { - // Hit end, so mark it as the end - marker_ch = MARKER_BEFORE_EOT; - } else if (*i == LDML_UC_SENTINEL) { - // it's another marker (presumably) - continue; // loop around - } else { - marker_ch = *i; - } - add_markers_to_map(*markers, marker_ch, last_markers); - last_markers.clear(); // mark as already recorded } } - // get the suffix between the last marker and the end + // add any remaining pending markers. + // if last == str.end() then this wil be MARKER_BEFORE_EOT + // otherwise it will be the glue character + add_pending_markers(markers, last_markers, last, str.end()); + // get the suffix between the last marker and the end (could be nothing) out.append(last, str.end()); - if (!last_markers.empty() && markers != nullptr) { - // we had markers but couldn't find the base. - // it's possible that there was a malformed UC_SENTINEL string in between. - // Add it to the end. - add_markers_to_map(*markers, MARKER_BEFORE_EOT, last_markers); - } return out; } diff --git a/core/tests/unit/ldml/test_transforms.cpp b/core/tests/unit/ldml/test_transforms.cpp index 6e60f5c294..b4d3a3e07a 100644 --- a/core/tests/unit/ldml/test_transforms.cpp +++ b/core/tests/unit/ldml/test_transforms.cpp @@ -845,8 +845,8 @@ int test_normalize() { // from tests - regex edition marker_map map; std::cout << __FILE__ << ":" << __LINE__ << " - complex test 9c+regex" << std::endl; - const std::u32string src = U"9ce\u0300\\uFFFF\\u0008\\u0002\u0320\\uFFFF\\u0008\\u0001"; - const std::u32string expect = U"9ce\\uFFFF\\u0008\\u0002\u0320\u0300\\uFFFF\\u0008\\u0001"; + const std::u32string src = U"9ce\u0300\\uffff\\u0008\\u0002\u0320\\uffff\\u0008\\u0001"; + const std::u32string expect = U"9ce\\uffff\\u0008\\u0002\u0320\u0300\\uffff\\u0008\\u0001"; std::u32string dst = src; assert(normalize_nfd_markers_segment(dst, map, regex_sentinel)); if (dst != expect) { @@ -864,8 +864,8 @@ int test_normalize() { // from tests - regex edition marker_map map; std::cout << __FILE__ << ":" << __LINE__ << " - complex test \\m{.}" << std::endl; - const std::u32string src = U"9ce\u0300\\uFFFF\\u0008[\\u0001-\\uD7FE]\u0320\\uFFFF\\u0008\\u0001"; - const std::u32string expect = U"9ce\\uFFFF\\u0008[\\u0001-\\uD7FE]\u0320\u0300\\uFFFF\\u0008\\u0001"; + const std::u32string src = U"9ce\u0300\\uffff\\u0008[\\u0001-\\ud7fe]\u0320\\uffff\\u0008\\u0001"; + const std::u32string expect = U"9ce\\uffff\\u0008[\\u0001-\\ud7fe]\u0320\u0300\\uffff\\u0008\\u0001"; std::u32string dst = src; assert(normalize_nfd_markers_segment(dst, map, regex_sentinel)); if (dst != expect) { From a5ae4e3b251d5719703acc99e9da74941af49756 Mon Sep 17 00:00:00 2001 From: "Steven R. Loomis" Date: Mon, 15 Jan 2024 16:39:38 -0600 Subject: [PATCH 30/60] =?UTF-8?q?feat(developer):=20don't=20re-escape=20no?= =?UTF-8?q?n-syntax=20chars=20=F0=9F=99=80?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - \u{0300} needs to turn back into an actual char, so normalization gets an opportunity to run. #10319 --- common/web/types/src/util/util.ts | 14 +++++++++++++- common/web/types/test/util/test-unescape.ts | 18 ++++++++++++------ 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/common/web/types/src/util/util.ts b/common/web/types/src/util/util.ts index de06216383..712efe8246 100644 --- a/common/web/types/src/util/util.ts +++ b/common/web/types/src/util/util.ts @@ -126,6 +126,18 @@ function escapeRegexChar(ch: string) { } } +/** chars that must be escaped: syntax, C0 + C1 controls */ +const REGEX_SYNTAX_CHAR = /^[\u0000-\u001F\u007F-\u009F{}\[\]\\?.^$*-]$/; + +function escapeRegexCharIfSyntax(ch: string) { + // escape if syntax or not valid + if (REGEX_SYNTAX_CHAR.test(ch) || !isValidUnicode(ch.codePointAt(0))) { + return escapeRegexChar(ch); + } else { + return ch; // leave unescaped + } +} + /** * Unescape one codepoint to \u or \U format * @param hex one codepoint in hex, such as '0127' @@ -134,7 +146,7 @@ function escapeRegexChar(ch: string) { function regexOne(hex: string): string { const unescaped = unescapeOne(hex); // re-escape as 16 or 32 bit code units - return Array.from(unescaped).map(ch => escapeRegexChar(ch)).join(''); + return Array.from(unescaped).map(ch => escapeRegexCharIfSyntax(ch)).join(''); } /** * Unescapes a string according to UTS#18§1.1, see diff --git a/common/web/types/test/util/test-unescape.ts b/common/web/types/test/util/test-unescape.ts index 089aca5371..989951ae5c 100644 --- a/common/web/types/test/util/test-unescape.ts +++ b/common/web/types/test/util/test-unescape.ts @@ -59,12 +59,18 @@ describe('test unescapeString()', function() { describe('test unescapeRegex()', () => { it("should correctly handle 1..6 char escapes", function() { - assert.equal(unescapeStringToRegex('\\u{9}'), '\\u0009'); // TAB - assert.equal(unescapeStringToRegex('\\u{4a}'), '\\u004a'); // J - assert.equal(unescapeStringToRegex('\\u{3c8}'), '\\u03c8'); // ψ - assert.equal(unescapeStringToRegex('\\u{304B}'), '\\u304b'); // か - assert.equal(unescapeStringToRegex('\\u{1e109}'), '\\U0001e109'); // 𞄉 - assert.equal(unescapeStringToRegex('\\u{10fff0}'), '\\U0010fff0'); // Plane 16 Private Use + assert.equal(unescapeStringToRegex('\\u{9}'), '\\u0009'); // TAB + assert.equal(unescapeStringToRegex('\\u{5b}'), '\\u005b'); // [ + assert.equal(unescapeStringToRegex('\\u{005b}'), '\\u005b'); // [ + assert.equal(unescapeStringToRegex('\\u{4a}'), 'J'); // J + assert.equal(unescapeStringToRegex('\\u{8a}'), '\\u008a'); // J + assert.equal(unescapeStringToRegex('\\u{3c8}'), 'ψ'); // ψ + assert.equal(unescapeStringToRegex('\\u{304B}'), 'か'); // か + assert.equal(unescapeStringToRegex('\\u{ffff}'), '\\uffff'); // noncharacter + assert.equal(unescapeStringToRegex('\\u{1e109}'), '𞄉'); // 𞄉 + assert.equal(unescapeStringToRegex('\\u{1ffff}'), '\\U0001ffff'); // nonchar + assert.equal(unescapeStringToRegex('\\u{10fff0}'), '\u{10fff0}'); // Plane 16 Private Use + assert.equal(unescapeStringToRegex('\\u{10ffff}'), '\\U0010ffff'); // nonchar }); }); From eb39bba3744d25bbb18f220c496245bfd4be2752 Mon Sep 17 00:00:00 2001 From: "Steven R. Loomis" Date: Mon, 15 Jan 2024 17:27:41 -0600 Subject: [PATCH 31/60] =?UTF-8?q?chore(developer,core):=20change=20sample?= =?UTF-8?q?=20and=20test=20files=20to=20use=20\u{=E2=80=A6}=20=F0=9F=99=80?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - #10389 will require this, but at least change the test files #10319 --- core/tests/unit/ldml/keyboards/k_020_fr.xml | 2 +- .../unit/ldml/keyboards/k_200_reorder_nod_Lana.xml | 12 ++++++------ developer/src/kmc-ldml/test/fixtures/basic.xml | 2 +- .../test/fixtures/sections/ordr/nod-Lana.xml | 12 ++++++------ .../test/fixtures/sections/strs/warn-unassigned.xml | 2 +- .../ldml-keyboards/techpreview/3.0/bn.xml | 2 +- .../techpreview/3.0/fr-t-k0-optimise.xml | 8 ++++---- 7 files changed, 20 insertions(+), 20 deletions(-) diff --git a/core/tests/unit/ldml/keyboards/k_020_fr.xml b/core/tests/unit/ldml/keyboards/k_020_fr.xml index a5fd85f1a3..297062e5d1 100644 --- a/core/tests/unit/ldml/keyboards/k_020_fr.xml +++ b/core/tests/unit/ldml/keyboards/k_020_fr.xml @@ -16,7 +16,7 @@ - - - - - - - + + + + + + diff --git a/developer/src/kmc-ldml/test/fixtures/basic.xml b/developer/src/kmc-ldml/test/fixtures/basic.xml index 0044812652..275aa0f437 100644 --- a/developer/src/kmc-ldml/test/fixtures/basic.xml +++ b/developer/src/kmc-ldml/test/fixtures/basic.xml @@ -48,7 +48,7 @@ - + diff --git a/developer/src/kmc-ldml/test/fixtures/sections/ordr/nod-Lana.xml b/developer/src/kmc-ldml/test/fixtures/sections/ordr/nod-Lana.xml index 8395ea581a..ec1fe5c6e8 100644 --- a/developer/src/kmc-ldml/test/fixtures/sections/ordr/nod-Lana.xml +++ b/developer/src/kmc-ldml/test/fixtures/sections/ordr/nod-Lana.xml @@ -8,12 +8,12 @@ - - - - - - + + + + + + diff --git a/developer/src/kmc-ldml/test/fixtures/sections/strs/warn-unassigned.xml b/developer/src/kmc-ldml/test/fixtures/sections/strs/warn-unassigned.xml index 571fd2d4da..553795a0df 100644 --- a/developer/src/kmc-ldml/test/fixtures/sections/strs/warn-unassigned.xml +++ b/developer/src/kmc-ldml/test/fixtures/sections/strs/warn-unassigned.xml @@ -46,7 +46,7 @@ - + diff --git a/resources/standards-data/ldml-keyboards/techpreview/3.0/bn.xml b/resources/standards-data/ldml-keyboards/techpreview/3.0/bn.xml index 167416e944..a4b4133591 100644 --- a/resources/standards-data/ldml-keyboards/techpreview/3.0/bn.xml +++ b/resources/standards-data/ldml-keyboards/techpreview/3.0/bn.xml @@ -129,7 +129,7 @@ - + diff --git a/resources/standards-data/ldml-keyboards/techpreview/3.0/fr-t-k0-optimise.xml b/resources/standards-data/ldml-keyboards/techpreview/3.0/fr-t-k0-optimise.xml index 8db073cc76..b8c6f76b47 100644 --- a/resources/standards-data/ldml-keyboards/techpreview/3.0/fr-t-k0-optimise.xml +++ b/resources/standards-data/ldml-keyboards/techpreview/3.0/fr-t-k0-optimise.xml @@ -8,7 +8,7 @@ - + @@ -191,9 +191,9 @@ - - - + + + From fdb14f2582cd07545ab99d5cc79bc026e3b7d4c2 Mon Sep 17 00:00:00 2001 From: Darcy Wong Date: Tue, 16 Jan 2024 09:26:24 +0700 Subject: [PATCH 32/60] chore(android): Update targetSdkVersion to 34 --- android/KMAPro/kMAPro/build.gradle | 4 ++-- android/KMEA/app/build.gradle | 4 ++-- android/Samples/KMSample1/app/build.gradle | 4 ++-- android/Samples/KMSample2/app/build.gradle | 4 ++-- android/Tests/KeyboardHarness/app/build.gradle | 4 ++-- android/Tests/keycode/app/build.gradle | 3 +-- oem/firstvoices/android/app/build.gradle | 4 ++-- 7 files changed, 13 insertions(+), 14 deletions(-) diff --git a/android/KMAPro/kMAPro/build.gradle b/android/KMAPro/kMAPro/build.gradle index e12b928a9c..226e912fbe 100644 --- a/android/KMAPro/kMAPro/build.gradle +++ b/android/KMAPro/kMAPro/build.gradle @@ -10,7 +10,7 @@ ext.rootPath = '../../' apply from: "$rootPath/version.gradle" android { - compileSdkVersion 33 + compileSdk 34 namespace="com.tavultesoft.kmapro" // Don't compress kmp files so they can be copied via AssetManager @@ -21,7 +21,7 @@ android { defaultConfig { applicationId "com.tavultesoft.kmapro" minSdkVersion 21 - targetSdkVersion 33 + targetSdkVersion 34 //println "===DUMPING PROPERTIES===" //dumpProperties(project) // Use this to dump all external properties for debugging TeamCity integration diff --git a/android/KMEA/app/build.gradle b/android/KMEA/app/build.gradle index 423862b79b..feed5dd66c 100644 --- a/android/KMEA/app/build.gradle +++ b/android/KMEA/app/build.gradle @@ -7,12 +7,12 @@ ext.rootPath = '../../' apply from: "$rootPath/version.gradle" android { - compileSdkVersion 33 + compileSdk 34 namespace "com.keyman.engine" defaultConfig { minSdkVersion 21 - targetSdkVersion 33 + targetSdkVersion 34 // VERSION_CODE and VERSION_NAME from version.gradle but Gradle removes them for libraries buildConfigField "String", "KEYMAN_ENGINE_VERSION_NAME", "\""+VERSION_NAME+"\"" diff --git a/android/Samples/KMSample1/app/build.gradle b/android/Samples/KMSample1/app/build.gradle index 7be5ef6b1b..4f5f778c35 100644 --- a/android/Samples/KMSample1/app/build.gradle +++ b/android/Samples/KMSample1/app/build.gradle @@ -3,7 +3,7 @@ plugins { } android { - compileSdkVersion 33 + compileSdk 34 namespace="com.keyman.kmsample1" // Don't compress kmp files so they can be copied via AssetManager @@ -14,7 +14,7 @@ android { defaultConfig { applicationId "com.keyman.kmsample1" minSdkVersion 21 - targetSdkVersion 33 + targetSdkVersion 34 versionCode 1 versionName "1.0" } diff --git a/android/Samples/KMSample2/app/build.gradle b/android/Samples/KMSample2/app/build.gradle index fc4ca11b94..04e73b8405 100644 --- a/android/Samples/KMSample2/app/build.gradle +++ b/android/Samples/KMSample2/app/build.gradle @@ -3,7 +3,7 @@ plugins { } android { - compileSdkVersion 33 + compileSdk 34 namespace="com.keyman.kmsample2" // Don't compress kmp files so they can be copied via AssetManager @@ -14,7 +14,7 @@ android { defaultConfig { applicationId "com.keyman.kmsample2" minSdkVersion 21 - targetSdkVersion 33 + targetSdkVersion 34 versionCode 1 versionName "1.0" } diff --git a/android/Tests/KeyboardHarness/app/build.gradle b/android/Tests/KeyboardHarness/app/build.gradle index 29dd7de081..61fe8a53fa 100644 --- a/android/Tests/KeyboardHarness/app/build.gradle +++ b/android/Tests/KeyboardHarness/app/build.gradle @@ -6,7 +6,7 @@ ext.rootPath = '../../../' apply from: "$rootPath/version.gradle" android { - compileSdkVersion 33 + compileSdk 34 namespace="com.keyman.android.tests.keyboardHarness" // Don't compress kmp files so they can be copied via AssetManager @@ -21,7 +21,7 @@ android { defaultConfig { applicationId "com.keyman.android.tests.keyboardHarness" minSdkVersion 21 - targetSdkVersion 33 + targetSdkVersion 34 // VERSION_CODE and VERSION_NAME from version.gradle versionCode VERSION_CODE as Integer diff --git a/android/Tests/keycode/app/build.gradle b/android/Tests/keycode/app/build.gradle index f5501020e6..0db69e9da2 100644 --- a/android/Tests/keycode/app/build.gradle +++ b/android/Tests/keycode/app/build.gradle @@ -4,13 +4,12 @@ ext.rootPath = '../../../' apply from: "$rootPath/version.gradle" android { - compileSdkVersion 33 namespace="com.keyman.android.tests.keycode" defaultConfig { applicationId "com.keyman.android.tests.keycode" minSdkVersion 21 - targetSdkVersion 33 + targetSdkVersion 34 // VERSION_CODE and VERSION_NAME from version.gradle versionCode VERSION_CODE as Integer diff --git a/oem/firstvoices/android/app/build.gradle b/oem/firstvoices/android/app/build.gradle index 646152a81a..43adc17265 100644 --- a/oem/firstvoices/android/app/build.gradle +++ b/oem/firstvoices/android/app/build.gradle @@ -8,13 +8,13 @@ ext.rootPath = '../../../../android' apply from: "$rootPath/version.gradle" android { - compileSdkVersion 33 + compileSdk 34 namespace="com.firstvoices.keyboards" defaultConfig { applicationId "com.firstvoices.keyboards" minSdkVersion 21 - targetSdkVersion 33 + targetSdkVersion 34 // VERSION_CODE and VERSION_NAME from version.gradle versionCode VERSION_CODE as Integer From b9d1cd044aadc688a81aee927a4e0f1b56afa86a Mon Sep 17 00:00:00 2001 From: Darcy Wong Date: Tue, 16 Jan 2024 09:51:33 +0700 Subject: [PATCH 33/60] chore(android/app): Update whatsnew for 17.0 --- android/help/about/whatsnew.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/android/help/about/whatsnew.md b/android/help/about/whatsnew.md index 2714defd81..71c47854fb 100644 --- a/android/help/about/whatsnew.md +++ b/android/help/about/whatsnew.md @@ -2,3 +2,6 @@ title: What's New --- Here are some of the new features we have added to Keyman 17.0 for Android: + +* When suggestions aren't enabled, display a themed banner (#9696) +* Smoother keyboard initialization (#10022) From fd2f3ca8d11a11cb9e096361f14e32f26da4cc66 Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Tue, 16 Jan 2024 10:24:59 +0700 Subject: [PATCH 34/60] feat(web): more prominent fade, fade state management --- .../engine/osk/src/banner/suggestionBanner.ts | 33 +++++++++++++++++++ web/src/resources/osk/kmwosk.css | 27 ++++++++++++--- 2 files changed, 56 insertions(+), 4 deletions(-) diff --git a/web/src/engine/osk/src/banner/suggestionBanner.ts b/web/src/engine/osk/src/banner/suggestionBanner.ts index 80b0f20ff3..3f90f8943c 100644 --- a/web/src/engine/osk/src/banner/suggestionBanner.ts +++ b/web/src/engine/osk/src/banner/suggestionBanner.ts @@ -27,6 +27,12 @@ const BANNER_SCROLLER_CLASS = 'kmw-suggest-banner-scroller'; const BANNER_VERT_ROAMING_HEIGHT_RATIO = 0.666; +/** + * The style to temporarily apply when updating suggestion text in order to prevent + * fade transitions at that time. + */ +const FADE_SWALLOW_STYLE = 'swallow-fade-transition'; + /** * Defines various parameters used by `BannerSuggestion` instances for layout and formatting. * This object is designed first and foremost for use with `BannerSuggestion.update()`. @@ -193,6 +199,33 @@ export class BannerSuggestion { } else { collapserStyle.marginLeft = (this.collapsedWidth - this.expandedWidth) + 'px'; } + + this.updateFade(); + } + + public updateFade() { + // Note: selected suggestion fade transitions are handled purely by CSS. + // We want to prevent them when updating a suggestion, though. + this.div.classList.add(FADE_SWALLOW_STYLE); + // Be sure that our fade-swallow mechanism is able to trigger once; + // we'll remove it after the current animation frame. + window.requestAnimationFrame(() => { + this.div.classList.remove(FADE_SWALLOW_STYLE); + }) + + // Never apply fading to the side that doesn't overflow. + this.div.classList.add(`kmw-hide-fade-${this.rtl ? 'left' : 'right'}`); + + // Matches the side that overflows, depending on if LTR or RTL. + const fadeClass = `kmw-hide-fade-${this.rtl ? 'right' : 'left'}`; + + // Is the suggestion already its ideal width?. + if(!(this.expandedWidth - this.collapsedWidth)) { + // Yes? Don't do any fading. + this.div.classList.add(fadeClass); + } else { + this.div.classList.remove(fadeClass); + } } /** diff --git a/web/src/resources/osk/kmwosk.css b/web/src/resources/osk/kmwosk.css index 4389d93d62..d0b9980d21 100644 --- a/web/src/resources/osk/kmwosk.css +++ b/web/src/resources/osk/kmwosk.css @@ -396,7 +396,7 @@ /* Set scrollable-suggestion fade width here. Make sure to also set .kmw-suggestion-text * padding-left and padding-right accordingly! */ - width: 4px; + width: 32px; height: 100%; content: ''; top: 0; @@ -406,6 +406,12 @@ touch-action: none; /* Doesn't seem to allow touch-through, though - even with touch-action: none */ /* https://stackoverflow.com/q/21474722 - poster never could find a solution, and settled*/ /* on the same workaround: a 'before' and 'after' piece instead of a single overlay.*/ + transition: opacity 0.25s linear; +} + +.kmw-suggest-option.swallow-fade-transition::before, +.kmw-suggest-option.swallow-fade-transition::after { + transition-duration: 0s; } .kmw-suggest-option::before { @@ -413,6 +419,12 @@ left: 0; } +.kmw-suggest-option.kmw-hide-fade-left::before, +.kmw-suggest-option.kmw-hide-fade-right::after { + opacity: 0; + /* visibility: hidden; */ +} + .kmw-suggest-option::after { background: linear-gradient(90deg, transparent 0%, darkorange 100%); right: 0; @@ -423,6 +435,12 @@ background: #bbb; } +.kmw-suggest-option.kmw-suggest-touched::before, +.kmw-suggest-option.kmw-suggest-touched::after { + /* Immediately start hiding the fade styling for touched suggestions. */ + opacity: 0; +} + /* Creates a gradient to fade text at the borders, providing visual indication of overflow */ /* Make sure the non-transparent color of the gradient matches .kmw-banner-bar's background-color. */ .kmw-suggest-option.kmw-suggest-touched::before { @@ -440,10 +458,11 @@ line-height: normal; position: relative; vertical-align: middle; - padding-left: 4px; /* To prevent start & end of suggestion from being affected by gradient effects */ - padding-right: 4px; /* Set these to match scrollable-suggestion fade width set above. */ + /* Contrast with .kmw-suggest-option::before .width styling. */ + padding-left: 8px; /* Keeps a bit of whitespace on the suggestion's side. */ + padding-right: 8px; /* Keeps a bit of whitespace on the suggestion's side. */ width: max-content; /* Ensure the text span acts like it contains its text */ - min-width: calc(100% - 8px); /* To ensure the span stays centered; also adjusts for scrollable-suggestion fade width. */ + min-width: calc(100% - 16px); /* To ensure the span stays centered. */ white-space: nowrap; } From f38af53681b329265c96ef09549cce2c709bb74d Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Tue, 16 Jan 2024 14:24:26 +0700 Subject: [PATCH 35/60] fix(web): rtl maintenance --- .../engine/osk/src/banner/bannerController.ts | 21 +++++++++++++++++++ .../engine/osk/src/banner/suggestionBanner.ts | 4 +++- web/src/engine/osk/src/views/oskView.ts | 4 +--- 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/web/src/engine/osk/src/banner/bannerController.ts b/web/src/engine/osk/src/banner/bannerController.ts index 45ee746bbf..70925268f5 100644 --- a/web/src/engine/osk/src/banner/bannerController.ts +++ b/web/src/engine/osk/src/banner/bannerController.ts @@ -6,6 +6,7 @@ import { BannerView } from './bannerView.js'; import { Banner } from './banner.js'; import { BlankBanner } from './blankBanner.js'; import { HTMLBanner } from './htmlBanner.js'; +import { Keyboard, KeyboardProperties } from '@keymanapp/keyboard-processor'; export class BannerController { private container: BannerView; @@ -16,6 +17,9 @@ export class BannerController { private _inactiveBanner: Banner; + private keyboard: Keyboard; + private keyboardStub: KeyboardProperties; + /** * Builds a banner for use when predictions are not active, supporting a single image. */ @@ -92,6 +96,23 @@ export class BannerController { selectBanner(state: StateChangeEnum) { // Only display a SuggestionBanner when LanguageProcessor states it is active. this.activateBanner(state == 'active' || state == 'configured'); + + if(this.keyboard) { + this.container.banner.configureForKeyboard(this.keyboard, this.keyboardStub); + } + } + + /** + * Allows banners to adapt based on the active keyboard and related properties, such as + * associated fonts. + * @param keyboard + * @param keyboardProperties + */ + public configureForKeyboard(keyboard: Keyboard, keyboardProperties: KeyboardProperties) { + this.keyboard = keyboard; + this.keyboardStub = keyboardProperties; + + this.container.banner.configureForKeyboard(keyboard, keyboardProperties); } public shutdown() { diff --git a/web/src/engine/osk/src/banner/suggestionBanner.ts b/web/src/engine/osk/src/banner/suggestionBanner.ts index 3f90f8943c..0a17935b9f 100644 --- a/web/src/engine/osk/src/banner/suggestionBanner.ts +++ b/web/src/engine/osk/src/banner/suggestionBanner.ts @@ -461,7 +461,9 @@ export class SuggestionBanner extends Banner { ds.marginRight = `calc(${(SuggestionBanner.MARGIN / 2)}% - 0.5px)`; this.container.appendChild(separatorDiv); - this.separators.push(separatorDiv); + // Ensure the separators are maintained in the same order as the + // suggestion elements! + this.separators[indexToInsert - (rtl ? 1 : 0)] = separatorDiv; } } } diff --git a/web/src/engine/osk/src/views/oskView.ts b/web/src/engine/osk/src/views/oskView.ts index 525985734a..b1d16d292f 100644 --- a/web/src/engine/osk/src/views/oskView.ts +++ b/web/src/engine/osk/src/views/oskView.ts @@ -729,9 +729,7 @@ export default abstract class OSKView // Add suggestion banner bar to OSK this._Box.appendChild(this.banner.element); - if(this.bannerView.banner) { - this.banner.banner.configureForKeyboard(this.keyboardData?.keyboard, this.keyboardData?.metadata); - } + this.bannerController?.configureForKeyboard(this.keyboardData?.keyboard, this.keyboardData?.metadata); let kbdView: KeyboardView = this.keyboardView = this._GenerateKeyboardView(this.keyboardData?.keyboard, this.keyboardData?.metadata); this._Box.appendChild(kbdView.element); From 39a4278b8bd9af7900051fc575749757b60f35a4 Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Tue, 16 Jan 2024 14:24:41 +0700 Subject: [PATCH 36/60] chore(web): unused method removal --- web/src/engine/osk/src/banner/suggestionBanner.ts | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/web/src/engine/osk/src/banner/suggestionBanner.ts b/web/src/engine/osk/src/banner/suggestionBanner.ts index 0a17935b9f..930b884c84 100644 --- a/web/src/engine/osk/src/banner/suggestionBanner.ts +++ b/web/src/engine/osk/src/banner/suggestionBanner.ts @@ -469,21 +469,6 @@ export class SuggestionBanner extends Banner { } private setupInputHandling(): GestureRecognizer { - - const findTargetFrom = (e: HTMLElement): HTMLDivElement => { - try { - if(e) { - if(e.classList.contains('kmw-suggest-option')) { - return e as HTMLDivElement; - } - if(e.parentElement && e.parentElement.classList.contains('kmw-suggest-option')) { - return e.parentElement as HTMLDivElement; - } - } - } catch(ex) {} - return null; - } - // Auto-cancels suggestion-selection if the finger moves too far; having very generous // safe-zone settings also helps keep scrolls active on demo pages, etc. const safeBounds = new PaddedZoneSource(this.getDiv(), [-Number.MAX_SAFE_INTEGER]); From 18a9924b1ba9eaac5db055ef1ba99c5eacbd5264 Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Tue, 16 Jan 2024 14:32:32 +0700 Subject: [PATCH 37/60] docs(web): PR doc-comment fixes --- web/src/engine/osk/src/banner/bannerScrollState.ts | 6 +----- web/src/engine/osk/src/banner/suggestionBanner.ts | 9 +++------ web/src/engine/osk/src/keyboard-layout/getTextMetrics.ts | 1 + 3 files changed, 5 insertions(+), 11 deletions(-) diff --git a/web/src/engine/osk/src/banner/bannerScrollState.ts b/web/src/engine/osk/src/banner/bannerScrollState.ts index bef4ba7d74..a0741ac6f5 100644 --- a/web/src/engine/osk/src/banner/bannerScrollState.ts +++ b/web/src/engine/osk/src/banner/bannerScrollState.ts @@ -1,7 +1,7 @@ import { InputSample } from "@keymanapp/gesture-recognizer"; /** - * The amount of coordinate 'noise' allowed during a scroll-enabled touch allowed + * The amount of coordinate 'noise' allowed during a scroll-enabled touch * before interpreting the currently-ongoing touch command as having scrolled. */ const HAS_SCROLLED_FUDGE_FACTOR = 10; @@ -20,10 +20,6 @@ export class BannerScrollState { curCoord: InputSample; baseScrollLeft: number; - // The amount of coordinate 'noise' allowed during a scroll-enabled touch allowed - // before interpreting the currently-ongoing touch command as having scrolled. - static readonly HAS_SCROLLED_FUDGE_FACTOR = 10; - constructor(coord: InputSample, baseScrollLeft: number) { this.baseCoord = coord; this.curCoord = coord; diff --git a/web/src/engine/osk/src/banner/suggestionBanner.ts b/web/src/engine/osk/src/banner/suggestionBanner.ts index 930b884c84..ff5d8298dc 100644 --- a/web/src/engine/osk/src/banner/suggestionBanner.ts +++ b/web/src/engine/osk/src/banner/suggestionBanner.ts @@ -149,13 +149,10 @@ export class BannerSuggestion { /** * Function update - * @param {string} id Element ID for the suggestion span * @param {Suggestion} suggestion Suggestion from the lexical model - * @param fontStyle The CSS styling expected for the suggestion text - * @param emSize The font size represented by 1em (in px, as from getComputedStyle on document.body) - * @param targetWidth - * @param collapsedTargetWidth - * Description Update the ID and text of the BannerSuggestionSpec + * @param {BannerSuggestionFormatSpec} format Formatting metadata to use for the Suggestion + * + * Update the ID and text of the BannerSuggestionSpec */ public update(suggestion: Suggestion, format: BannerSuggestionFormatSpec) { this._suggestion = suggestion; diff --git a/web/src/engine/osk/src/keyboard-layout/getTextMetrics.ts b/web/src/engine/osk/src/keyboard-layout/getTextMetrics.ts index d7856ec232..bb5db1cbb9 100644 --- a/web/src/engine/osk/src/keyboard-layout/getTextMetrics.ts +++ b/web/src/engine/osk/src/keyboard-layout/getTextMetrics.ts @@ -6,6 +6,7 @@ let metricsCanvas: HTMLCanvasElement; * Uses canvas.measureText to compute and return the width of the given text of given font in pixels. * * @param {String} text The text to be rendered. + * @param emScale The absolute `px` size expected to match `1em`. * @param {String} style The CSSStyleDeclaration for an element to measure against, without modification. * * @see https://stackoverflow.com/questions/118241/calculate-text-width-with-javascript/21015393#21015393 From 998f781d8f179767868c3ea5709b63ca9b08bf5b Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Tue, 16 Jan 2024 15:00:56 +0700 Subject: [PATCH 38/60] fix(web): banner scroll reset after selecting a suggestion --- web/src/engine/osk/src/banner/suggestionBanner.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/web/src/engine/osk/src/banner/suggestionBanner.ts b/web/src/engine/osk/src/banner/suggestionBanner.ts index ff5d8298dc..052ca63b2e 100644 --- a/web/src/engine/osk/src/banner/suggestionBanner.ts +++ b/web/src/engine/osk/src/banner/suggestionBanner.ts @@ -391,6 +391,8 @@ export class SuggestionBanner extends Banner { private options : BannerSuggestion[] = []; private separators: HTMLElement[] = []; + private isRTL: boolean = false; + private hostDevice: DeviceSpec; /** @@ -422,6 +424,7 @@ export class SuggestionBanner extends Banner { } buildInternals(rtl: boolean) { + this.isRTL = rtl; if(this.options.length > 0) { this.options = []; this.separators = []; @@ -603,7 +606,10 @@ export class SuggestionBanner extends Banner { sequence.once('stage', (result) => { const suggestion = result.item; // Should also == sourceTracker.suggestion. if(suggestion && !this.scrollState.hasScrolled) { - this.predictionContext.accept(suggestion.suggestion); + this.predictionContext.accept(suggestion.suggestion).then(() => { + // Reset the scroll state + this.container.scrollLeft = this.isRTL ? this.container.scrollWidth : 0; + }); } this.scrollState = null; From dfdb4fc9b3c581660401afd2b954cb5f3c071947 Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Wed, 17 Jan 2024 09:01:54 +0700 Subject: [PATCH 39/60] fix(web): if corrections not possible, returns empty Map instead of null --- web/src/engine/osk/src/visualKeyboard.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/src/engine/osk/src/visualKeyboard.ts b/web/src/engine/osk/src/visualKeyboard.ts index 2132cdca1a..0ef4494ee2 100644 --- a/web/src/engine/osk/src/visualKeyboard.ts +++ b/web/src/engine/osk/src/visualKeyboard.ts @@ -920,7 +920,7 @@ export default class VisualKeyboard extends EventEmitter implements Ke // Prevent NaN breakages. if (!width || !height) { - return null; + return new Map(); } let kbdAspectRatio = width / height; From 37bd9c09d64245121149856546de828d4b580e2b Mon Sep 17 00:00:00 2001 From: Darcy Wong Date: Wed, 17 Jan 2024 14:01:33 +0700 Subject: [PATCH 40/60] docs(android): Add additional changes --- android/help/about/whatsnew.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/android/help/about/whatsnew.md b/android/help/about/whatsnew.md index 71c47854fb..9e9dc7e117 100644 --- a/android/help/about/whatsnew.md +++ b/android/help/about/whatsnew.md @@ -3,5 +3,12 @@ title: What's New --- Here are some of the new features we have added to Keyman 17.0 for Android: +* New gesture support (#5029) * When suggestions aren't enabled, display a themed banner (#9696) * Smoother keyboard initialization (#10022) + +Additional changes: + +* Remove built-in browser (#8428) +* Use web-based popup key longpresses (#9591) +* Performance improvements From 51d5c87720488d01e37b338e1547206c46e1407c Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Wed, 17 Jan 2024 15:52:23 +0700 Subject: [PATCH 41/60] fix(web): keyboard-documentation rendering mode --- web/src/engine/osk/src/visualKeyboard.ts | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/web/src/engine/osk/src/visualKeyboard.ts b/web/src/engine/osk/src/visualKeyboard.ts index 79563beb75..3bfde96a92 100644 --- a/web/src/engine/osk/src/visualKeyboard.ts +++ b/web/src/engine/osk/src/visualKeyboard.ts @@ -1038,7 +1038,9 @@ export default class VisualKeyboard extends EventEmitter implements Ke return; } - this.gestureEngine.stateToken = layerId; + if(this.gestureEngine) { + this.gestureEngine.stateToken = layerId; + } // So... through KMW 14, we actually never tracked the capsKey, numKey, and scrollKey // properly for keyboard-defined layouts - only _default_, desktop-style layouts. @@ -1250,15 +1252,19 @@ export default class VisualKeyboard extends EventEmitter implements Ke return; } - // Step 3: perform layout operations. - const paddingZone = this.gestureEngine.config.maxRoamingBounds as PaddedZoneSource; - paddingZone.updatePadding([-0.333 * this.currentLayer.rowHeight]); + // Step 3: recalculate gesture parameter values + // Skip for doc-keyboards, since they don't do gestures. + if(!this.isStatic) { + const paddingZone = this.gestureEngine.config.maxRoamingBounds as PaddedZoneSource; + paddingZone.updatePadding([-0.333 * this.currentLayer.rowHeight]); - this.gestureParams.longpress.flickDist = 0.25 * this.currentLayer.rowHeight; - this.gestureParams.flick.startDist = 0.15 * this.currentLayer.rowHeight; - this.gestureParams.flick.dirLockDist = 0.35 * this.currentLayer.rowHeight; - this.gestureParams.flick.triggerDist = 0.75 * this.currentLayer.rowHeight; + this.gestureParams.longpress.flickDist = 0.25 * this.currentLayer.rowHeight; + this.gestureParams.flick.startDist = 0.15 * this.currentLayer.rowHeight; + this.gestureParams.flick.dirLockDist = 0.35 * this.currentLayer.rowHeight; + this.gestureParams.flick.triggerDist = 0.75 * this.currentLayer.rowHeight; + } + // Step 4: perform layout operations. // Needs the refreshed layout info to work correctly. if(this.currentLayer) { this.currentLayer.refreshLayout(this, this._computedHeight - this.getVerticalLayerGroupPadding()); From f759ecef76fdfc5841482927f417ec44756180a6 Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Wed, 17 Jan 2024 15:59:48 +0700 Subject: [PATCH 42/60] fix(ios): replicates TextView fix in TextField --- ios/engine/KMEI/KeymanEngine/Classes/TextField.swift | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/ios/engine/KMEI/KeymanEngine/Classes/TextField.swift b/ios/engine/KMEI/KeymanEngine/Classes/TextField.swift index 42f8ea8297..3aa871f267 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/TextField.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/TextField.swift @@ -146,10 +146,6 @@ public class TextField: UITextField, KeymanResponder { font = UIFont.systemFont(ofSize: fontSize) } - if isFirstResponder { - resignFirstResponder() - becomeFirstResponder() - } log.debug("TextField \(self.hashValue) setFont: \(font?.familyName ?? "nil")") } @@ -179,7 +175,7 @@ extension KeymanResponder where Self: TextField { resignFirstResponder() Manager.shared.inputViewController.endEditing(true) } - + public func summonKeyboard() { becomeFirstResponder() } From 664171572edefc29c9790eaa55cc2960ffc9bd3e Mon Sep 17 00:00:00 2001 From: Keyman Build Agent Date: Wed, 17 Jan 2024 13:03:12 -0500 Subject: [PATCH 43/60] auto: increment master version to 17.0.247 --- HISTORY.md | 8 ++++++++ VERSION.md | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/HISTORY.md b/HISTORY.md index f3c383d7b3..547d3eb40e 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -1,5 +1,13 @@ # Keyman Version History +## 17.0.246 alpha 2024-01-17 + +* fix(web): Add null check for changing the keyboard during typing (#10346) +* chore(common): Update crowdin strings for Khmer (#10411) +* docs(common): Update website README.md (#10399) +* docs(linux): Add documentation for Core API verification (#10409) +* fix(web): right-flick gesture-preview positioning (#10406) + ## 17.0.245 alpha 2024-01-16 * chore(core): Ignore C++ symbols (#10386) diff --git a/VERSION.md b/VERSION.md index 8c12aa78ac..87801d9d2a 100644 --- a/VERSION.md +++ b/VERSION.md @@ -1 +1 @@ -17.0.246 \ No newline at end of file +17.0.247 \ No newline at end of file From 763bc967399e0e0226c610c34fe34c50ffaed113 Mon Sep 17 00:00:00 2001 From: "Steven R. Loomis" Date: Wed, 17 Jan 2024 18:05:28 -0600 Subject: [PATCH 44/60] =?UTF-8?q?feat(core):=20ldml=20updates=20per=20code?= =?UTF-8?q?=20review=20=F0=9F=99=80?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - remove stray change to kmx_processevent.cpp - fix escaping of syntax chars - fix comment per review For: #10319 --- common/web/types/src/util/util.ts | 2 +- core/src/kmx/kmx_processevent.cpp | 2 +- core/src/ldml/ldml_markers.cpp | 3 ++- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/common/web/types/src/util/util.ts b/common/web/types/src/util/util.ts index 712efe8246..d86bd6f58f 100644 --- a/common/web/types/src/util/util.ts +++ b/common/web/types/src/util/util.ts @@ -127,7 +127,7 @@ function escapeRegexChar(ch: string) { } /** chars that must be escaped: syntax, C0 + C1 controls */ -const REGEX_SYNTAX_CHAR = /^[\u0000-\u001F\u007F-\u009F{}\[\]\\?.^$*-]$/; +const REGEX_SYNTAX_CHAR = /^[\u0000-\u001F\u007F-\u009F{}\[\]\\?.^$*()/-]$/; function escapeRegexCharIfSyntax(ch: string) { // escape if syntax or not valid diff --git a/core/src/kmx/kmx_processevent.cpp b/core/src/kmx/kmx_processevent.cpp index f298d87cd8..4b98312c36 100644 --- a/core/src/kmx/kmx_processevent.cpp +++ b/core/src/kmx/kmx_processevent.cpp @@ -11,7 +11,7 @@ using namespace kmx; /* Globals */ -KMX_BOOL km::core::kmx::g_debug_ToConsole = TRUE; +KMX_BOOL km::core::kmx::g_debug_ToConsole = FALSE; KMX_BOOL km::core::kmx::g_debug_KeymanLog = TRUE; KMX_BOOL km::core::kmx::g_silent = FALSE; diff --git a/core/src/ldml/ldml_markers.cpp b/core/src/ldml/ldml_markers.cpp index 1eb3bcf743..9e44c3d40b 100644 --- a/core/src/ldml/ldml_markers.cpp +++ b/core/src/ldml/ldml_markers.cpp @@ -19,7 +19,8 @@ namespace core { namespace ldml { -const std::u32string REGEX_PREFIX = U"\\uffff\\u0008"; // does not include '\\' because it might be a range +// the 'prefix part' of a regex marker sequence, followed by RAW_PREFIX or REGEX_ANY_MATCH +const std::u32string REGEX_PREFIX = U"\\uffff\\u0008"; const std::u32string RAW_PREFIX = U"\uffff\u0008"; const std::u32string REGEX_ANY_MATCH = U"[\\u0001-\\ud7fe]"; From d08e7051cff076bbbe4a2d5665e488fff4de7389 Mon Sep 17 00:00:00 2001 From: "Steven R. Loomis" Date: Wed, 17 Jan 2024 18:34:56 -0600 Subject: [PATCH 45/60] =?UTF-8?q?feat(core):=20ldml=20updates=20per=20code?= =?UTF-8?q?=20review=20=F0=9F=99=80?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - don't try to remove irregular sentinel sequences - update test For: #10319 --- core/src/ldml/ldml_markers.cpp | 8 -------- core/tests/unit/ldml/test_transforms.cpp | 6 +++--- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/core/src/ldml/ldml_markers.cpp b/core/src/ldml/ldml_markers.cpp index 9e44c3d40b..f4ab662ee9 100644 --- a/core/src/ldml/ldml_markers.cpp +++ b/core/src/ldml/ldml_markers.cpp @@ -295,10 +295,6 @@ std::u32string remove_markers(const std::u32string &str, marker_map *markers, ma if (rest.length() <= lookfor_str.length()) { // not enough left so it can't match, so continue i = str.end(); // end of string - if (encoding == plain_sentinel) { - // in plain mode, delete any irregular sequences - last = i; - } continue; } @@ -308,10 +304,6 @@ std::u32string remove_markers(const std::u32string &str, marker_map *markers, ma if (rest != lookfor_str) { // no match - could be backslash something else - if (encoding == plain_sentinel) { - // in plain mode, delete any irregular sequences - last = i; - } continue; } diff --git a/core/tests/unit/ldml/test_transforms.cpp b/core/tests/unit/ldml/test_transforms.cpp index b4d3a3e07a..bd1a6b77d6 100644 --- a/core/tests/unit/ldml/test_transforms.cpp +++ b/core/tests/unit/ldml/test_transforms.cpp @@ -647,7 +647,7 @@ int test_strutils() { std::cout << __FILE__ << ":" << __LINE__ << " - bad1" << std::endl; const std::u32string src = U"6\U0000ffffq"; // missing sentinel subtype const std::u32string dst = remove_markers(src, map); - const std::u32string expect = U"6"; // 'q' removed + const std::u32string expect = src; // 'q' removed zassert_string_equal(dst, expect); assert_equal(map.size(), 0); } @@ -656,7 +656,7 @@ int test_strutils() { std::cout << __FILE__ << ":" << __LINE__ << " - bad1b" << std::endl; const std::u32string src = U"6\U0000ffff"; // missing code const std::u32string dst = remove_markers(src, map); - const std::u32string expect = U"6"; + const std::u32string expect = src; zassert_string_equal(dst, expect); assert_equal(map.size(), 0); } @@ -665,7 +665,7 @@ int test_strutils() { std::cout << __FILE__ << ":" << __LINE__ << " - bad1c" << std::endl; const std::u32string src = U"6\U0000ffffzz"; // missing code const std::u32string dst = remove_markers(src, map); - const std::u32string expect = U"6z"; + const std::u32string expect = src; zassert_string_equal(dst, expect); assert_equal(map.size(), 0); } From b6a6b7e6b8e800b0dcadb2261422583df1a9782c Mon Sep 17 00:00:00 2001 From: "Steven R. Loomis" Date: Wed, 17 Jan 2024 18:32:39 -0600 Subject: [PATCH 46/60] =?UTF-8?q?feat(core):=20ldml=20fix=20test=20case=20?= =?UTF-8?q?and=20parser=20=F0=9F=99=80?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For: #10319 --- core/src/ldml/ldml_markers.cpp | 5 ++--- core/tests/unit/ldml/test_transforms.cpp | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/core/src/ldml/ldml_markers.cpp b/core/src/ldml/ldml_markers.cpp index f4ab662ee9..fdd34f23a4 100644 --- a/core/src/ldml/ldml_markers.cpp +++ b/core/src/ldml/ldml_markers.cpp @@ -289,7 +289,7 @@ std::u32string remove_markers(const std::u32string &str, marker_map *markers, ma } out.append(last, i); assert(*i == lookfor); // assert that find() worked - last = i; // keep track of the last segment appendd. + last = i; // keep track of the last segment appended. std::u32string rest(i, str.end()); if (rest.length() <= lookfor_str.length()) { @@ -307,8 +307,7 @@ std::u32string remove_markers(const std::u32string &str, marker_map *markers, ma continue; } - // matches. Skip over the prefix - last = i; + assert(i != str.end()); // caught above KMX_DWORD marker_no; if (encoding == plain_sentinel) { diff --git a/core/tests/unit/ldml/test_transforms.cpp b/core/tests/unit/ldml/test_transforms.cpp index bd1a6b77d6..484903af56 100644 --- a/core/tests/unit/ldml/test_transforms.cpp +++ b/core/tests/unit/ldml/test_transforms.cpp @@ -638,7 +638,7 @@ int test_strutils() { std::cout << __FILE__ << ":" << __LINE__ << " - bad0" << std::endl; const std::u32string src = U"6\U0000ffff\U00000008"; // missing trailing marker # const std::u32string dst = remove_markers(src, map); - const std::u32string expect = U"6"; + const std::u32string expect = src; zassert_string_equal(dst, expect); assert_equal(map.size(), 0); } From 990635eb09d9d688eb28b4f9c40966ec1596de4c Mon Sep 17 00:00:00 2001 From: "Steven R. Loomis" Date: Mon, 15 Jan 2024 20:21:18 -0600 Subject: [PATCH 47/60] =?UTF-8?q?feat(core):=20WIP=20attempt=20at=20cross?= =?UTF-8?q?=20segment=20markers=20=F0=9F=99=80?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #10369 --- core/src/ldml/ldml_markers.cpp | 37 +++---- core/src/ldml/ldml_markers.hpp | 9 +- core/tests/unit/ldml/test_transforms.cpp | 119 ++++++++++++++--------- 3 files changed, 93 insertions(+), 72 deletions(-) diff --git a/core/src/ldml/ldml_markers.cpp b/core/src/ldml/ldml_markers.cpp index fdd34f23a4..2d7d354f86 100644 --- a/core/src/ldml/ldml_markers.cpp +++ b/core/src/ldml/ldml_markers.cpp @@ -71,30 +71,24 @@ static void add_back_markers(std::u32string &str, const std::u32string &src, con marker_map map2(map); // make a copy of the map // clear the string str.clear(); - // add the end-of-text marker - { - const auto ch = MARKER_BEFORE_EOT; - const auto m = map2.find(ch); - if (m != map2.end()) { - for (auto q = (m->second).rbegin(); q < (m->second).rend(); q++) { - prepend_marker(str, *q, encoding); - } - map2.erase(ch); // remove it - } + // iterator + auto marki = map.rbegin(); + + // add any end-of-text markers + while(marki != map.rend() && marki->first == MARKER_BEFORE_EOT) { + prepend_marker(str, (marki++)->second, encoding); } + // go from end to beginning of string for (auto p = src.rbegin(); p != src.rend(); p++) { const auto ch = *p; str.insert(0, 1, ch); // prepend - const auto m = map2.find(ch); - if (m != map2.end()) { - for (auto q = (m->second).rbegin(); q < (m->second).rend(); q++) { - prepend_marker(str, *q, encoding); - } - map2.erase(ch); // remove it + while(marki != map.rend() && marki->first == ch) { + prepend_marker(str, (marki++)->second, encoding); } } +// assert(marki == map.rend()); // that we consumed everything } /** @@ -237,14 +231,9 @@ KMX_DWORD parse_hex_quad(const km_core_usv hex_str[]) { /** add the list to the map */ static void add_markers_to_map(marker_map &markers, char32_t marker_ch, const marker_list &list) { - auto rep = markers.emplace(marker_ch, list); - if (!rep.second) { - // already existed. - auto existing = rep.first; - // append all additional ones - for(auto m = list.begin(); m < list.end(); m++) { - existing->second.emplace_back(*m); - } + for (auto i = list.begin(); i < list.end(); i++) { + // marker_ch is duplicate, but keeps the structure more shallow. + markers.emplace_back(marker_ch, *i); } } diff --git a/core/src/ldml/ldml_markers.hpp b/core/src/ldml/ldml_markers.hpp index 68c7aa2f2d..8375543f85 100644 --- a/core/src/ldml/ldml_markers.hpp +++ b/core/src/ldml/ldml_markers.hpp @@ -48,11 +48,14 @@ enum marker_encoding { /** a marker ID (1-based) */ typedef KMX_DWORD marker_num; -/** list of markers */ +/** list of marker numbers */ typedef std::deque marker_list; -/** map from following-char to marker numbers. */ -typedef std::map marker_map; +/** map from one char to one entry */ +typedef std::pair marker_entry; + +/** map from following-char to marker numbers, in front to back order */ +typedef std::deque marker_map; /** Normalize a u32string inplace to NFD. @return false on failure */ bool normalize_nfd(std::u32string &str); diff --git a/core/tests/unit/ldml/test_transforms.cpp b/core/tests/unit/ldml/test_transforms.cpp index 484903af56..4f09d9af4e 100644 --- a/core/tests/unit/ldml/test_transforms.cpp +++ b/core/tests/unit/ldml/test_transforms.cpp @@ -41,26 +41,46 @@ using namespace km::core::ldml; using namespace km::core::kmx; -std::u32string marker_list_to_string(const marker_list &m) { +void +prepend_hex_oct(std::u32string &str, char32_t x) { + for (auto i = 0; i < 8; i++) { + KMX_DWORD remainder = x & 0xF; // get the last nibble + char32_t ch; + if (remainder < 0xA) { + ch = U'0' + remainder; + } else { + ch = U'A' + (remainder - 0xA); + } + str.insert(0, 1, ch); // prepend + x >>= 4; + } +} + + +std::u32string marker_map_to_string(const marker_map &m) { std::u32string s; for (auto i = m.rbegin(); i < m.rend(); i++) { - prepend_hex_quad(s, *i); + + prepend_hex_oct(s, i->first); + s.insert(0, U"=U+"); + + prepend_hex_quad(s, i->second); s.insert(0, U" \\m0x"); } return s; } -bool _assert_marker_list_equal(const char *f, int l, const marker_list a, const marker_list x) { +bool _assert_marker_map_equal(const char *f, int l, const marker_map a, const marker_map x) { if (a == x) return true; std::wcerr << f << ":" << l << ": " << console_color::fg(console_color::BRIGHT_RED); - std::wcerr << "got: " << marker_list_to_string(a); - std::wcerr << " expected: " << marker_list_to_string(x); + std::wcerr << "got: " << marker_map_to_string(a); + std::wcerr << " expected: " << marker_map_to_string(x); std::wcerr << console_color::reset() << std::endl; return false; } -#define assert_marker_list_equal(actual, expected) \ - if (!_assert_marker_list_equal(__FILE__, __LINE__, (actual), (expected))) \ +#define assert_marker_map_equal(actual, expected) \ + if (!_assert_marker_map_equal(__FILE__, __LINE__, (actual), (expected))) \ return EXIT_FAILURE; // using km::core::kmx::u16cmp; @@ -629,9 +649,9 @@ int test_strutils() { const std::u32string dst = remove_markers(src, map); const std::u32string expect = U"6e"; zassert_string_equal(dst, expect); + marker_map expm = { {U'e', 0x1L} }; + assert_marker_map_equal(map, expm); // marker 1 @ e assert_equal(map.size(), 1); - marker_list exp_e = { 0x1L }; - assert_marker_list_equal(map[U'e'], exp_e); // marker 1 @ e } { marker_map map; @@ -676,9 +696,9 @@ int test_strutils() { const std::u32string dst = remove_markers(src, map); const std::u32string expect = U"6"; zassert_string_equal(dst, expect); + marker_map expm({{MARKER_BEFORE_EOT, 0x1L}}); + assert_marker_map_equal(map, expm); assert_equal(map.size(), 1); - marker_list exp_end = { 0x1L }; - assert_marker_list_equal(map[MARKER_BEFORE_EOT], exp_end); // marker 1 @ e } { marker_map map; @@ -687,15 +707,9 @@ int test_strutils() { const std::u32string dst = remove_markers(src, map); const std::u32string expect = U"6e\U00000320\U00000300"; zassert_string_equal(dst, expect); + marker_map expm({{U'e', 0x1L}, {0x0320, 0x2L}, {0x0300, 0x3L}, {MARKER_BEFORE_EOT, 0x4L}}); + assert_marker_map_equal(map, expm); assert_equal(map.size(), 4); - marker_list exp_e = { 0x1L }; - assert_marker_list_equal(map[U'e'], exp_e); - marker_list exp_320 = { 0x2L }; - assert_marker_list_equal(map[0x0320], exp_320); - marker_list exp_300 = { 0x3L }; - assert_marker_list_equal(map[0x0300], exp_300); - marker_list exp_end = { 0x4L }; - assert_marker_list_equal(map[MARKER_BEFORE_EOT], exp_end); } { std::cout << __FILE__ << ":" << __LINE__ << " - prepend hex quad" << std::endl; @@ -763,11 +777,9 @@ int test_normalize() { std::u32string dst = src; assert(normalize_nfd_markers_segment(dst, map)); zassert_string_equal(dst, expect); + marker_map expm({{U'e', 0x1L}, {0x320, 0x2L}, {0x300, 0x3L}, {MARKER_BEFORE_EOT, 0x4L}}); + assert_marker_map_equal(map, expm); assert_equal(map.size(), 4); - assert_marker_list_equal(map[U'e'], marker_list({0x1L})); - assert_marker_list_equal(map[0x0320], marker_list({0x2L})); - assert_marker_list_equal(map[0x0300], marker_list({0x3L})); - assert_marker_list_equal(map[MARKER_BEFORE_EOT],marker_list({0x4L})); } { @@ -779,11 +791,9 @@ int test_normalize() { std::u32string dst = src; assert(normalize_nfd_markers_segment(dst, map)); zassert_string_equal(dst, expect); + marker_map expm({{U'e', 0x1L}, {0x320, 0x2L}, {0x300, 0x3L}, {MARKER_BEFORE_EOT, 0x4L}}); + assert_marker_map_equal(map, expm); assert_equal(map.size(), 4); - assert_marker_list_equal(map[U'e'], marker_list({0x1L})); - assert_marker_list_equal(map[0x0320], marker_list({0x2L})); - assert_marker_list_equal(map[0x0300], marker_list({0x3L})); - assert_marker_list_equal(map[MARKER_BEFORE_EOT], marker_list({0x4L})); } { marker_map map; @@ -799,11 +809,9 @@ int test_normalize() { std::cout << "exp: " << Debug_UnicodeString(expect) << std::endl; } zassert_string_equal(dst, expect); + marker_map expm({{U'e', 0x1L}, {0x320, 0x3L}, {0x300, 0x2L}, {MARKER_BEFORE_EOT, 0x4L}}); + assert_marker_map_equal(map, expm); assert_equal(map.size(), 4); - assert_marker_list_equal(map[U'e'], marker_list({0x1L})); - assert_marker_list_equal(map[0x0320], marker_list({0x3L})); - assert_marker_list_equal(map[0x0300], marker_list({0x2L})); - assert_marker_list_equal(map[MARKER_BEFORE_EOT], marker_list({0x4L})); } { @@ -819,8 +827,9 @@ int test_normalize() { std::cout << "exp: " << Debug_UnicodeString(expect) << std::endl; } zassert_string_equal(dst, expect); + marker_map expm({{0x320, 0x1L}}); + assert_marker_map_equal(map, expm); assert_equal(map.size(), 1); - assert_marker_list_equal(map[0x0320], marker_list({0x1L})); } { @@ -836,9 +845,9 @@ int test_normalize() { std::cout << "exp: " << Debug_UnicodeString(expect) << std::endl; } zassert_string_equal(dst, expect); + marker_map expm({{0x320, 0x2L},{MARKER_BEFORE_EOT, 0x1L}}); + assert_marker_map_equal(map, expm); assert_equal(map.size(), 2); - assert_marker_list_equal(map[0x0320], marker_list({0x2L})); - assert_marker_list_equal(map[MARKER_BEFORE_EOT], marker_list({0x1L})); } { @@ -856,9 +865,9 @@ int test_normalize() { std::cout << " " << expect << std::endl; } zassert_string_equal(dst, expect); + marker_map expm({{0x320, 0x2L},{MARKER_BEFORE_EOT, 0x1L}}); + assert_marker_map_equal(map, expm); assert_equal(map.size(), 2); - assert_marker_list_equal(map[0x0320], marker_list({0x2L})); - assert_marker_list_equal(map[MARKER_BEFORE_EOT], marker_list({0x1L})); } { // from tests - regex edition @@ -873,9 +882,9 @@ int test_normalize() { std::cout << "exp: " << Debug_UnicodeString(expect) << std::endl; } zassert_string_equal(dst, expect); + marker_map expm({{0x320, LDML_MARKER_ANY_INDEX},{MARKER_BEFORE_EOT, 0x1L}}); + assert_marker_map_equal(map, expm); assert_equal(map.size(), 2); - assert_marker_list_equal(map[0x0320], marker_list({LDML_MARKER_ANY_INDEX})); - assert_marker_list_equal(map[MARKER_BEFORE_EOT], marker_list({0x1L})); } { @@ -890,9 +899,10 @@ int test_normalize() { std::cout << "dst: " << Debug_UnicodeString(dst) << std::endl; std::cout << "exp: " << Debug_UnicodeString(expect) << std::endl; } - assert_equal(map.size(), 1); - assert_marker_list_equal(map[0x0320], (marker_list({0x2L, 0x2L}))); zassert_string_equal(dst, expect); + marker_map expm({{0x320, 0x2L}, {0x320, 0x2L}}); + assert_marker_map_equal(map, expm); + assert_equal(map.size(), 2); } @@ -909,8 +919,8 @@ int test_normalize() { std::cout << "exp: " << Debug_UnicodeString(expect) << std::endl; } zassert_string_equal(dst, expect); - assert_equal(map.size(), 1); - assert_marker_list_equal(map[0x0320], (marker_list({0x2L, 0x1L, 0x3L}))); + marker_map expm({{0x320, 0x2L}, {0x320, 0x1L}, {0x320, 0x3L}}); + assert_marker_map_equal(map, expm); } @@ -921,11 +931,30 @@ int test_normalize() { const std::u32string dst = remove_markers(src, map); const std::u32string expect = U"a\u0300e\u0300"; // U+0300 twice! This should be removed in 2 segments zassert_string_equal(dst, expect); - assert_equal(map.size(), 1); - marker_list exp_ae = { 0x1L, 0x2L }; // Not what the user would see in practice. - assert_marker_list_equal(map[0x0300], exp_ae); // marker 1 @ e + marker_map expm({{0x300, 0x1L}, {0x300, 0x2L}}); + assert_marker_map_equal(map, expm); } + { + // from tests + marker_map map; + std::cout << __FILE__ << ":" << __LINE__ << " - support 2-segment markers " << std::endl; + // e\m{1}`\m{2}_E\m{3}`\m{4}_ + const std::u32string src = U"e\uFFFF\u0008\u0001\u0300\uFFFF\u0008\u0002\u0320E\uFFFF\u0008\u0003\u0300\uFFFF\u0008\u0004\u0320"; + // e\m{2}_\m{1}`E\m{4}_\m{3}` + const std::u32string expect = U"e\uFFFF\u0008\u0002\u0320\uFFFF\u0008\u0001\u0300E\uFFFF\u0008\u0004\u0320\uFFFF\u0008\u0003\u0300"; + std::u32string dst = src; + assert(normalize_nfd_markers_segment(dst, map)); + if (dst != expect) { + std::cout << "dst: " << Debug_UnicodeString(dst) << std::endl; + std::cout << "exp: " << Debug_UnicodeString(expect) << std::endl; + } + zassert_string_equal(dst, expect); + marker_map expm({{0x300, 0x1L}, {0x320, 0x2L}, {0x300, 0x3L}, {0x320, 0x4L}}); + assert_marker_map_equal(map, expm); + } + + return EXIT_SUCCESS; } From 2e46bfea81933c2c89b0ad74578438fb3cd69452 Mon Sep 17 00:00:00 2001 From: "Steven R. Loomis" Date: Mon, 15 Jan 2024 20:22:01 -0600 Subject: [PATCH 48/60] chore: reformat --- core/tests/unit/ldml/test_transforms.cpp | 219 ++++++++++++----------- 1 file changed, 111 insertions(+), 108 deletions(-) diff --git a/core/tests/unit/ldml/test_transforms.cpp b/core/tests/unit/ldml/test_transforms.cpp index 4f09d9af4e..0526fdb21d 100644 --- a/core/tests/unit/ldml/test_transforms.cpp +++ b/core/tests/unit/ldml/test_transforms.cpp @@ -1,11 +1,11 @@ -#include "../../../src/ldml/ldml_transforms.hpp" #include "../../../src/ldml/ldml_markers.hpp" +#include "../../../src/ldml/ldml_transforms.hpp" #include "kmx/kmx_plus.h" #include "kmx/kmx_xstring.h" +#include "test_color.h" #include #include #include -#include "test_color.h" // TODO-LDML: normal asserts wern't working, so using some hacks. // #include "ldml_test_utils.hpp" @@ -13,18 +13,19 @@ // #include "debuglog.h" #ifndef zassert_string_equal -#define zassert_string_equal(actual, expected) \ - { \ - if (actual != expected) { \ - std::wcerr << __FILE__ << ":" << __LINE__ << ": " << console_color::fg(console_color::BRIGHT_RED) << "got: " << km::core::kmx::Debug_UnicodeString(actual, 0) \ - << " expected " << km::core::kmx::Debug_UnicodeString(expected, 1) << console_color::reset() << std::endl; \ - return EXIT_FAILURE; \ - } \ +#define zassert_string_equal(actual, expected) \ + { \ + if (actual != expected) { \ + std::wcerr << __FILE__ << ":" << __LINE__ << ": " << console_color::fg(console_color::BRIGHT_RED) \ + << "got: " << km::core::kmx::Debug_UnicodeString(actual, 0) << " expected " \ + << km::core::kmx::Debug_UnicodeString(expected, 1) << console_color::reset() << std::endl; \ + return EXIT_FAILURE; \ + } \ } #endif #ifndef zassert_equal -#define zassert_equal(actual, expected) \ +#define zassert_equal(actual, expected) \ { \ if (actual != expected) { \ std::wcerr << __FILE__ << ":" << __LINE__ << ": " << console_color::fg(console_color::BRIGHT_RED) << "got: " << actual \ @@ -34,7 +35,6 @@ } #endif - // needed for streaming operators #include "utfcodec.hpp" @@ -44,23 +44,22 @@ using namespace km::core::kmx; void prepend_hex_oct(std::u32string &str, char32_t x) { for (auto i = 0; i < 8; i++) { - KMX_DWORD remainder = x & 0xF; // get the last nibble + KMX_DWORD remainder = x & 0xF; // get the last nibble char32_t ch; if (remainder < 0xA) { ch = U'0' + remainder; } else { ch = U'A' + (remainder - 0xA); } - str.insert(0, 1, ch); // prepend + str.insert(0, 1, ch); // prepend x >>= 4; } } - -std::u32string marker_map_to_string(const marker_map &m) { +std::u32string +marker_map_to_string(const marker_map &m) { std::u32string s; for (auto i = m.rbegin(); i < m.rend(); i++) { - prepend_hex_oct(s, i->first); s.insert(0, U"=U+"); @@ -70,8 +69,10 @@ std::u32string marker_map_to_string(const marker_map &m) { return s; } -bool _assert_marker_map_equal(const char *f, int l, const marker_map a, const marker_map x) { - if (a == x) return true; +bool +_assert_marker_map_equal(const char *f, int l, const marker_map a, const marker_map x) { + if (a == x) + return true; std::wcerr << f << ":" << l << ": " << console_color::fg(console_color::BRIGHT_RED); std::wcerr << "got: " << marker_map_to_string(a); std::wcerr << " expected: " << marker_map_to_string(x); @@ -242,7 +243,7 @@ test_reorder_standalone() { std::cout << __FILE__ << ":" << __LINE__ << " - element API test " << std::endl; // element test { - element es(U'a', (80 << LDML_ELEM_FLAGS_ORDER_BITSHIFT) | LDML_ELEM_FLAGS_PREBASE); // tertiary -12, primary 80 + element es(U'a', (80 << LDML_ELEM_FLAGS_ORDER_BITSHIFT) | LDML_ELEM_FLAGS_PREBASE); // tertiary -12, primary 80 std::cout << "es flags" << std::hex << es.get_flags() << std::dec << std::endl; // verify element metadata assert_equal(es.is_uset(), false); @@ -406,19 +407,19 @@ test_reorder_standalone() { std::cout << __FILE__ << ":" << __LINE__ << " - trying roast #" << r << "=" << roast << std::endl; // try apply with string { - std::cout << "- try apply(text, output)" << std::endl; - std::u32string text = roast; - std::u32string output; - size_t len = tr.apply(text, output); - if (len == 0) { - std::cout << " (did not apply)" << std::endl; - } else { - std::cout << " applied, matchLen= " << len << std::endl; - text.resize(text.size()-len); // shrink - text.append(output); - std::cout << " = " << text << std::endl; - } - zassert_string_equal(text, expect); + std::cout << "- try apply(text, output)" << std::endl; + std::u32string text = roast; + std::u32string output; + size_t len = tr.apply(text, output); + if (len == 0) { + std::cout << " (did not apply)" << std::endl; + } else { + std::cout << " applied, matchLen= " << len << std::endl; + text.resize(text.size() - len); // shrink + text.append(output); + std::cout << " = " << text << std::endl; + } + zassert_string_equal(text, expect); } // try all-at-once { @@ -455,7 +456,7 @@ test_reorder_standalone() { // special test { std::cout << __FILE__ << ":" << __LINE__ << " - special test " << std::endl; - const std::u32string expect = U"\u1A21\u1A60\u1A45"; // this string shouldn't mutate at all. + const std::u32string expect = U"\u1A21\u1A60\u1A45"; // this string shouldn't mutate at all. { std::u32string text = expect; tr.apply(text); @@ -474,7 +475,6 @@ test_reorder_standalone() { return EXIT_SUCCESS; } - // this test case is also in XML form under 'k_201_*' int test_reorder_esk() { @@ -536,34 +536,34 @@ test_reorder_esk() { // now actually test it std::cout << __FILE__ << ":" << __LINE__ << " - cases " << std::endl; const std::u32string orig_expect[] = { - // 1short - U"ax\u0305", // orig - U"a\u0305x", // expect + // 1short + U"ax\u0305", // orig + U"a\u0305x", // expect - // 2longer - U"az\u0305x\u0332", // orig - U"a\u0332\u0305xz", // expect + // 2longer + U"az\u0305x\u0332", // orig + U"a\u0332\u0305xz", // expect }; // TODO-LDML: move this into test code perhaps - for (size_t r = 0; r < sizeof(orig_expect) / sizeof(orig_expect[0]); r+= 2) { + for (size_t r = 0; r < sizeof(orig_expect) / sizeof(orig_expect[0]); r += 2) { const auto &orig = orig_expect[r + 0]; const auto &expect = orig_expect[r + 1]; - std::cout << __FILE__ << ":" << __LINE__ << " - trying str #" << r+1 << "=" << orig << std::endl; + std::cout << __FILE__ << ":" << __LINE__ << " - trying str #" << r + 1 << "=" << orig << std::endl; // try apply with string { - std::cout << "- try apply(text, output)" << std::endl; - std::u32string text = orig; - std::u32string output; - size_t len = tr.apply(text, output); - if (len == 0) { - std::cout << " (did not apply)" << std::endl; - } else { - std::cout << " applied, matchLen= " << len << std::endl; - text.resize(text.size()-len); // shrink - text.append(output); - std::cout << " = " << text << std::endl; - } - zassert_string_equal(text, expect); + std::cout << "- try apply(text, output)" << std::endl; + std::u32string text = orig; + std::u32string output; + size_t len = tr.apply(text, output); + if (len == 0) { + std::cout << " (did not apply)" << std::endl; + } else { + std::cout << " applied, matchLen= " << len << std::endl; + text.resize(text.size() - len); // shrink + text.append(output); + std::cout << " = " << text << std::endl; + } + zassert_string_equal(text, expect); } // try all-at-once { @@ -601,7 +601,8 @@ test_reorder_esk() { return EXIT_SUCCESS; } -int test_map() { +int +test_map() { std::cout << "== " << __FUNCTION__ << std::endl; std::cout << __FILE__ << ":" << __LINE__ << " transform_entry::findIndex" << std::endl; @@ -622,42 +623,42 @@ int test_map() { return EXIT_SUCCESS; } -int test_strutils() { +int +test_strutils() { std::cout << "== " << __FUNCTION__ << std::endl; std::cout << __FILE__ << ":" << __LINE__ << " * remove_markers" << std::endl; - { std::cout << __FILE__ << ":" << __LINE__ << " - basic test0" << std::endl; const std::u32string src = U"abc"; const std::u32string dst = remove_markers(src); - zassert_string_equal(dst, src); // unchanged + zassert_string_equal(dst, src); // unchanged } { marker_map map; std::cout << __FILE__ << ":" << __LINE__ << " - basic test" << std::endl; const std::u32string src = U"abc"; const std::u32string dst = remove_markers(src, map); - zassert_string_equal(dst, src); // unchanged + zassert_string_equal(dst, src); // unchanged assert_equal(map.size(), 0); } { marker_map map; std::cout << __FILE__ << ":" << __LINE__ << " - marker test" << std::endl; - const std::u32string src = U"6\U0000ffff\U00000008\U00000001e"; - const std::u32string dst = remove_markers(src, map); + const std::u32string src = U"6\U0000ffff\U00000008\U00000001e"; + const std::u32string dst = remove_markers(src, map); const std::u32string expect = U"6e"; zassert_string_equal(dst, expect); - marker_map expm = { {U'e', 0x1L} }; - assert_marker_map_equal(map, expm); // marker 1 @ e + marker_map expm = {{U'e', 0x1L}}; + assert_marker_map_equal(map, expm); // marker 1 @ e assert_equal(map.size(), 1); } { marker_map map; std::cout << __FILE__ << ":" << __LINE__ << " - bad0" << std::endl; - const std::u32string src = U"6\U0000ffff\U00000008"; // missing trailing marker # - const std::u32string dst = remove_markers(src, map); + const std::u32string src = U"6\U0000ffff\U00000008"; // missing trailing marker # + const std::u32string dst = remove_markers(src, map); const std::u32string expect = src; zassert_string_equal(dst, expect); assert_equal(map.size(), 0); @@ -665,17 +666,17 @@ int test_strutils() { { marker_map map; std::cout << __FILE__ << ":" << __LINE__ << " - bad1" << std::endl; - const std::u32string src = U"6\U0000ffffq"; // missing sentinel subtype - const std::u32string dst = remove_markers(src, map); - const std::u32string expect = src; // 'q' removed + const std::u32string src = U"6\U0000ffffq"; // missing sentinel subtype + const std::u32string dst = remove_markers(src, map); + const std::u32string expect = src; // 'q' removed zassert_string_equal(dst, expect); assert_equal(map.size(), 0); } { marker_map map; std::cout << __FILE__ << ":" << __LINE__ << " - bad1b" << std::endl; - const std::u32string src = U"6\U0000ffff"; // missing code - const std::u32string dst = remove_markers(src, map); + const std::u32string src = U"6\U0000ffff"; // missing code + const std::u32string dst = remove_markers(src, map); const std::u32string expect = src; zassert_string_equal(dst, expect); assert_equal(map.size(), 0); @@ -683,8 +684,8 @@ int test_strutils() { { marker_map map; std::cout << __FILE__ << ":" << __LINE__ << " - bad1c" << std::endl; - const std::u32string src = U"6\U0000ffffzz"; // missing code - const std::u32string dst = remove_markers(src, map); + const std::u32string src = U"6\U0000ffffzz"; // missing code + const std::u32string dst = remove_markers(src, map); const std::u32string expect = src; zassert_string_equal(dst, expect); assert_equal(map.size(), 0); @@ -692,8 +693,8 @@ int test_strutils() { { marker_map map; std::cout << __FILE__ << ":" << __LINE__ << " - marker end test" << std::endl; - const std::u32string src = U"6\U0000ffff\U00000008\U00000001"; - const std::u32string dst = remove_markers(src, map); + const std::u32string src = U"6\U0000ffff\U00000008\U00000001"; + const std::u32string dst = remove_markers(src, map); const std::u32string expect = U"6"; zassert_string_equal(dst, expect); marker_map expm({{MARKER_BEFORE_EOT, 0x1L}}); @@ -703,8 +704,10 @@ int test_strutils() { { marker_map map; std::cout << __FILE__ << ":" << __LINE__ << " - complex test" << std::endl; - const std::u32string src = U"6\U0000ffff\U00000008\U00000001e\U0000ffff\U00000008\U00000002\U00000320\U0000ffff\U00000008\U00000003\U00000300\U0000ffff\U00000008\U00000004"; - const std::u32string dst = remove_markers(src, map); + const std::u32string src = + U"6\U0000ffff\U00000008\U00000001e\U0000ffff\U00000008\U00000002\U00000320\U0000ffff\U00000008\U00000003\U00000300" + U"\U0000ffff\U00000008\U00000004"; + const std::u32string dst = remove_markers(src, map); const std::u32string expect = U"6e\U00000320\U00000300"; zassert_string_equal(dst, expect); marker_map expm({{U'e', 0x1L}, {0x0320, 0x2L}, {0x0300, 0x3L}, {MARKER_BEFORE_EOT, 0x4L}}); @@ -735,17 +738,17 @@ int test_strutils() { assert_equal(parse_hex_quad(U"CAFE"), 0xCAFE); assert_equal(parse_hex_quad(U"D00d"), 0xD00D); assert_equal(parse_hex_quad(U"FFFF"), 0xFFFF); - assert_equal(parse_hex_quad(U"zzzz"), 0); // err + assert_equal(parse_hex_quad(U"zzzz"), 0); // err } return EXIT_SUCCESS; } -int test_normalize() { +int +test_normalize() { std::cout << "== " << __FUNCTION__ << std::endl; std::cout << __FILE__ << ":" << __LINE__ << " * normalize_nfd_markers" << std::endl; - { marker_map map; std::cout << __FILE__ << ":" << __LINE__ << " - noop test" << std::endl; @@ -774,7 +777,7 @@ int test_normalize() { U"6\U0000ffff\U00000008\U00000001e\U0000ffff\U00000008\U00000002\U00000320\U0000ffff\U00000008\U00000003\U00000300" U"\U0000ffff\U00000008\U00000004"; const std::u32string expect = src; - std::u32string dst = src; + std::u32string dst = src; assert(normalize_nfd_markers_segment(dst, map)); zassert_string_equal(dst, expect); marker_map expm({{U'e', 0x1L}, {0x320, 0x2L}, {0x300, 0x3L}, {MARKER_BEFORE_EOT, 0x4L}}); @@ -785,10 +788,11 @@ int test_normalize() { { marker_map map; std::cout << __FILE__ << ":" << __LINE__ << " - complex test" << std::endl; - const std::u32string src = // already in order: 320+300 - U"6\U0000ffff\U00000008\U00000001e\U0000ffff\U00000008\U00000002\U00000320\U0000ffff\U00000008\U00000003\U00000300\U0000ffff\U00000008\U00000004"; + const std::u32string src = // already in order: 320+300 + U"6\U0000ffff\U00000008\U00000001e\U0000ffff\U00000008\U00000002\U00000320\U0000ffff\U00000008\U00000003\U00000300" + U"\U0000ffff\U00000008\U00000004"; const std::u32string expect = src; - std::u32string dst = src; + std::u32string dst = src; assert(normalize_nfd_markers_segment(dst, map)); zassert_string_equal(dst, expect); marker_map expm({{U'e', 0x1L}, {0x320, 0x2L}, {0x300, 0x3L}, {MARKER_BEFORE_EOT, 0x4L}}); @@ -798,10 +802,12 @@ int test_normalize() { { marker_map map; std::cout << __FILE__ << ":" << __LINE__ << " - complex test2" << std::endl; - const std::u32string src = // out of order, 300-320 - U"6\U0000ffff\U00000008\U00000001e\U0000ffff\U00000008\U00000002\U00000300\U0000ffff\U00000008\U00000003\U00000320\U0000ffff\U00000008\U00000004"; + const std::u32string src = // out of order, 300-320 + U"6\U0000ffff\U00000008\U00000001e\U0000ffff\U00000008\U00000002\U00000300\U0000ffff\U00000008\U00000003\U00000320" + U"\U0000ffff\U00000008\U00000004"; const std::u32string expect = - U"6\U0000ffff\U00000008\U00000001e\U0000ffff\U00000008\U00000003\U00000320\U0000ffff\U00000008\U00000002\U00000300\U0000ffff\U00000008\U00000004"; + U"6\U0000ffff\U00000008\U00000001e\U0000ffff\U00000008\U00000003\U00000320\U0000ffff\U00000008\U00000002\U00000300" + U"\U0000ffff\U00000008\U00000004"; std::u32string dst = src; assert(normalize_nfd_markers_segment(dst, map)); if (dst != expect) { @@ -820,7 +826,7 @@ int test_normalize() { std::cout << __FILE__ << ":" << __LINE__ << " - complex test 4a" << std::endl; const std::u32string src = U"4e\u0300\uFFFF\u0008\u0001\u0320"; const std::u32string expect = U"4e\uFFFF\u0008\u0001\u0320\u0300"; - std::u32string dst = src; + std::u32string dst = src; assert(normalize_nfd_markers_segment(dst, map)); if (dst != expect) { std::cout << "dst: " << Debug_UnicodeString(dst) << std::endl; @@ -838,14 +844,14 @@ int test_normalize() { std::cout << __FILE__ << ":" << __LINE__ << " - complex test 9c" << std::endl; const std::u32string src = U"9ce\u0300\uFFFF\u0008\u0002\u0320\uFFFF\u0008\u0001"; const std::u32string expect = U"9ce\uFFFF\u0008\u0002\u0320\u0300\uFFFF\u0008\u0001"; - std::u32string dst = src; + std::u32string dst = src; assert(normalize_nfd_markers_segment(dst, map)); if (dst != expect) { std::cout << "dst: " << Debug_UnicodeString(dst) << std::endl; std::cout << "exp: " << Debug_UnicodeString(expect) << std::endl; } zassert_string_equal(dst, expect); - marker_map expm({{0x320, 0x2L},{MARKER_BEFORE_EOT, 0x1L}}); + marker_map expm({{0x320, 0x2L}, {MARKER_BEFORE_EOT, 0x1L}}); assert_marker_map_equal(map, expm); assert_equal(map.size(), 2); } @@ -856,7 +862,7 @@ int test_normalize() { std::cout << __FILE__ << ":" << __LINE__ << " - complex test 9c+regex" << std::endl; const std::u32string src = U"9ce\u0300\\uffff\\u0008\\u0002\u0320\\uffff\\u0008\\u0001"; const std::u32string expect = U"9ce\\uffff\\u0008\\u0002\u0320\u0300\\uffff\\u0008\\u0001"; - std::u32string dst = src; + std::u32string dst = src; assert(normalize_nfd_markers_segment(dst, map, regex_sentinel)); if (dst != expect) { std::cout << "dst: " << Debug_UnicodeString(dst) << std::endl; @@ -865,7 +871,7 @@ int test_normalize() { std::cout << " " << expect << std::endl; } zassert_string_equal(dst, expect); - marker_map expm({{0x320, 0x2L},{MARKER_BEFORE_EOT, 0x1L}}); + marker_map expm({{0x320, 0x2L}, {MARKER_BEFORE_EOT, 0x1L}}); assert_marker_map_equal(map, expm); assert_equal(map.size(), 2); } @@ -875,14 +881,14 @@ int test_normalize() { std::cout << __FILE__ << ":" << __LINE__ << " - complex test \\m{.}" << std::endl; const std::u32string src = U"9ce\u0300\\uffff\\u0008[\\u0001-\\ud7fe]\u0320\\uffff\\u0008\\u0001"; const std::u32string expect = U"9ce\\uffff\\u0008[\\u0001-\\ud7fe]\u0320\u0300\\uffff\\u0008\\u0001"; - std::u32string dst = src; + std::u32string dst = src; assert(normalize_nfd_markers_segment(dst, map, regex_sentinel)); if (dst != expect) { std::cout << "dst: " << Debug_UnicodeString(dst) << std::endl; std::cout << "exp: " << Debug_UnicodeString(expect) << std::endl; } zassert_string_equal(dst, expect); - marker_map expm({{0x320, LDML_MARKER_ANY_INDEX},{MARKER_BEFORE_EOT, 0x1L}}); + marker_map expm({{0x320, LDML_MARKER_ANY_INDEX}, {MARKER_BEFORE_EOT, 0x1L}}); assert_marker_map_equal(map, expm); assert_equal(map.size(), 2); } @@ -893,7 +899,7 @@ int test_normalize() { std::cout << __FILE__ << ":" << __LINE__ << " - complex test 10 stack o' 2x2" << std::endl; const std::u32string src = U"9ce\u0300\uFFFF\u0008\u0002\uFFFF\u0008\u0002\u0320"; const std::u32string expect = U"9ce\uFFFF\u0008\u0002\uFFFF\u0008\u0002\u0320\u0300"; - std::u32string dst = src; + std::u32string dst = src; assert(normalize_nfd_markers_segment(dst, map)); if (dst != expect) { std::cout << "dst: " << Debug_UnicodeString(dst) << std::endl; @@ -905,14 +911,13 @@ int test_normalize() { assert_equal(map.size(), 2); } - { // from tests marker_map map; std::cout << __FILE__ << ":" << __LINE__ << " - complex test 10 stack o' 2x1x2" << std::endl; const std::u32string src = U"9ce\u0300\uFFFF\u0008\u0002\uFFFF\u0008\u0001\uFFFF\u0008\u0003\u0320"; const std::u32string expect = U"9ce\uFFFF\u0008\u0002\uFFFF\u0008\u0001\uFFFF\u0008\u0003\u0320\u0300"; - std::u32string dst = src; + std::u32string dst = src; assert(normalize_nfd_markers_segment(dst, map)); if (dst != expect) { std::cout << "dst: " << Debug_UnicodeString(dst) << std::endl; @@ -923,26 +928,26 @@ int test_normalize() { assert_marker_map_equal(map, expm); } - { marker_map map; std::cout << __FILE__ << ":" << __LINE__ << " - dup-char test" << std::endl; - const std::u32string src = U"a\uFFFF\u0008\u0001\u0300e\uFFFF\u0008\u0002\u0300"; - const std::u32string dst = remove_markers(src, map); - const std::u32string expect = U"a\u0300e\u0300"; // U+0300 twice! This should be removed in 2 segments + const std::u32string src = U"a\uFFFF\u0008\u0001\u0300e\uFFFF\u0008\u0002\u0300"; + const std::u32string dst = remove_markers(src, map); + const std::u32string expect = U"a\u0300e\u0300"; // U+0300 twice! This should be removed in 2 segments zassert_string_equal(dst, expect); marker_map expm({{0x300, 0x1L}, {0x300, 0x2L}}); assert_marker_map_equal(map, expm); } - { - // from tests + { marker_map map; std::cout << __FILE__ << ":" << __LINE__ << " - support 2-segment markers " << std::endl; // e\m{1}`\m{2}_E\m{3}`\m{4}_ - const std::u32string src = U"e\uFFFF\u0008\u0001\u0300\uFFFF\u0008\u0002\u0320E\uFFFF\u0008\u0003\u0300\uFFFF\u0008\u0004\u0320"; + const std::u32string src = + U"e\uFFFF\u0008\u0001\u0300\uFFFF\u0008\u0002\u0320E\uFFFF\u0008\u0003\u0300\uFFFF\u0008\u0004\u0320"; // e\m{2}_\m{1}`E\m{4}_\m{3}` - const std::u32string expect = U"e\uFFFF\u0008\u0002\u0320\uFFFF\u0008\u0001\u0300E\uFFFF\u0008\u0004\u0320\uFFFF\u0008\u0003\u0300"; + const std::u32string expect = + U"e\uFFFF\u0008\u0002\u0320\uFFFF\u0008\u0001\u0300E\uFFFF\u0008\u0004\u0320\uFFFF\u0008\u0003\u0300"; std::u32string dst = src; assert(normalize_nfd_markers_segment(dst, map)); if (dst != expect) { @@ -954,11 +959,9 @@ int test_normalize() { assert_marker_map_equal(map, expm); } - return EXIT_SUCCESS; } - int main(int argc, const char *argv[]) { int rc = EXIT_SUCCESS; @@ -969,7 +972,7 @@ main(int argc, const char *argv[]) { if (first_arg < argc) { arg_color = std::string(argv[first_arg]) == "--color"; - if(arg_color) { + if (arg_color) { first_arg++; } } From 7bc8831267e10320eed97e96d139748a042db555 Mon Sep 17 00:00:00 2001 From: "Steven R. Loomis" Date: Tue, 16 Jan 2024 17:27:26 -0600 Subject: [PATCH 49/60] =?UTF-8?q?feat(core):=20updates=20to=20add=5Fback?= =?UTF-8?q?=5Fmarkers=20=F0=9F=99=80?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - support out-of-order re-adding - test for updated marker map --- core/src/ldml/ldml_markers.cpp | 26 +++++++++++++++++------- core/tests/unit/ldml/test_transforms.cpp | 23 ++++++++++++--------- 2 files changed, 32 insertions(+), 17 deletions(-) diff --git a/core/src/ldml/ldml_markers.cpp b/core/src/ldml/ldml_markers.cpp index 2d7d354f86..7e4d496e87 100644 --- a/core/src/ldml/ldml_markers.cpp +++ b/core/src/ldml/ldml_markers.cpp @@ -66,16 +66,16 @@ bool normalize_nfd_markers_segment(std::u16string &str, marker_map &map, marker_ } } -static void add_back_markers(std::u32string &str, const std::u32string &src, const marker_map &map, marker_encoding encoding) { +static void add_back_markers(std::u32string &str, const std::u32string &src, marker_map &map, marker_encoding encoding) { // need to reconstitute. marker_map map2(map); // make a copy of the map // clear the string str.clear(); - // iterator - auto marki = map.rbegin(); + // iterator over the marker map + auto marki = map2.rbegin(); // add any end-of-text markers - while(marki != map.rend() && marki->first == MARKER_BEFORE_EOT) { + while(marki != map2.rend() && marki->first == MARKER_BEFORE_EOT) { prepend_marker(str, (marki++)->second, encoding); } @@ -84,8 +84,21 @@ static void add_back_markers(std::u32string &str, const std::u32string &src, con const auto ch = *p; str.insert(0, 1, ch); // prepend - while(marki != map.rend() && marki->first == ch) { - prepend_marker(str, (marki++)->second, encoding); + // add the markers at the end of the list first. + for (; marki != map2.rend() && marki->first == ch; marki++) { + if (marki->second != 0) { + // set to '0' if already applied + prepend_marker(str, marki->second, encoding); + marki->second = 0; // mark as already applied + } + } + + // now, add any out of order markers. + for (auto marki2 = marki; marki2 != map2.rend(); marki2++) { + if (marki2->second != 0 && marki2->first == ch) { + prepend_marker(str, marki2->second, encoding); + marki2->second = 0; // mark as already applied + } } } // assert(marki == map.rend()); // that we consumed everything @@ -93,7 +106,6 @@ static void add_back_markers(std::u32string &str, const std::u32string &src, con /** * TODO-LDML: - * - doesn't support >1 marker per char - may need a set instead of a map! * - ideally this should be used on a normalization safe subsequence */ bool normalize_nfd_markers_segment(std::u32string &str, marker_map &map, marker_encoding encoding) { diff --git a/core/tests/unit/ldml/test_transforms.cpp b/core/tests/unit/ldml/test_transforms.cpp index 0526fdb21d..27f5a9fb6b 100644 --- a/core/tests/unit/ldml/test_transforms.cpp +++ b/core/tests/unit/ldml/test_transforms.cpp @@ -815,7 +815,7 @@ test_normalize() { std::cout << "exp: " << Debug_UnicodeString(expect) << std::endl; } zassert_string_equal(dst, expect); - marker_map expm({{U'e', 0x1L}, {0x320, 0x3L}, {0x300, 0x2L}, {MARKER_BEFORE_EOT, 0x4L}}); + marker_map expm({{U'e', 0x1L}, {0x300, 0x2L}, {0x320, 0x3L}, {MARKER_BEFORE_EOT, 0x4L}}); assert_marker_map_equal(map, expm); assert_equal(map.size(), 4); } @@ -938,7 +938,6 @@ test_normalize() { marker_map expm({{0x300, 0x1L}, {0x300, 0x2L}}); assert_marker_map_equal(map, expm); } - { marker_map map; std::cout << __FILE__ << ":" << __LINE__ << " - support 2-segment markers " << std::endl; @@ -946,17 +945,21 @@ test_normalize() { const std::u32string src = U"e\uFFFF\u0008\u0001\u0300\uFFFF\u0008\u0002\u0320E\uFFFF\u0008\u0003\u0300\uFFFF\u0008\u0004\u0320"; // e\m{2}_\m{1}`E\m{4}_\m{3}` - const std::u32string expect = + const std::u32string expect_rem = + U"e\u0300\u0320E\u0300\u0320"; + const std::u32string expect_nfd = U"e\uFFFF\u0008\u0002\u0320\uFFFF\u0008\u0001\u0300E\uFFFF\u0008\u0004\u0320\uFFFF\u0008\u0003\u0300"; - std::u32string dst = src; - assert(normalize_nfd_markers_segment(dst, map)); - if (dst != expect) { - std::cout << "dst: " << Debug_UnicodeString(dst) << std::endl; - std::cout << "exp: " << Debug_UnicodeString(expect) << std::endl; - } - zassert_string_equal(dst, expect); + auto dst_rem = remove_markers(src, &map); // note: this is bigger than a single segment. so it is a degenerate test case. marker_map expm({{0x300, 0x1L}, {0x320, 0x2L}, {0x300, 0x3L}, {0x320, 0x4L}}); assert_marker_map_equal(map, expm); + zassert_string_equal(dst_rem, expect_rem); + std::u32string dst_nfd = src; + assert(normalize_nfd_markers(dst_nfd)); + if (dst_nfd != expect_nfd) { + std::cout << "dst: " << Debug_UnicodeString(dst_nfd) << std::endl; + std::cout << "exp: " << Debug_UnicodeString(expect_nfd) << std::endl; + } + zassert_string_equal(dst_nfd, expect_nfd); } return EXIT_SUCCESS; From d58260b4d67e7f84a7fb0cf454dc1381a6353b74 Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Thu, 18 Jan 2024 09:17:47 +0700 Subject: [PATCH 50/60] fix(web): bulk renderer interface for recent-version targeting keyboards --- .../tools/testing/bulk_rendering/README.md | 5 +-- .../tools/testing/bulk_rendering/index.html | 2 +- .../testing/bulk_rendering/renderer_core.ts | 33 ++++++++++++++----- 3 files changed, 29 insertions(+), 11 deletions(-) diff --git a/web/src/tools/testing/bulk_rendering/README.md b/web/src/tools/testing/bulk_rendering/README.md index 2f9703d1c7..3c062a2658 100644 --- a/web/src/tools/testing/bulk_rendering/README.md +++ b/web/src/tools/testing/bulk_rendering/README.md @@ -12,8 +12,9 @@ This renderer loads all the cloud keyboards from api.keyman.com and renders each - Note that it is preferable to run this on an actual device if possible. - If no prompt is given re: screensharing when you click the 'run' button, use Chrome's Developer Tools on a desktop or laptop to run this via emulation instead. - - If emulating a mobile device, when prompted to screenshare, be sure to share _the Chrome tab_. The screen capture - system will fail to capture the OSK properly otherwise. + - If emulating a mobile device, when prompted to screenshare... + - Be sure to share _the Chrome tab_. The screen capture system will fail to capture the OSK properly otherwise. + - Also verify that emulation zoom is set to 100%; the system will fail to capture the OSK properly otherwise. 6. Save the result to a .html file, either before.html or after.html. 7. When swapping versions, don't forget to rebuild. diff --git a/web/src/tools/testing/bulk_rendering/index.html b/web/src/tools/testing/bulk_rendering/index.html index 1b94f02c65..28350cbddd 100644 --- a/web/src/tools/testing/bulk_rendering/index.html +++ b/web/src/tools/testing/bulk_rendering/index.html @@ -27,7 +27,7 @@ - +