Skip to content

feat(web): delayed modipress completion 🐵#9973

Merged
jahorton merged 25 commits intofeature-gesturesfrom
feat/web/delayed-modipress-completion
Nov 24, 2023
Merged

feat(web): delayed modipress completion 🐵#9973
jahorton merged 25 commits intofeature-gesturesfrom
feat/web/delayed-modipress-completion

Conversation

@jahorton
Copy link
Copy Markdown
Contributor

@jahorton jahorton commented Nov 8, 2023 •
edited
Loading

This aims to cover a few "fun" edge cases that can easily arise during quick typing that involves modipresses:

  1. For longpresses triggered during a modipress that has since been released, the underlying layer for the OSK will not revert until subkey-selection is complete.
    • It felt "weird" when it would swap back early, underneath the subkey menu.
    • Granted, it also feels weird to me when it stays in place, but it's a better representation of the system's state.
  2. For flicks triggered during a modipress that has since been released, the underlying layer for the OSK will not revert until the flick is complete.
    • Note: flick completion may be triggered if a new, incoming tap is received. In such a case:
      1. The flick completes as normal, for the original key from the modipressed layer.
      2. The modipress is then fully resolved, including reversion of the layer to the one that was originally active before the modipress
      3. And then the new, incoming tap is processed - for the pre-modipress layer.

The main idea behind point 2 above: during quick typing, it's very natural to lift the modipress finger to prepare to type a new key. This could easily trigger earlier than intended, so we keep the modipress active while the flick is still active.

Point 1 above then provides consistency with point 2 - in both cases, the layer "sticks" until the pending gesture is completed.

In case the user accidentally presses a new key either slightly before or simultaneously with release during this state, we can still allow the flick to complete. Since this is generally during quick typing, and the user did lift the modipress key, they've likely has anticipated the layer to return - and so we adjust for that expectation.

