feat(web): VisualKeyboard layout-reflow optimization 🪠 #11177
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,17 +1,20 @@ | ||
| import { ActiveKey, ActiveSubKey, ButtonClass, ButtonClasses, DeviceSpec } from '@keymanapp/keyboard-processor'; | ||
|
|
||
| // At present, we don't use @keymanapp/keyman. Just `keyman`. (Refer to <root>/web/package.json.) | ||
| import specialChars from '../specialCharacters.js'; | ||
| import buttonClassNames from '../buttonClassNames.js'; | ||
|
|
||
| import { KeyElement } from '../keyElement.js'; | ||
| import VisualKeyboard from '../visualKeyboard.js'; | ||
| import { getTextMetrics } from './getTextMetrics.js'; | ||
| import { ParsedLengthStyle } from '../lengthStyle.js'; | ||
|
|
||
| export interface KeyLayoutParams { | ||
| keyWidth: number; | ||
| keyHeight: number; | ||
| baseEmFontSize: ParsedLengthStyle; | ||
| layoutFontSize: ParsedLengthStyle; | ||
| } | ||
|
|
||
| /** | ||
| * Replace default key names by special font codes for modifier keys | ||
|
|
@@ -59,6 +62,9 @@ export default abstract class OSKKey { | |
| label: HTMLSpanElement; | ||
| square: HTMLDivElement; | ||
|
|
||
| private _fontSize: ParsedLengthStyle; | ||
| private _fontFamily: string; | ||
|
|
||
| /** | ||
| * The layer of the OSK on which the key is displayed. | ||
| */ | ||
|
|
@@ -169,58 +175,54 @@ export default abstract class OSKKey { | |
|
|
||
| /** | ||
| * Calculate the font size required for a key cap, scaling to fit longer text | ||
| * @param text | ||
| * @param layoutParams specification for the key | ||
| * @param scale additional scaling to apply for the font-size calculation (used by keytips) | ||
| * @returns font size as a style string | ||
| */ | ||
| getIdealFontSize(text: string, layoutParams: KeyLayoutParams, scale?: number): ParsedLengthStyle { | ||
| // Fallback in case not all style info is currently ready. | ||
| if(!this._fontFamily) { | ||
| return new ParsedLengthStyle('1em'); | ||
| } | ||
|
|
||
| scale ??= 1; | ||
|
|
||
| const keyWidth = layoutParams.keyWidth; | ||
| const keyHeight = layoutParams.keyHeight; | ||
| const emScale = layoutParams.baseEmFontSize.scaledBy(layoutParams.layoutFontSize.val); | ||
|
|
||
| // Among other things, ensures we use SpecialOSK styling for special key text. | ||
| // It's set on the key-span, not on the button. | ||
| // | ||
| // Also helps ensure that the stub's font-family name is used for keys, should | ||
| // that mismatch the font-family name specified within the keyboard's touch layout. | ||
|
|
||
| let originalSize = this._fontSize; | ||
| if(!originalSize.absolute) { | ||
| originalSize = emScale.scaledBy(originalSize.val); | ||
| } | ||
|
|
||
| const style = { | ||
| fontFamily: this._fontFamily, | ||
| fontSize: originalSize.styleString, | ||
| height: layoutParams.keyHeight | ||
| } | ||
|
|
||
| let metrics = getTextMetrics(text, emScale.scaledBy(scale).val, style); | ||
|
|
||
| const MAX_X_PROPORTION = 0.90; | ||
| const MAX_Y_PROPORTION = 0.90; | ||
| const X_PADDING = 2; | ||
|
|
||
| var fontHeight: number; | ||
| if(metrics.fontBoundingBoxAscent) { | ||
| fontHeight = metrics.fontBoundingBoxAscent + metrics.fontBoundingBoxDescent; | ||
| } | ||
|
|
||
| // Don't add extra padding to height - multiplying with MAX_Y_PROPORTION already gives | ||
| // padding | ||
| let textHeight = fontHeight ?? 0; | ||
| let xProportion = (keyWidth * MAX_X_PROPORTION) / (metrics.width + X_PADDING); // How much of the key does the text want to take? | ||
| let yProportion = textHeight && keyHeight ? (keyHeight * MAX_Y_PROPORTION) / textHeight : undefined; | ||
|
|
||
|
|
@@ -229,28 +231,10 @@ export default abstract class OSKKey { | |
| proportion = yProportion; | ||
| } | ||
|
|
||
| // Never upscale keys past the default * the specified scale - only downscale them. | ||
| // Proportion < 1: ratio of key width to (padded [loosely speaking]) text width | ||
| // maxProportion determines the 'padding' involved. | ||
| return ParsedLengthStyle.forScalar(scale * Math.min(proportion, 1)); | ||
| } | ||
|
|
||
| public get keyText(): string { | ||
|
|
@@ -310,47 +294,73 @@ export default abstract class OSKKey { | |
| } | ||
|
|
||
| // Check the key's display width - does the key visualize well? | ||
| if(vkbd.isRTL) { | ||
| // Add the RTL marker to ensure it displays properly. | ||
| keyText = '\u200f' + keyText; | ||
| } | ||
|
|
||
| // Finalize the key's text. | ||
| t.innerText = keyText; | ||
|
|
||
| return t; | ||
| } | ||
|
|
||
| public resetFontPrecalc() { | ||
| this._fontFamily = undefined; | ||
| this._fontSize = undefined; | ||
| this.label.style.fontSize = ''; | ||
| } | ||
|
|
||
| public refreshLayout(layoutParams: KeyLayoutParams) { | ||
| // Avoid doing any font-size related calculations if there's no text to display. | ||
| if(this.spec.sp == ButtonClasses.spacer || this.spec.sp == ButtonClasses.blank) { | ||
| return () => {}; | ||
| } | ||
|
There was a problem hiding this comment. It'd be better to |
||
|
|
||
| // Attempt to detect static but key-specific style properties if they haven't yet | ||
| // been detected. | ||
| if(this._fontFamily === undefined) { | ||
| const lblStyle = getComputedStyle(this.label); | ||
|
|
||
| // Abort if the element is not currently in the DOM; we can't get any info this way. | ||
| if(!lblStyle.fontFamily) { | ||
| return () => {}; | ||
| } | ||
| this._fontFamily = lblStyle.fontFamily; | ||
|
|
||
| // Detect any difference in base (em) font size and that which is computed for the key itself. | ||
| const computedFontSize = new ParsedLengthStyle(lblStyle.fontSize); | ||
| const layoutFontSize = layoutParams.layoutFontSize; | ||
| if(layoutFontSize.absolute) { | ||
| // rather than just straight-up taking .layoutFontSize | ||
| this._fontSize = computedFontSize; | ||
| } else { | ||
| const baseEmFontSize = layoutParams.baseEmFontSize; | ||
| const baseFontSize = layoutFontSize.scaledBy(baseEmFontSize.val); | ||
| const localFontScaling = computedFontSize.val / baseFontSize.val; | ||
| this._fontSize = ParsedLengthStyle.forScalar(localFontScaling); | ||
| } | ||
| } | ||
|
|
||
| // space bar may not define the text span! | ||
| if(this.label) { | ||
| if(!this.label.classList.contains('kmw-spacebar-caption')) { | ||
| // Do not use `this.keyText` - it holds *___* codes for special keys, not the actual glyph! | ||
| const keyCapText = this.label.textContent; | ||
| const fontSize = this.getIdealFontSize(keyCapText, layoutParams); | ||
| return () => { | ||
| this.label.style.fontSize = fontSize.styleString; | ||
| }; | ||
| } else { | ||
|
Comment on lines
+351
to
+353
There was a problem hiding this comment. This is kinda ugly and it's unclear why this is postponed and how it gets called There was a problem hiding this comment. This is the other side of that Admittedly, yeah, the |
||
| // Spacebar text, on the other hand, is available via this.keyText. | ||
| // Using this field helps prevent layout reflow during updates. | ||
| const fontSize = this.getIdealFontSize(this.keyText, layoutParams); | ||
|
|
||
| return () => { | ||
| // Since the kmw-spacebar-caption version uses !important, we must specify | ||
| // it directly on the element too; otherwise, scaling gets ignored. | ||
| this.label.style.setProperty("font-size", fontSize.styleString, "important"); | ||
| }; | ||
| } | ||
| } | ||
| } | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why 'keyTextClosure'? It's really unclear what that does! Perhaps we should call it
superFinalizationto clarify that it's more work that super.refreshlayout has to do once this function is finished?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because it's a closure to finalize/apply layout changes directly tied to key text as seen in OSKKey.
If I swapped to
superFinalizationor something else unrelated to the code-contents of the closure... well, I suppose I can always just add a comment about what the closure's actual effects are.