-
-
Notifications
You must be signed in to change notification settings - Fork 111
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(web): longpress shortcut activation should only consider northward part #11306
Conversation
…thward component change(web): stricter threshold for up-flick shortcut
User Test ResultsTest specification and instructions
Test Artifacts
|
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.
LGTM
Test Results
Screen_Recording_20240429_052157_Keyman.Beta-test-11306.mp4[
[
{
"gestureSetId": "default",
"matchedId": "initial-tap",
"linkType": "chain",
"item": "default-K_ENTER",
"sources": [
{
"identifier": "touch:195",
"isFromTouch": true,
"path": {
"coords": [
{
"targetX": 314.3333435058594,
"targetY": 163,
"t": 1232883.5999999046,
"item": "default-K_ENTER"
},
{
"targetX": 313.7109375,
"targetY": 162.33334350585938,
"t": 1232998.4000000954,
"item": "default-K_ENTER"
},
{
"targetX": 313.66668701171875,
"targetY": 162.33334350585938,
"t": 1233004.9000000954,
"item": "default-K_ENTER"
}
],
"wasCancelled": false,
"stats": {
"netDistance": 0.9427946554577716,
"duration": 121.30000019073486,
"sampleCount": 3,
"rawDistance": 0.9562922183077271
}
},
"stateToken": "default"
}
],
"allSourceIds": [
"touch:195"
]
}
],
[
{
"gestureSetId": "default",
"matchedId": "initial-tap",
"linkType": "chain",
"item": "default-K_E",
"sources": [
{
"identifier": "touch:196",
"isFromTouch": true,
"path": {
"coords": [
{
"targetX": 77,
"targetY": 21,
"t": 1234036.9000000954,
"item": "default-K_E"
},
{
"targetX": 77,
"targetY": 21,
"t": 1234078.5999999046,
"item": "default-K_E"
},
{
"targetX": 74.36003112792969,
"targetY": 29.586593627929688,
"t": 1234129.2000000477,
"item": "default-K_E"
},
{
"targetX": 74.04427337646484,
"targetY": 29.666671752929688,
"t": 1234137.0999999046,
"item": "default-K_E"
},
{
"targetX": 74,
"targetY": 29.666671752929688,
"t": 1234144.9000000954,
"item": "default-K_E"
}
],
"wasCancelled": false,
"stats": {
"angle": 3.4748367732892733,
"cardinal": "s",
"netDistance": 9.171215801246275,
"duration": 108,
"sampleCount": 5,
"rawDistance": 9.353290710515564
}
},
"stateToken": "default"
}
],
"allSourceIds": [
"touch:196"
]
}
],
[
{
"gestureSetId": "default",
"matchedId": "initial-tap",
"linkType": "chain",
"item": "default-K_D",
"sources": [
{
"identifier": "touch:197",
"isFromTouch": true,
"path": {
"coords": [
{
"targetX": 81.33333587646484,
"targetY": 98,
"t": 1234230.7999999523,
"item": "default-K_D"
},
{
"targetX": 81.33333587646484,
"targetY": 98.08822631835938,
"t": 1234252.7000000477,
"item": "default-K_D"
},
{
"targetX": 81.33333587646484,
"targetY": 98,
"t": 1234255.2999999523,
"item": "default-K_D"
},
{
"targetX": 81.33333587646484,
"targetY": 98,
"t": 1234270.5,
"item": "default-K_D"
}
],
"wasCancelled": false,
"stats": {
"netDistance": 0,
"duration": 39.700000047683716,
"sampleCount": 4,
"rawDistance": 0.17645263671875
}
},
"stateToken": "default"
}
],
"allSourceIds": [
"touch:197"
]
}
],
[
{
"gestureSetId": "default",
"matchedId": "longpress",
"linkType": "chain",
"sources": [
{
"identifier": "touch:198",
"isFromTouch": true,
"path": {
"coords": [
{
"targetX": 123.66667175292969,
"targetY": 129.33334350585938,
"t": 1234428.0999999046,
"item": "default-K_C"
},
{
"targetX": 123.62239837646484,
"targetY": 129.33334350585938,
"t": 1234444.9000000954,
"item": "default-K_C"
},
{
"targetX": 123.66667175292969,
"targetY": 129.1666717529297,
"t": 1234473.0999999046,
"item": "default-K_C"
},
{
"targetX": 122.37760925292969,
"targetY": 119.39096069335938,
"t": 1234479.7999999523,
"item": "default-K_C"
},
{
"targetX": 120.70345306396484,
"targetY": 112.1376953125,
"t": 1234489.9000000954,
"item": "default-K_C"
},
{
"targetX": 120.70345306396484,
"targetY": 112.1376953125,
"t": 1234492.4000000954,
"item": "default-K_C"
}
],
"stats": {
"angle": 6.112537534595476,
"cardinal": "n",
"netDistance": 17.44909687601092,
"duration": 64.30000019073486,
"sampleCount": 6,
"rawDistance": 17.521028120934968
}
},
"stateToken": "default"
}
],
"allSourceIds": [
"touch:198"
]
},
{
"gestureSetId": "none",
"matchedId": "subkey-select",
"linkType": "complete",
"item": "popup-default-U_010B",
"sources": [
{
"identifier": "touch:198",
"isFromTouch": true,
"path": {
"coords": [
{
"targetX": 123.66667175292969,
"targetY": 83.33334350585938,
"t": 1234428.0999999046,
"item": "default-K_C"
},
{
"targetX": 123.62239837646484,
"targetY": 83.33334350585938,
"t": 1234444.9000000954,
"item": "default-K_C"
},
{
"targetX": 123.66667175292969,
"targetY": 83.16667175292969,
"t": 1234473.0999999046,
"item": "default-K_C"
},
{
"targetX": 122.37760925292969,
"targetY": 73.39096069335938,
"t": 1234479.7999999523,
"item": "default-K_C"
},
{
"targetX": 120.70345306396484,
"targetY": 66.1376953125,
"t": 1234489.9000000954,
"item": "default-K_C"
},
{
"targetX": 120.70345306396484,
"targetY": 66.1376953125,
"t": 1234492.4000000954,
"item": "default-K_C"
},
{
"targetX": 120.33333587646484,
"targetY": 65,
"t": 1234537.4000000954,
"item": "popup-default-U_010B"
}
],
"wasCancelled": false,
"stats": {
"angle": 6.103331770766034,
"cardinal": "n",
"netDistance": 18.633910275869738,
"duration": 109.30000019073486,
"sampleCount": 7,
"rawDistance": 18.717413241440646
}
},
"stateToken": "default"
}
],
"allSourceIds": [
"touch:198"
]
}
],
[
{
"gestureSetId": "default",
"matchedId": "initial-tap",
"linkType": "chain",
"item": "default-K_E",
"sources": [
{
"identifier": "touch:199",
"isFromTouch": true,
"path": {
"coords": [
{
"targetX": 82,
"targetY": 27.333335876464844,
"t": 1234638,
"item": "default-K_E"
},
{
"targetX": 82.08821868896484,
"targetY": 27.333335876464844,
"t": 1234668.2000000477,
"item": "default-K_E"
},
{
"targetX": 82,
"targetY": 27.333335876464844,
"t": 1234670.5,
"item": "default-K_E"
},
{
"targetX": 82,
"targetY": 27.333335876464844,
"t": 1234681.5,
"item": "default-K_E"
}
],
"wasCancelled": false,
"stats": {
"netDistance": 0,
"duration": 43.5,
"sampleCount": 4,
"rawDistance": 0.1764373779296875
}
},
"stateToken": "default"
}
],
"allSourceIds": [
"touch:199"
]
}
],
[
{
"gestureSetId": "default",
"matchedId": "initial-tap",
"linkType": "chain",
"item": "default-K_L",
"sources": [
{
"identifier": "touch:200",
"isFromTouch": true,
"path": {
"coords": [
{
"targetX": 314.3333435058594,
"targetY": 75,
"t": 1234729.7999999523,
"item": "default-K_L"
}
],
"wasCancelled": false,
"stats": {
"netDistance": 0,
"duration": 0,
"sampleCount": 1,
"rawDistance": 0
}
},
"stateToken": "default"
}
],
"allSourceIds": [
"touch:200"
]
}
],
[
{
"gestureSetId": "default",
"matchedId": "initial-tap",
"linkType": "chain",
"item": "default-K_L",
"sources": [
{
"identifier": "touch:201",
"isFromTouch": true,
"path": {
"coords": [
{
"targetX": 314,
"targetY": 77,
"t": 1234843.7999999523,
"item": "default-K_L"
},
{
"targetX": 314.0882263183594,
"targetY": 76.95573425292969,
"t": 1234876.7999999523,
"item": "default-K_L"
},
{
"targetX": 314,
"targetY": 77,
"t": 1234888.0999999046,
"item": "default-K_L"
}
],
"wasCancelled": false,
"stats": {
"netDistance": 0,
"duration": 44.299999952316284,
"sampleCount": 3,
"rawDistance": 0.19741671271645336
}
},
"stateToken": "default"
}
],
"allSourceIds": [
"touch:201"
]
}
],
[
{
"gestureSetId": "default",
"matchedId": "initial-tap",
"linkType": "chain",
"item": "default-K_E",
"sources": [
{
"identifier": "touch:202",
"isFromTouch": true,
"path": {
"coords": [
{
"targetX": 84.33333587646484,
"targetY": 39.333335876464844,
"t": 1234956.5,
"item": "default-K_E"
},
{
"targetX": 84.33333587646484,
"targetY": 39.333335876464844,
"t": 1234978.5999999046,
"item": "default-K_E"
}
],
"wasCancelled": false,
"stats": {
"netDistance": 0,
"duration": 22.09999990463257,
"sampleCount": 2,
"rawDistance": 0
}
},
"stateToken": "default"
}
],
"allSourceIds": [
"touch:202"
]
}
],
[
{
"gestureSetId": "default",
"matchedId": "initial-tap",
"linkType": "chain",
"item": "default-K_N",
"sources": [
{
"identifier": "touch:203",
"isFromTouch": true,
"path": {
"coords": [
{
"targetX": 225.33334350585938,
"targetY": 133.6666717529297,
"t": 1235109.4000000954,
"item": "default-K_N"
}
],
"wasCancelled": false,
"stats": {
"netDistance": 0,
"duration": 0,
"sampleCount": 1,
"rawDistance": 0
}
},
"stateToken": "default"
}
],
"allSourceIds": [
"touch:203"
]
}
],
[
{
"gestureSetId": "default",
"matchedId": "initial-tap",
"linkType": "chain",
"item": "default-K_T",
"sources": [
{
"identifier": "touch:204",
"isFromTouch": true,
"path": {
"coords": [
{
"targetX": 150.33334350585938,
"targetY": 30.333335876464844,
"t": 1235222.5999999046,
"item": "default-K_T"
},
{
"targetX": 150.33334350585938,
"targetY": 30.333335876464844,
"t": 1235295.5999999046,
"item": "default-K_T"
},
{
"targetX": 150.33334350585938,
"targetY": 30.333335876464844,
"t": 1235305,
"item": "default-K_T"
}
],
"wasCancelled": false,
"stats": {
"netDistance": 0,
"duration": 82.40000009536743,
"sampleCount": 3,
"rawDistance": 0
}
},
"stateToken": "default"
}
],
"allSourceIds": [
"touch:204"
]
}
]
] |
I hear you, but the case you ran into here looks pretty hard to distinguish. Here's where your tap started: And here's where the tap pretty much stopped moving: I personally have similar issues when using joystick-based controller inputs while playing video games; my input direction is often not in the direction I believe it to be.
At the point where the engine "locked in" the longpress: "stats": {
"angle": 6.112537534595476,
"cardinal": "n",
"netDistance": 17.44909687601092,
"duration": 64.30000019073486,
"sampleCount": 6,
"rawDistance": 17.521028120934968
} The angle's value is in radians: that's 350.2 degree angle, only 10 degrees off of pure north. 98.5% of the net distance for such an angle is vertically aligned. I believe that this is well within any reasonable tolerance for up-flick detection, partly due to the aforementioned "angle alignment" issues I indicated I personally have when making inputs. |
After a brief in-person discussion... part of the reason for
is that we auto-cancel flicks that don't reach the final required distance. We aren't doing so for the longpress up-flick, so it feels like it triggers far more easily. That's a fair point. To help rectify this difference, I've adjusted the behavior a bit more.
I've added a new parameter to match, though I suppose we could just use the same param as is defined for flick finalization. This makes things a tad bit more complex, but it allows us to avoid adjusting the internal gesture state model graph - that would be a far riskier change to make at this stage of the game. Given the changes made... @keymanapp-test-bot retest all |
@@ -31,10 +31,16 @@ export interface GestureParams<Item = any> { | |||
*/ | |||
permitsFlick: (item?: Item) => boolean, | |||
|
|||
/** | |||
* The minimum _net_ distance traveled before a longpress flick-shortcut will cancel any | |||
* conflicting flick models. |
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.
To be clear, it only does so if there are no actual northish flicks. (nw, n, ne)
Took a while to get it, but I got a local reproduction:
The 'flick-start' model shouldn't even be evaluated if the key doesn't support flicks... yet... somehow we ended up with a case where it mattered. Interestingly, when it triggered, the keyboard locked up on my end; the Edit: it is! And it's actually stupidly easy to reproduce once I investigate it - just tap the period key. It's a non-flick key on a flick-enabled keyboard. Will document more details on a related issue. |
…o fix/web/longpress-shortcut-constraints
@keymanapp-test-bot retest all I found the cause of the reported errors and have added a fix with #11321. This PR is now based upon that PR, so the fixes are merged in - the errors should no longer occur. |
I have been testing this build, and it is looking really solid -- not really experiencing issues with longpress shortcuts any more, even with rapid typing. |
Test Results
|
Changes in this pull request will be available for download in Keyman version 17.0.318-beta |
Fixes #10647.
In retrospect, I did find it odd, once or twice before, how the longpress contact-model wasn't extracting the vertical component of the traveled distance and was instead using the whole, unprojected distance. Turns out... it had noticeable cross-effects and was the primary underlying contributor to #10647. I just... didn't think that part of the math completely through for some reason. (
git blame
-> me)As noted in a recording and repro of two such occurrences, the user's touchpoint traveled just past the up-flick threshold distance... at a nearly sixty degree angle from it! We should only be considering vertical distance here.
User Testing
TEST_UP_FLICKS: Using the Keyman for Android app, use the default EuroLatin keyboard to verify that an up-flick on base-layer keys quickly displays the subkey menu, without a need to wait half a second.
TEST_QUICK_TYPING: Using the Keyman for Android app, use the default EuroLatin keyboard and type rapidly, without trying to use flicks or longpresses. If a character appears with a diacritic when not intended, FAIL this test.