fix(android): rework longpress movement trigger 🎁#6984
Conversation
Fixes #6981. Longpress menus were being triggered on any detected movement on keys on Android. This would cause rapid typing to periodically fail as the user would make a single-pixel movement while typing, which would then block subsequent key events. This issue was introduced in #6637, which resolved a somewhat related problem with sticky longpress menus. The fix is in three parts: 1. Remove the touch movement detection which caused the primary problem 2. Address some logic issues around visibility of `subkeysWindow` 3. Introduce a new 'scroll' gesture handler which detects a negative-y movement above a 5px threshold, to open the longpress menu. The 5px threshold is the minimum default used by Keyman Engine for Web. However, it may not be ideal, and we should consider moving to using the 0.25 x row height value that Keyman Engine for Web uses in a future update. However, I do not consider this to be a reason to block this fix, as it would require significant extra engineering.
User Test ResultsTest specification and instructions
Test Artifacts |
|
jahorton
left a comment
There was a problem hiding this comment.
While I have to admit that I don't directly see exactly how the changes match with the first point listed in the description, the changes overall do seem reasonable. That, and the fact that the user tests passed, is enough for me to LGTM this.
This relates to the removal of this code (L296-299 of KMKeyboard.java): if (action == MotionEvent.ACTION_MOVE && subKeysList != null && subKeysWindow == null) {
// Display subkeys during move
showSubKeys(context);
} |
|
Many thanks @mcdurdin |
|
Changes in this pull request will be available for download in Keyman version 16.0.38-alpha |
Fixes #6981.
Longpress menus were being triggered on any detected movement on keys on Android. This would cause rapid typing to periodically fail as the user would make a single-pixel movement while typing, which would then block subsequent key events.
This issue was introduced in #6637, which resolved a somewhat related problem with sticky longpress menus.
The fix is in three parts:
subkeysWindowwhich meant that the longpress delay trigger was never fired.The 5px threshold is the minimum default used by Keyman Engine for Web. However, it may not be ideal, and we should consider moving to using the 0.25 x row height value that Keyman Engine for Web uses in a future update. However, I do not consider this to be a reason to block this fix, as it would require significant extra engineering.
User Testing
We need to re-run the main test from #6637, first, to verify that this does not re-introduce any of the behaviours that that PR was intended to fix.
Setup - A physical Android device. Don't use emulator because the scenario involves typing fast on the OSK (e.g. two thumbs)
eand immediately swipe upwards to access the long-press menu, keeping your finger down. This should show long-press options without the 0.5 second delay.