change(web): floating tablet key-previews 🐵#10102
Conversation
User Test ResultsTest specification and instructions User tests are not required |
| const DEFAULT_TIP_ORIENTATION = 'top'; | ||
| const DEFAULT_TIP_ORIENTATION: PhoneKeyTipOrientation = 'top'; | ||
|
|
||
| export type PhoneKeyTipOrientation = 'top' | 'bottom'; |
There was a problem hiding this comment.
This should be above line 8, even though it seems to build okay, just for clarity.
| public clearHint() { | ||
| this.hintLabel.classList.add('hint-clear'); | ||
| } |
There was a problem hiding this comment.
What does clearHint() / hint-clear do? It sounds like it is removing the hint but doing so by adding a class feels a bit strange. How is hint-clear removed again?
Also, it should be kmw-hint-clear for consistency and namespace protection
There was a problem hiding this comment.
If a key-preview state (somehow) supports both flicks and multitaps, clearHint will hide the multitap hint once a flick starts, as the start of a flick locks out the ability to multitap. Granted, I don't think such a state may occur due to the order of events.
As a new GesturePreview instance is built for future gestures... hint-clear isn't ever removed. There's no gesture life-cycle where undoing it makes sense... assuming that the preview instance is replaced when starting gestures.
So... replacing the instance is the "removal", so to speak?
There was a problem hiding this comment.
So ... then doesn't it make more sense to destroy it?
There was a problem hiding this comment.
Oh, right. For posterity, since it didn't seem to get stated otherwise - this allows fading the hints via CSS transition. That'd be the reason not to outright destroy it.
In order to facilitate good, well-behaved key-previews for multitaps, it's necessary to change the tablet preview pattern to use an element that floats over the keyboard, similar to the phone-oriented key tip. This PR implements the changes needed for this, while a followup will use them to facilitate multitap previews.
This also may fix the last dregs regarding #9187, as I discovered that keyboards which do not include files for widely-distributed fonts (such as Arial) were not having that font-spec respected by key previews - neither for the phone-style or this new tablet style.
As it's best to get the full body of work all at once...
@keymanapp-test-bot skip