fix(web): restore flick functionality#12187
Merged
Conversation
User Test ResultsTest specification and instructions Test Artifacts
|
darcywong00
reviewed
Aug 15, 2024
mcdurdin
approved these changes
Aug 15, 2024
ermshiperete
approved these changes
Aug 15, 2024
Test ResultsI tested this issue with the attached "Keyman 18.0.90-alpha-test-12187" build on the Android 14 physical device. Here is my observation.
|
Contributor
Author
|
Did a quick check on Sentry, but I don't see any error reports that line up with the timestamp of your comment. Even with a few hours' fudge-factor, there's nothing even close to related that I can see. Ah well. |
Collaborator
|
Changes in this pull request will be available for download in Keyman version 18.0.91-alpha |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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.
Turns out that #12176 actually broke flicks due to a side-effect of no longer rebuilding the gesture-parameterization object when swapping keyboards. The culprit:
keyman/web/src/engine/osk/src/input/gestures/specsForLayout.ts
Line 251 in 69a3fa1
If rebuilding the parameter object from scratch, this is all well and good. If not, however... we run the risk of keeping 'roaming' mode in place if the previous keyboard had roaming enabled. This was the only spot that ever set the flag, so... it would never turn 'off' once turned 'on'. Short version: due to certain initialization patterns, it was basically always turned on. (When the OSK initializes during standard use, there's no actual Keyman keyboard ready yet... and so the engine builds a default, blank one to match a standard US English keyboard... a keyboard without flicks and thus with roaming support.)
The solution? Ignore any previous value for the setting and just override it with the value appropriate to the current keyboard. #12176 made comment changes noting this behavior for
longpress.flickEnabledas well; now that there are two such members on the overall structure, I felt it worth some type work to hide them from the type visible from the gesture-parameterization API, since they are not to be externally set.I also did a second check for anything needing similar changes to #12176, just in case, and discovered that the multitap 'sustain' timer needed a similar pattern, so I threw that in.
User Testing
TEST_GENERAL_GESTURES: Using the EuroLatin (SIL) keyboard on an Android device, verify that gestures work normally.