I will say that point 2 above was an interesting niche case to implement - it involved a second coming of the TWo caps problem (#7173 / #9802) from a slightly different angle. While it does add some complexity, it should help make flicks more robust to certain natural 'optimizations' we often subconsciously make while typing quickly.

Notes for future revisitation

❗ ❗ ❗

I ended up throwing in quite a number of other fixes into this one, as it held the big user test suite and was used as the anchor-point for demo builds. Not exactly ideal, but it is what it is at this point. FWIW, I did aim to keep the commits reasonably clean, so going commit by commit might help... though there was a merge from the base (including a master update) that may make certain aspects interesting to follow.

User Testing

Note that most of these tests will be repeats of user tests from previous PRs. Consider this largely a feature-branch regression-test suite... and thus tests worthy of consideration for becoming permanent regression tests once the feature-branch lands.

TEST_10_KEY_ROTA: Using the standard "Test unminified KeymanWeb" test page from a mobile device...

  1. Select "English...", then "Classic 10-key".
  2. Verify that all of the keys numbered 1-9 aside from 7 have three or more lowercase letters as key hints.
  3. Tap the 8 key once, then wait for a second.
  4. Tap the 8 key twice in quick succession, then wait for a second.
    • Expected output: 8a.
  5. Tap the 8 key continuously for at least 2 seconds.
    • it should rotate through: 8a8, 8aa, 8ab, 8ac, 8aA, 8aB, 8aC, then back to 8a8 (and continue from there)
    • stop tapping when the new character is a capital letter.
  6. Verify that the key hints are now capitalized.
  7. Double-tap the 9 key once. A D should be emitted as the result.
  8. Repeatedly tap 9 for at least five seconds, paying attention to the key hints.
    • Whenever a lowercase letter is the result, the key hints should be lowercase
    • Whenever an uppercase letter is the result, the key hints should be uppercase

TEST_10_KEY_DIACRITICS: Using the standard "Test unminified KeymanWeb" test page from a mobile device...

  1. Select the text-area element - the first input-accepting element on the page.
    • Placeholder text: "Type here".
    • Is directly underneath "Type in your language in this text area:"
  2. Select "English...", then "Diacritic 10-key Rota".
  3. Tap the '◌̀' key. (The key between backspace [above] and enter [below], representing a diacritic)
  4. Double-tap the 8 key.
    • Expected result: à
  5. Triple-tap the '◌̀' key - a circumflex diacritic should appear over the à.
  6. Triple-tap the 9 key.
    • Expected total result: àê
  7. Tap the '◌̀' key. A diacritic should appear over the ê.
  8. Deselect the text-area element.
  9. Reselect the text-area element, placing the point of text entry (the caret) at the end, after the ê.
    • Note for future test-maintenance: the point is to trigger a context-reset.
  10. Double-tap the 8 key.
    • Expected final result: àềa
      image
    • For clarity:
      • With a copy of the à's accent-mark on top the original ê, as in the cropped screenshot above.
        • It looks different in raw-text form here due to GitHub's font / rendering.
      • Note: there is no matching accent-mark on the second a.

TEST_APP_10_KEY_DIACRITICS: Using the Keyman for Android test artifact...

  1. Download this KMP: https://jahorton.github.io/diacritic_rota.kmp
  2. Open / install it for use within the Keyman app.
  3. Tap the 7 key twice; verify that 77 is output.
  4. Tap the '◌̀' key. (The key between backspace [above] and enter [below], representing a diacritic)
  5. Double-tap the 8 key.
    • Expected total result: 77à
  6. Triple-tap the '◌̀' key - a circumflex diacritic should appear over the à.
  7. Triple-tap the 9 key.
    • Expected total result: 77àê
  8. Tap the '◌̀' key. A diacritic should appear over the ê.
  9. Open the in-app Settings menu.
    • Note, for test maintenance: the point of this step and the next one is to trigger a context-reset.
  10. Close the in-app Settings menu.
  11. Double-tap the 8 key.
    • Expected final result: 77àềa
      image
    • For clarity:
      • With a copy of the à's accent-mark on top the original ê, as in the cropped screenshot above.
        • It looks different in raw-text form here due to GitHub's font / rendering.
      • Note: there is no matching accent-mark on the second a.

TEST_BASIC_SIMPLE_SHIFT: Using the standard "Test unminified KeymanWeb" test page from a mobile device...

  1. Select "Norwegian" (for the SIL EuroLatin keyboard)
  2. Tap the spacebar. (The keyboard should be on the default [lowercase] layer after this.)
  3. Tap the shift key once.
  4. Verify that the keyboard changes to, and stays on, the shift layer.

TEST_BASIC_MODIPRESS: Using the standard "Test unminified KeymanWeb" test page from a mobile device...

  1. Select "Norwegian" (for the SIL EuroLatin keyboard)
  2. Tap the spacebar. (The keyboard should be on the default [lowercase] layer after this.)
  3. Tap and hold the shift key.
  4. Verify that the keyboard changes to the shift layer.
  5. Tap the A key, verifying that it produces an A.
  6. Release the shift key.
  7. Verify that the keyboard changes to the default layer as a result of step 6.
  8. Repeat steps 3 through 7 quickly, holding shift just long enough to tap the shift layer's A key, then releasing quickly.

TEST_BASIC_MODIPRESS_HOLD: Using the standard "Test unminified KeymanWeb" test page from a mobile device...

  1. Select "Norwegian" (for the SIL EuroLatin keyboard)
  2. Tap the spacebar. (The keyboard should be on the default [lowercase] layer after this.)
  3. Tap and hold the shift key.
  4. Verify that the keyboard changes to the shift layer.
  5. Wait at least one second.
  6. Release the shift key.
  7. Verify that the keyboard changes to the default layer as a result of step 6.

TEST_NUMERIC_FROM_SHIFT: Using the standard "Test unminified KeymanWeb" test page from a mobile device...

  1. Select "Norwegian" (for the SIL EuroLatin keyboard)
  2. Tap the spacebar. (The keyboard should be on the default [lowercase] layer after this.)
  3. Tap the shift key, making the shift layer active.
  4. Tap and hold the numeric key (123) underneath the shift key.
  5. Verify that the keyboard changes to the numeric layer - numbers in the top row, mathematical symbols elsewhere.
  6. Longpress the % key and select the ‱ subkey.
  7. Verify that ‱ is emitted as text.
  8. Release the numeric key.
  9. Verify that the keyboard changes back to the shift layer.
  10. Tap the numeric key, releasing it immediately.
  11. Repeat steps 6 and 7, longpressing the % key and select the ‱ subkey.
  12. Verify that the keyboard automatically changes back to the default (lowercase) layer.

TEST_DELAYED_SUBKEY: Using the standard "Test unminified KeymanWeb" test page from a mobile device...

  1. Select "Norwegian" (for the SIL EuroLatin keyboard)
  2. Tap the spacebar. (The keyboard should be on the default [lowercase] layer after this.)
  3. Tap the shift key, making the shift layer active.
  4. Tap and hold the numeric key (123) underneath the shift key.
  5. Verify that the keyboard changes to the numeric layer - numbers in the top row, mathematical symbols elsewhere.
  6. Longpress the % key, but do not select a subkey yet.
  7. Release the numeric key.
  8. Verify that the keyboard changes remains on the numeric layer, with the subkey menu still displayed.
  9. Select the ‱ subkey.
  10. Verify that ‱ is emitted as text and that the layer has changed back to the shift layer.

TEST_DOUBLETAP_CAPS: Using the standard "Test unminified KeymanWeb" test page from a mobile device...

  1. Select "Norwegian" - you should then see "Norwegian - EuroLatin (SIL)" in the space bar.
  2. Type All .
  3. Double-tap the shift key. Afterward, the shift key should be highlighted, and its up-arrow should have a thin line underneath, as if underlining it.
  4. Without touching the shift key at any point, type CAPS .
    • If this is impossible - i.e., the layer shifted out from underneath you - FAIL the test.
  5. Single-tap the shift key. Afterward, you should be returned to the base layer.
  6. Type work .
  7. Tap the shift key once.
  8. Without touching the shift key at any point, type Well.
    • If this is impossible - i.e., the layer did not drop the the base layer when expected - FAIL the test.
    • Expected final result, in total: All CAPS work Well

TEST_CUSTOM_MULTITAP_MODIFIER: Using the standard "Test unminified KeymanWeb" test page from a mobile device...

  1. Select "English...", then "Gesture Prototyping".
  2. Tap and hold the ◌́ key. (The key between the spacebar [left] and the Enter key [right].)
  3. A number of letters, including all of the vowels, should change to versions using the ◌́ accent mark.
  4. Tap the á and verify that the corresponding letter is output.
  5. Release the ◌́ key; you should be returned to the default layer.
  6. Note the hints on the ◌́, then triple-tap it, holding on the third tap.
  7. A number of letters, including all of the vowels, should change to versions using the ◌̂ mark
    • This corresponds to the second diacritic from the ◌́-key's hint.
  8. Tap the â and verify that the corresponding letter is output.
  9. Release the ◌́ key; you should be returned to the default layer.

TEST_ALTERNATING_SHIFT_AND_KEY: Using the standard "Test unminified KeymanWeb" test page from a mobile device...

  1. Select "English...", then "Gesture Prototyping".
  2. Tap and hold the SHIFT key
  3. Tap the H key and release it.
  4. Release the SHIFT key.
  5. Verify that an H is output and that the layer returns to default after step 4.
  6. Repeat steps 2-5 five times.
  7. Repeat steps 2-4 quickly for at least 10 seconds.
    • Do not bother checking the result layer; the priority is to be quick here.
    • FAIL this test if any output text is replaced or removed during these repetitions.
    • FAIL this test if text (either h or H) is not output each time you press the h / H key.
  8. When done with step 7's repetitions, tap the numeric (123) key and verify that the layer switches properly to keys with numbers in the top row and symbols in the middle two rows.

TEST_FLICKS_BASIC: Using the standard "Test unminified KeymanWeb" test page from a mobile device...

  1. Select "English...", then "Gesture Prototyping".
  2. Tap and hold the u key.
  3. Drag it in an northwest direction (up and to the left) for at least a key-width.
  4. Release it.
  5. Expected result: ù
  6. Tap the u key again and drag it in a northeast direction for at least a key-width before releasing it.
  7. Expected result: ú (does not replace the previous output)
  8. Tap u again and drag north (straight up) in the same manner.
  9. Expected result: û.

TEST_FLICK_CORRECTION: Using the standard "Test unminified KeymanWeb" test page from a mobile device...

  1. Select "English...", then "Gesture Prototyping".
  2. Tap and hold the u key.
  3. Drag it in an purely west direction (straight left).
  4. Release it.
  5. Expected result: ù
  6. Tap and hold the u key.
  7. Drag it purely south (straight down).
  8. Release it.
  9. Expected result: u. The flick animation and key preview should disappear.
  10. Tap and hold the u key again.
  11. Drag it up just a few pixels / millimeters - a very short distance - then release it.
  12. Expected output: u.

TEST_FLICK_LOCKING: Using the standard "Test unminified KeymanWeb" test page from a mobile device...

  1. Select "English...", then "Gesture Prototyping".
  2. Tap and hold the u key.
  3. For the following steps, pay attention to the animated preview.
  4. Drag it purely north (straight up) - you should see the û key in the preview area (assuming it's not blocked by your finger).
    • If the animation was "jumpy" - if there was a lack of motion at the same time you moved your finger, then a big "jump" to catch up - FAIL this test.
    • If any such "jumps" were directly tied to you moving your finger quickly, do not fail this test.
  5. Return your finger to its original position; the 'u' key should slide back into place.
    • Note: "original position" is kind of important; if you return slightly to the left of the original position, the next step may not work well.
  6. Now, drag your finger in a up-and-right motion: you should also see a preview for ú as you do so.
    • Again, if the animation felt "jumpy" and not well connected to your input, FAIL this test.
  7. While the ú is visible and in the center, lift your finger and end the gesture.
  8. Verify that a ú is output.
  9. Repeat steps 2 through 6 for the a key; you should see similar previews and text there. (â, á)

TEST_MODIPRESS_MULTITAP_FLICK_PREVIEW: Using the standard "Test unminified KeymanWeb" test page from a mobile device...

  1. Select "English...", then "Gesture Prototyping".
  2. Double-tap and hold the SHIFT key, keeping it stationary for all following steps.
  3. Tap and hold the U key.
  4. Verify that the key preview is visible.
  5. Drag the finger on U purely north (straight up) - you should see the Û key in the preview area (assuming it's not blocked by your finger).
  6. Verify that the flick-animation in the preview scrolls to a Û.
  7. With the Û fully in-view, release the flick.
  8. Verify that a Û is output.
  9. Repeat steps 2 through 6 quickly three times, including the "verify" steps.

TEST_FLICK_DURING_MODIPRESS: Using the standard "Test unminified KeymanWeb" test page from a mobile device...

  1. Select "English...", then "Gesture Prototyping".
  2. Tap and hold the SHIFT key, keeping it stationary for all following steps.
  3. Tap and hold the U key.
  4. For the following steps, pay attention to the animated preview.
  5. Drag it purely north (straight up) - you should see the Û key in the preview area (assuming it's not blocked by your finger).
    • If the animation was "jumpy" - if there was a lack of motion at the same time you moved your finger, then a big "jump" to catch up - FAIL this test.
    • If any such "jumps" were directly tied to you moving your finger quickly, do not fail this test.
  6. Release the SHIFT key (held since step 2).
  7. Verify that the shift layer (all uppercase) remains visible and that the flick animation is still active.
  8. Return your finger to its original position; the 'U' key should slide back into place.
  9. Now, drag your finger in a up-and-right motion: you should also see a preview for Ú as you do so.
    • Again, if the animation felt "jumpy" and not well connected to your input, FAIL this test.
  10. While the Ú is visible and in the center, tap on the Q key.
  11. Verify that the keyboard returns to the default (lowercase) layer and that either Ú or Úq is output.
    • Úq is ideal, but if your a tap is quick, you may only get the Ú.

I will note that sometimes, when doing the input quickly, I get only Ú (and not the q). Not sure exactly why that is, but I felt it may be worth noting... and worth a future investigation. It is likely typing-speed related, since there's a fair bit of await-async there.

@keymanapp-test-bot keymanapp-test-bot bot added the user-test-missing User tests have not yet been defined for the PR label Nov 8, 2023
@keymanapp-test-bot
Copy link
Copy Markdown

keymanapp-test-bot bot commented Nov 8, 2023 •
edited
Loading

User Test Results

Test specification and instructions

  • ✅ TEST_10_KEY_ROTA (PASSED): Tested using the standard "Test unminified KeymanWeb" test page from a mobile device (Samsung Galaxy A23 - Android 13) and here is my observation: 1. Opened "English - Classic 10-key" keyboard. 2. Verified that all of the keys numbered from 1 to 9 aside from 7 have three or more lowercase letters as key hints. 3. Followed steps 3 and 4 and verified that the expected output is 8a. 4. Tapped the 8 key continuously for at least 2 seconds and verified that it output different combination of letters and numbers. Stopped tapping when the new character is a capital letter. Verified that the key hints are now capitalized. 5. Double-tapped the 9 key once. A 'D' key appeared in the text area. 6. Repeatedly tapping 9 for at least 5 seconds and verified that the key hits change from lowercase letter to uppercase letter or uppercase letter to lowercase letter. (notes)
  • ✅ TEST_10_KEY_DIACRITICS (PASSED): Retested using the standard "Test unminified KeymanWeb" test page from a mobile device (Samsung Galaxy A23 - Android 13) and here is my observation: 1. Selected "English - Diacritic 10-key Rota" keyboard. 2. Tapped the ◌̀ key. Then, double-tapped the 8 Key. Verified that the expected result à appears in the text area. 3. Triple-tapped the ◌̀ key. Triped tapped the 9 key. Verified that the àê letters appeared on the screen. 4. Tapped the '◌̀' key. Then, tapped the 4 key once. Hit backspace. Double-tapped the 8 key showed the expected result àềa. (notes)
  • ✅ TEST_APP_10_KEY_DIACRITICS (PASSED): Retested using the Keyman for Android test artifact.. from a mobile device (Samsung Galaxy A23 - Android 13) and here is my observation: 1. Installed the attached Keyman build 17.0.209-alpha-test-9973 in the device. 2. Downloaded and then installed diacritic_rota.kmp file in Keyman App. 3. Tapped the 7 key twice. Verified that 77 appears on the screen. 4. After running step 5, able to see 77à as a result. 5. After running step 7, able to see the 77àê as a result. 6. After running step 11, able to see 77àềa as a result. (notes)
  • ✅ TEST_BASIC_SIMPLE_SHIFT (PASSED): Retested using the standard "Test unminified KeymanWeb" test page from a mobile device (Samsung Galaxy A23 - Android 13) and here is my observation: 1. Selected "Norwegian" keyboard. 2. After running step 3, it is verified that the keyboard remains on the shift layer. (notes)
  • ✅ TEST_BASIC_MODIPRESS (PASSED): Retested using the standard "Test unminified KeymanWeb" test page from a mobile device (Samsung Galaxy A23 - Android 13) and here is my observation: 1. Selected "Norwegian" keyboard. 2. After running step 8, it is verified that the keyboard changed to the default layer. (notes)
  • ✅ TEST_BASIC_MODIPRESS_HOLD (PASSED): Retested using the standard "Test unminified KeymanWeb" test page from a mobile device (Samsung Galaxy A23 - Android 13) and here is my observation: 1. Selected "Norwegian" keyboard. 2. After running step 6, it is verified that the keyboard changed to the default layer due to step 6. (notes)
  • ✅ TEST_NUMERIC_FROM_SHIFT (PASSED): Retested using the standard "Test unminified KeymanWeb" test page from a mobile device (Samsung Galaxy A23 - Android 13) and here is my observation: 1. Selected "Norwegian" keyboard. 2. Verified that ‱ is emitted as text and the keyboard changed back to the shift layer, after running steps 6 and 8. 3. Verified that the keyboard automatically reverted to the default layer, after running step 11. (notes)
  • ✅ TEST_DELAYED_SUBKEY (PASSED): Retested using the standard "Test unminified KeymanWeb" test page from a mobile device (Samsung Galaxy A23 - Android 13) and here is my observation: 1. Selected "Norwegian" keyboard. 2. Verified that the keyboard keyboard changed to the numeric layer after running step 4. 3. Also, verified that the keyboard changes remain on the numeric layer, with the subkey menu still displayed, after running step 8. 4. Verified that ‱ is emitted as text and that the layer has changed back to the shift layer. (notes)
  • ✅ TEST_DOUBLETAP_CAPS (PASSED): Retested using the standard "Test unminified KeymanWeb" test page from a mobile device (Samsung Galaxy A23 - Android 13) and here is my observation: 1. Selected "Norwegian" keyboard. 2. Verified that it is showing the expected outputs after running from 2 to 8. (notes)
  • ✅ TEST_CUSTOM_MULTITAP_MODIFIER (PASSED): In which I update a test spec again; had the wrong output specified for a step, but the test results were correct for actual (and intended!) behavior.
  • ✅ TEST_ALTERNATING_SHIFT_AND_KEY (PASSED): Retested using the standard "Test unminified KeymanWeb" test page from a mobile device (Samsung Galaxy A23 - Android 13) and here is my observation: 1. Selected "Gesture Prototyping" keyboard. 2. Verified that an H is output and the keyboard layer returns to default layer after running step 4. 3. Verified that there is no deviations in the output after running step 7. 4. Verified that the layer switches properly to keys with numbers in the top row and symbols in the middle two rows, after clicking the 'num' key. (notes)
  • ✅ TEST_FLICKS_BASIC (PASSED): Retested using the standard "Test unminified KeymanWeb" test page from a mobile device (Samsung Galaxy A23 - Android 13) and here is my observation: 1. Selected "Gesture Prototyping" keyboard. 2. After running step 4, it is verified that it outputs ù on the screen. 3. After running step 6, it is verified that it outputs ù on the screen. 4. After running step 8, it is verified that it outputs û on the screen. (notes)
  • ✅ TEST_FLICK_CORRECTION (PASSED): In which it turns out another test spec needed updating to match current behavior. I think I wrote that test before the locked-flicks stuff was fully implemented; it makes sense for step 8 to emit that 'u', now that I inspect it more closely. (notes)
  • ✅ TEST_FLICK_LOCKING (PASSED): Retested using the standard "Test unminified KeymanWeb" test page from a mobile device (Samsung Galaxy A23 - Android 13) and here is my observation: 1. Selected "Gesture Prototyping" keyboard. 2. Tapped and hold the 'u' key. 3. Dragged it purely straight-up position, and I was able to see the letter û. Observed that there was no lack of motion after dragging it. 4. After moving back my finger to the original position, the 'u' key slides back into place. 5. Tapped and hold the 'u' key, then drag it in an up-and-right motion. I was able to see a preview of ú key. Observed that no animated preview happened. Lift my finger and end the gesture. Verified that a ú outputs on the screen. 6. Followed steps 2 through 6 for the 'a' key and I am able to see the similar previews and text there. Seems to be working fine. Thanks. (notes)
  • ✅ TEST_MODIPRESS_MULTITAP_FLICK_PREVIEW (PASSED): Retested using the standard "Test unminified KeymanWeb" test page from a mobile device (Samsung Galaxy A23 - Android 13) and here is my observation: 1. Selected "Gesture Prototyping" keyboard. 2. Double tapped then hold the Shift key. Tapped and hold the U key. Verified that the key preview is visible. 3. Dragged the finger on U purely north (straight up) and I was able to see Û key in the preview area. 4. Verified that the flick-animation in the preview scrolls to a Û. 5. Release the flick, when the Û key is fully visible. Verified that Û is output. 6. Repeated steps 2 through 6 quickly three or four times and verified that the Û appears repeatedly as the output. Seems to be working fine. (notes)
  • ✅ TEST_FLICK_DURING_MODIPRESS (PASSED): Retested using the standard "Test unminified KeymanWeb" test page from a mobile device (Samsung Galaxy A23 - Android 13) and here is my observation: 1. Selected "Gesture Prototyping" keyboard. 2. Tapped and hold the Shift key. Tapped and hold the U key. 3. Dragged it to straight-up position. I was able to see the Û key without "jumpy" in the preview area. Released the Shift key. Verified that the shift layer remains visible and that the flick animation is still active. 4. Returned the finger to its original position, able to see the U key slide back into place. 5. Dragged the finger in an up-and-right motion and I was able to see the Ú key in the preview. 6. While the Ú is visible in the center, tapped on the q key with the other finger. Verified that the keyboard returns to the default layer and Úq is output on the screen. Seems to be working fine. (notes)

Test Artifacts

@keymanapp-test-bot keymanapp-test-bot bot changed the title feat(web): delayed modipress completion feat(web): delayed modipress completion 🐵 Nov 8, 2023
@keymanapp-test-bot keymanapp-test-bot bot added this to the A17S25 milestone Nov 8, 2023
@mcdurdin mcdurdin modified the milestones: A17S25, A17S26 Nov 13, 2023
@jahorton jahorton changed the base branch from change/web/polish-pass to feat/web/flick-lock-polish November 13, 2023 08:50
@jahorton jahorton force-pushed the feat/web/delayed-modipress-completion branch from deb35e2 to f47fd0d Compare November 14, 2023 02:56
@jahorton jahorton mentioned this pull request Nov 14, 2023
Merged
@jahorton jahorton changed the base branch from feat/web/flick-lock-polish to change/web/polish-pass November 14, 2023 07:11
@jahorton jahorton mentioned this pull request Nov 14, 2023
Merged
@keymanapp-test-bot keymanapp-test-bot bot added has-user-test user-test-required User tests have not been completed and removed user-test-missing User tests have not yet been defined for the PR labels Nov 14, 2023
@jahorton jahorton marked this pull request as ready for review November 14, 2023 08:28
@jahorton jahorton requested a review from mcdurdin as a code owner November 14, 2023 08:28
mcdurdin
mcdurdin approved these changes Nov 15, 2023
View reviewed changes
common/web/gesture-recognizer/src/engine/headless/touchpointCoordinator.ts
// For multitouch gestures, only report the gesture **once**.
const sourceIDs = selection.matcher.allSourceIds;
for(let sequence of this._activeGestures) {
if(!!sequence.allSourceIds.find((id1) => !!sourceIDs.find((id2) => id1 == id2))) {
Copy link
Copy Markdown
Member

@mcdurdin mcdurdin Nov 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

O(m*n)? Are these arrays short?

Copy link
Copy Markdown
Contributor Author

@jahorton jahorton Nov 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A person only has ten fingers (corresponding to allSourceIds), and I'd be quite impressed if someone actually managed to use each of them independently while building concurrent gestures (which corresponds to the for... let part). Technically, completed touches can be included in the allSourceIds array - it's needed for multitap history.

Secondly, with the way Web currently works, I believe the maximum count for concurrent gestures is 2; a modipress and whatever the current input is on the modipress layer. Starting a third will immediately resolve the previously-current input (simple tap, flick, multitap) or be blocked (longpress). There's a chance of it actually being 3 - I'm not 100% sure that a held backspace will be auto-cancelled on new input. But yeah, that should be it.

This does depend on proper state management of the 'active gestures' array. If errors happen that prevent removal of finished gesture objects from that array, then over time it may become "less short" than intended.

The potential for most growth is in allSourceIds if someone goes crazy with constant 'rota' key mashing, but even then, it's still practically just O(n) due to the low cap on this._activeGestures's practical length.

Copy link
Copy Markdown
Member

@mcdurdin mcdurdin Nov 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what I figured but helpful info on the potential edge cases, thanks!

@bharanidharanj
Copy link
Copy Markdown

bharanidharanj commented Nov 15, 2023

Test Results

  • TEST_10_KEY_ROTA (PASSED): Tested using the standard "Test unminified KeymanWeb" test page from a mobile device (Samsung Galaxy A23 - Android 13) and here is my observation: 1. Opened "English - Classic 10-key" keyboard. 2. Verified that all of the keys numbered from 1 to 9 aside from 7 have three or more lowercase letters as key hints. 3. Followed steps 3 and 4 and verified that the expected output is 8a. 4. Tapped the 8 key continuously for at least 2 seconds and verified that it output different combination of letters and numbers. Stopped tapping when the new character is a capital letter. Verified that the key hints are now capitalized. 5. Double-tapped the 9 key once. A 'D' key appeared in the text area. 6. Repeatedly tapping 9 for at least 5 seconds and verified that the key hits change from lowercase letter to uppercase letter or uppercase letter to lowercase letter.

..1to9 numbers with key hints

..Tap the 8 key twice in quick succession

..Tap the 8 key continuously for at least 2 seconds

..Repeatedly tap 9 for at least five seconds..

@bharanidharanj
Copy link
Copy Markdown

bharanidharanj commented Nov 15, 2023

Test Results

  • TEST_10_KEY_DIACRITICS (PASSED): Tested using the standard "Test unminified KeymanWeb" test page from a mobile device (Samsung Galaxy A23 - Android 13) and here is my observation: 1. Selected "English - Diacritic 10-key Rota" keyboard. 2. Tapped the ◌̀ key. Then, double tapped the 8 Key. Verified that the expected result à appear on the text area. 3. Triple-tapped the ◌̀ key. Triped tapped the 9 key. Verified that the àê letters appeared on the screen. 4. Tapped the '◌̀' key. Then, tapped the 4 key once. Hit backspace. Double-tapped the 8 key showed the expected result àềa.

..result 1

..result 2

..result 3

@bharanidharanj
Copy link
Copy Markdown

bharanidharanj commented Nov 16, 2023

Test Results

  • TEST_CUSTOM_MULTITAP_MODIFIER (FAILED): Tested using the standard "Test unminified KeymanWeb" test page from the mobile device (Samsung Galaxy A23 - Android 13) and here is my observation: 1. Selected "English - Gesture Prototyping" keyboard. 2. Single-tapped the ◌́ key. Noticed that the number of letters, including all of the vowels are changed to versions using the wrong output of the accent mark. Seems to be an issue.

@jahorton
Copy link
Copy Markdown
Contributor Author

jahorton commented Nov 22, 2023 •
edited
Loading

@keymanapp-test-bot retest TEST_10_KEY_DIACRITICS

@bharanidharanj That failed test is on me; I forgot that we changed a related behavior with #9944. I've updated the test instructions accordingly.

Turns out that I discovered a bug on master while doing so; for the sake of user testing, I've gone ahead and fixed it here... and 🍒-picked it as #10039 for master. It'd require a retooling of the test keyboard and heavier retooling of the instructions otherwise.

Also, something probably worth note/making an issue: TEST_APP_10_KEY_DIACRITICS will fail if performed on an iOS device if the 7 key step is removed... if the text area is empty before starting the test. My intuition tells me that this is due to one of the iOS quirks we noticed a while back: the iOS equivalent to a text-services framework signals a context reset any time the active text-input receiver has its text cleared. Well... multitapping on a context-initial diacritic-output-only key probably isn't helping things there, possibly causing a context-desync between the app and the keyboard engine. That doesn't seem entirely right, but it's probably quite related.

As it appears to be due to something iOS-specific and does not occur on Android, I think this issue should be documented and pursued separately.

On Android, the context-initial diacritic doesn't seem to get displayed properly in the emulator setup I use most frequently, but it is still present and does still combine properly according to keyboard rules. (Notably, placeholder text disappears when it should be visible, indicating the presence of text... even if it - the isolated diacritic - is not being displayed properly.) That'd be more of a rendering issue within the app and/or on Android, I guess.

@jahorton jahorton mentioned this pull request Nov 22, 2023
Merged
@jahorton
Copy link
Copy Markdown
Contributor Author

jahorton commented Nov 22, 2023 •
edited
Loading

Also, something probably worth note/making an issue: TEST_APP_10_KEY_DIACRITICS will fail if performed on an iOS device if the 7 key step is removed... if the text area is empty before starting the test. My intuition tells me that this is due to one of the iOS quirks we noticed a while back: the iOS equivalent to a text-services framework signals a context reset any time the active text-input receiver has its text cleared. Well... multitapping on a context-initial diacritic-output-only key probably isn't helping things there, possibly causing a context-desync between the app and the keyboard engine. That doesn't seem entirely right, but it's probably quite related.

As it appears to be due to something iOS-specific and does not occur on Android, I think this issue should be documented and pursued separately.

Eh, why not.

Did a little investigation: our old enemy, U+200E (https://unicode-explorer.com/c/200E) has returned to haunt us. It's being automatically inserted at the start of the context whenever a diacritic is added in that position or directly after a different diacritic meeting this compound condition (e.g., it can chain).

This is occurring via setKeymanVal calls to the ios-host.js script:

keyman/ios/engine/KMEI/KeymanEngine/resources/Keyman.bundle/Contents/Resources/ios-host.js

Lines 292 to 295 in 6b8a06a

if(keyman.context.getText() != text) {
keyman.context.setText(text);
keyman.resetContext();
}

This has been verified via breakpoint when performing a repro for the conditions described above - that if-condition will trigger, resulting in, among other things, a deadkey-wipe.

The Swift-side interface that calls this:

keyman/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/KeymanWebViewController.swift

Lines 231 to 251 in 6b8a06a

func setText(_ text: String?) {
var text = text ?? ""
// Remove any system-added LTR/RTL marks.
text = text.replacingOccurrences(of: "\u{200e}", with: "") // Unicode's LTR codepoint
text = text.replacingOccurrences(of: "\u{200f}", with: "") // Unicode's RTL codepoint (v1)
text = text.replacingOccurrences(of: "\u{202e}", with: "") // Unicode's RTL codepoint (v2)
do {
let encodingArray = [ text ];
let jsonString = try String(data: JSONSerialization.data(withJSONObject: encodingArray), encoding: .utf8)!
let start = jsonString.index(jsonString.startIndex, offsetBy: 2)
let end = jsonString.index(jsonString.endIndex, offsetBy: -2)
let jsonText = jsonString[start..<end]
self.currentText = String(jsonText)
webView!.evaluateJavaScript("setKeymanVal(\"\(jsonText)\");", completionHandler: nil)
} catch {
SentryManager.captureAndLog(error.localizedDescription)
}
}

Well, huh. That should handle it, right? Well... here's the thing...

Screen Shot 2023-11-22 at 10 27 52 AM

Screen Shot 2023-11-22 at 10 26 16 AM

So... there's no match, the byte-header doesn't get filtered, do not pass Go, do not collect $200, etc. Of course, that did work before... so a recent-ish version of iOS must have changed how the byte-header gets encoded. The filter-block was written in #1756 for version 14.0 in May 2019, and there have been quite a few new versions of iOS since then.

Also, I'd noticed weirdness with predictive-text even on master - how an extra backspace was needed after reaching cleared text at the start of context for predictions to work right. It's definitely the same thing - our pred-text engine doesn't filter out byte-order marks either.

@jahorton jahorton mentioned this pull request Nov 22, 2023
Closed
@jahorton
Copy link
Copy Markdown
Contributor Author

jahorton commented Nov 22, 2023

Given the investigation and related behaviors I've noted, it's a bug on the current master and thus should be handled separately.

@jahorton
Copy link
Copy Markdown
Contributor Author

jahorton commented Nov 22, 2023

And, of course, fixing the fact that context-resets weren't resetting deadkeys (a la #10039)... caused me to run into a different context-sync issue, at least on iOS. Now checking if it also happens on Android.

Core issue there: there are two text updates during a multitap: 'restore original context' and then 'apply new text effects'. If text is updated from the app based on the first and not the second, while the JS side has done both, that causes a context desync.

@jahorton
Copy link
Copy Markdown
Contributor Author

jahorton commented Nov 22, 2023

As that last comment is something only relevant for feature-gestures - because of multitaps - I went ahead and fixed that here.

@bharanidharanj
Copy link
Copy Markdown

bharanidharanj commented Nov 22, 2023 •
edited
Loading

Test Results

  • TEST_10_KEY_DIACRITICS (PASSED): Retested using the standard "Test unminified KeymanWeb" test page from a mobile device (Samsung Galaxy A23 - Android 13) and here is my observation: 1. Selected "English - Diacritic 10-key Rota" keyboard. 2. Tapped the ◌̀ key. Then, double-tapped the 8 Key. Verified that the expected result à appears in the text area. 3. Triple-tapped the ◌̀ key. Triped tapped the 9 key. Verified that the àê letters appeared on the screen. 4. Tapped the '◌̀' key. Then, tapped the 4 key once. Hit backspace. Double-tapped the 8 key showed the expected result àềa.

..Double-tap 8 key

..Triple-tap 9 key

..Double-tap the 8 key after the change

@bharanidharanj
Copy link
Copy Markdown

bharanidharanj commented Nov 22, 2023 •
edited
Loading

Test Results

  • TEST_APP_10_KEY_DIACRITICS (PASSED): Retested using the Keyman for Android test artifact.. from a mobile device (Samsung Galaxy A23 - Android 13) and here is my observation: 1. Installed the attached Keyman build 17.0.209-alpha-test-9973 in the device. 2. Downloaded and then installed diacritic_rota.kmp file in Keyman App. 3. Tapped the 7 key twice. Verified that 77 appears on the screen. 4. After running step 5, able to see 77à as a result. 5. After running step 7, able to see the 77àê as a result. 6. After running step 11, able to see 77àềa as a result.

..Double-tap the 8 key

..Triple-tap the 9-key

..Double-tap the 8th key

@bharanidharanj
Copy link
Copy Markdown

bharanidharanj commented Nov 22, 2023

Test Results

  • TEST_BASIC_SIMPLE_SHIFT (PASSED): Retested using the standard "Test unminified KeymanWeb" test page from a mobile device (Samsung Galaxy A23 - Android 13) and here is my observation: 1. Selected "Norwegian" keyboard. 2. After running step 3, it is verified that the keyboard remains on the shift layer.

  • TEST_BASIC_MODIPRESS (PASSED): Retested using the standard "Test unminified KeymanWeb" test page from a mobile device (Samsung Galaxy A23 - Android 13) and here is my observation: 1. Selected "Norwegian" keyboard. 2. After running step 8, it is verified that the keyboard changed to the default layer.

  • TEST_BASIC_MODIPRESS_HOLD (PASSED): Retested using the standard "Test unminified KeymanWeb" test page from a mobile device (Samsung Galaxy A23 - Android 13) and here is my observation: 1. Selected "Norwegian" keyboard. 2. After running step 6, it is verified that the keyboard changed to the default layer due to step 6.

@bharanidharanj
Copy link
Copy Markdown

bharanidharanj commented Nov 22, 2023

Test Results

  • TEST_NUMERIC_FROM_SHIFT (PASSED): Retested using the standard "Test unminified KeymanWeb" test page from a mobile device (Samsung Galaxy A23 - Android 13) and here is my observation: 1. Selected "Norwegian" keyboard. 2. Verified that ‱ is emitted as text and the keyboard changed back to the shift layer, after running steps 6 and 8. 3. Verified that the keyboard automatically reverted to the default layer, after running step 11.

..After running step 7

..After running step 12

@bharanidharanj
Copy link
Copy Markdown

bharanidharanj commented Nov 22, 2023

Test Results

  • TEST_DELAYED_SUBKEY (PASSED): Retested using the standard "Test unminified KeymanWeb" test page from a mobile device (Samsung Galaxy A23 - Android 13) and here is my observation: 1. Selected "Norwegian" keyboard. 2. Verified that the keyboard keyboard changed to the numeric layer after running step 4. 3. Also, verified that the keyboard changes remain on the numeric layer, with the subkey menu still displayed, after running step 8. 4. Verified that ‱ is emitted as text and that the layer has changed back to the shift layer.

@bharanidharanj
Copy link
Copy Markdown

bharanidharanj commented Nov 22, 2023

Test Results

  • TEST_DOUBLETAP_CAPS (PASSED): Retested using the standard "Test unminified KeymanWeb" test page from a mobile device (Samsung Galaxy A23 - Android 13) and here is my observation: 1. Selected "Norwegian" keyboard. 2. Verified that it is showing the expected outputs after running from 2 to 8.

@bharanidharanj
Copy link
Copy Markdown

bharanidharanj commented Nov 22, 2023

Test Results

  • TEST_CUSTOM_MULTITAP_MODIFIER (FAILED): Retested using the standard "Test unminified KeymanWeb" test page from a mobile device (Samsung Galaxy A23 - Android 13) and here is my observation: 1. Selected "Gesture Prototyping" keyboard. 2. Noticed that after running step 4, it is showing the wrong output in the text screen.

@bharanidharanj
Copy link
Copy Markdown

bharanidharanj commented Nov 22, 2023

Test Results

  • TEST_ALTERNATING_SHIFT_AND_KEY (PASSED): Retested using the standard "Test unminified KeymanWeb" test page from a mobile device (Samsung Galaxy A23 - Android 13) and here is my observation: 1. Selected "Gesture Prototyping" keyboard. 2. Verified that an H is output and the keyboard layer returns to default layer after running step 4. 3. Verified that there is no deviations in the output after running step 7. 4. Verified that the layer switches properly to keys with numbers in the top row and symbols in the middle two rows, after clicking the 'num' key.

..H is the output after running step 5

..No deviation in the output after running step 7

..Numeric key appears after running step 8

@bharanidharanj
Copy link
Copy Markdown

bharanidharanj commented Nov 22, 2023

Test Results

  • TEST_FLICKS_BASIC (PASSED): Retested using the standard "Test unminified KeymanWeb" test page from a mobile device (Samsung Galaxy A23 - Android 13) and here is my observation: 1. Selected "Gesture Prototyping" keyboard. 2. After running step 4, it is verified that it outputs ù on the screen. 3. After running step 6, it is verified that it outputs ù on the screen. 4. After running step 8, it is verified that it outputs û on the screen.

@bharanidharanj
Copy link
Copy Markdown

bharanidharanj commented Nov 22, 2023

Test Results

  • TEST_FLICK_CORRECTION (FAILED): Retested using the standard "Test unminified KeymanWeb" test page from a mobile device (Samsung Galaxy A23 - Android 13) and here is my observation: 1. Selected "Gesture Prototyping" keyboard. 2. After running step 4, it is verified that it outputs ù on the screen. 3. After running step 8, it is observed that the u letter appears on the screen. seems to be an issue. 4. After running step 11, it is verified that it outputs u on the screen.

@jahorton
Copy link
Copy Markdown
Contributor Author

jahorton commented Nov 23, 2023 •
edited
Loading

Test Results

  • TEST_CUSTOM_MULTITAP_MODIFIER (FAILED): Retested using the standard "Test unminified KeymanWeb" test page from a mobile device (Samsung Galaxy A23 - Android 13) and here is my observation: 1. Selected "Gesture Prototyping" keyboard. 2. Noticed that after running step 4, it is showing the wrong output in the text screen.

Could you please elaborate on what the error is here? In the screenshot above... isn't that the expected output from the a key from the layer reached by triple-tap?

To be clear - from this layer:

image

... wait a second. Step 4 - the first key. And yep, test-spec error again. That output is correct for the layer you should be reaching. Man, I did not proofread the tests with the varied diacritic forms correctly.

TEST_CUSTOM_MULTITAP_MODIFIER (PASSED): In which I update a test spec again; had the wrong output specified for a step, but the test results were correct for actual (and intended!) behavior.

@jahorton
Copy link
Copy Markdown
Contributor Author

jahorton commented Nov 23, 2023

Test Results

  • TEST_FLICK_CORRECTION (FAILED): Retested using the standard "Test unminified KeymanWeb" test page from a mobile device (Samsung Galaxy A23 - Android 13) and here is my observation: 1. Selected "Gesture Prototyping" keyboard. 2. After running step 4, it is verified that it outputs ù on the screen. 3. After running step 8, it is observed that the u letter appears on the screen. seems to be an issue. 4. After running step 11, it is verified that it outputs u on the screen.

TEST_FLICK_CORRECTION (PASSED): In which it turns out another test spec needed updating to match current behavior. I think I wrote that test before the locked-flicks stuff was fully implemented; it makes sense for step 8 to emit that 'u', now that I inspect it more closely.

I've gone ahead and adjusted the test-spec accordingly.

@bharanidharanj
Copy link
Copy Markdown

bharanidharanj commented Nov 23, 2023 •
edited
Loading

Test Results

  • TEST_FLICK_LOCKING (PASSED): Retested using the standard "Test unminified KeymanWeb" test page from a mobile device (Samsung Galaxy A23 - Android 13) and here is my observation: 1. Selected "Gesture Prototyping" keyboard. 2. Tapped and hold the 'u' key. 3. Dragged it purely straight-up position, and I was able to see the letter û. Observed that there was no lack of motion after dragging it. 4. After moving back my finger to the original position, the 'u' key slides back into place. 5. Tapped and hold the 'u' key, then drag it in an up-and-right motion. I was able to see a preview of ú key. Observed that no animated preview happened. Lift my finger and end the gesture. Verified that a ú outputs on the screen. 6. Followed steps 2 through 6 for the 'a' key and I am able to see the similar previews and text there. Seems to be working fine. Thanks.

..u text animated preview

..a text-animated preview

@bharanidharanj
Copy link
Copy Markdown

bharanidharanj commented Nov 23, 2023

Test Results

  • TEST_MODIPRESS_MULTITAP_FLICK_PREVIEW (PASSED): Retested using the standard "Test unminified KeymanWeb" test page from a mobile device (Samsung Galaxy A23 - Android 13) and here is my observation: 1. Selected "Gesture Prototyping" keyboard. 2. Double tapped then hold the Shift key. Tapped and hold the U key. Verified that the key preview is visible. 3. Dragged the finger on U purely north (straight up) and I was able to see Û key in the preview area. 4. Verified that the flick-animation in the preview scrolls to a Û. 5. Release the flick, when the Û key is fully visible. Verified that Û is output. 6. Repeated steps 2 through 6 quickly three or four times and verified that the Û appears repeatedly as the output. Seems to be working fine.

@bharanidharanj
Copy link
Copy Markdown

bharanidharanj commented Nov 23, 2023

Test Results

  • TEST_FLICK_DURING_MODIPRESS (PASSED): Retested using the standard "Test unminified KeymanWeb" test page from a mobile device (Samsung Galaxy A23 - Android 13) and here is my observation: 1. Selected "Gesture Prototyping" keyboard. 2. Tapped and hold the Shift key. Tapped and hold the U key. 3. Dragged it to straight-up position. I was able to see the Û key without "jumpy" in the preview area. Released the Shift key. Verified that the shift layer remains visible and that the flick animation is still active. 4. Returned the finger to its original position, able to see the U key slide back into place. 5. Dragged the finger in an up-and-right motion and I was able to see the Ú key in the preview. 6. While the Ú is visible in the center, tapped on the q key with the other finger. Verified that the keyboard returns to the default layer and Úq is output on the screen. Seems to be working fine.

@keymanapp-test-bot keymanapp-test-bot bot removed the user-test-required User tests have not been completed label Nov 23, 2023
@jahorton jahorton merged commit 6fdd966 into feature-gestures Nov 24, 2023
@jahorton jahorton deleted the feat/web/delayed-modipress-completion branch November 24, 2023 04:36
@jahorton jahorton mentioned this pull request Nov 24, 2023
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mcdurdin mcdurdin mcdurdin approved these changes

Assignees

No one assigned

Labels

common/web/ common/ epic-gestures feat has-user-test web/

Projects

None yet

Milestone

A17S26

Development

Successfully merging this pull request may close these issues.

3 participants

@jahorton @bharanidharanj @mcdurdin