Fix erroneous keyswipes

[devycarol]

This patch fixes the bug where keyswipe gestures can be triggered if they're hovered when browsing popup-keys panels.

It also keeps the space bar from preventing alternative move-events if it doesn't have a swipe gesture enabled. Examples: a space-swipe for the other direction, popup-panel browsing, and gesture typing (so same behavior as keys like return, punctuations, and AOSP space).

This fix opens the door to possibly having keys with pop-ups and swipe gestures in the future. I also moved keyswipe handling into a separate function to help with that to-do comment. Let me know if I misinterpreted "and finally extend it" when re-writing the comment.

Something I'm curious about: should this mDidShowPopupKeys be set to false if onCancelEvent() is called? I notice that method doesn't really adjust any of the fields.

Fixes #918

[devycarol]

I also added a requirement that keyswipes can't start if a key is repeating. That way the delete swipe won't trigger if it's started repeating, and you can't trigger something like a spacebar gesture when dragging out of a repeating delete key (case where space gesture is enabled and delete swipe isn't)

[devycarol]

Honestly, would you just prefer the behavior to be "don't initiate a keyswipe unless the swiping key was the one that was initially pressed down on?" Sorry this has kind of expanded past the scope of the initial issue.

I've discovered all manner of edge-cases, like an early gesture-typing gesture getting broken by a keyswipe, sliding your finger across the keys of a non-gestural layout, etc. And, I worry that the current changes I have here are difficult to read and keep track of. The latter approach would make it simpler I think, like the way sliding key input is activated.

[Helium314]

Honestly, would you just prefer the behavior to be "don't initiate a keyswipe unless the swiping key was the one that was initially pressed down on?"

This sounds like it makes sense, and should be a simple change.
Does it solve all the other edge cases you mentioned? Or do you see any problems with that simple solution?

I also moved keyswipe handling into a separate function to help with that to-do comment. Let me know if I misinterpreted "and finally extend it" when re-writing the comment.

What I had planned is to change it into a more generic thing, where the KeyboardActionListener gets a function like onKeySwipe(code, steps, direction), which returns false if there is no swipe action, and something like onUpWhileSwiping(code). So the logic of deciding how which key is swiped is moved out of the pointer tracker into a more suitable place, where it can be extended later.

Something I'm curious about: should this mDidShowPopupKeys be set to false if onCancelEvent() is called? I notice that method doesn't really adjust any of the fields.

What is mDidShowPopupKeys? I can't find it in the code. Was it in a previous state of this PR?

[devycarol]

Was it in a previous state of this PR?

It was, sorry. Basically the question is: should the state of all the field variables be reset in that onCancelEvent() method? mInHorizontalSwipe, mIsInSlidingKeyInput, etc.. I don't know the circumstances where it'd be called.

Does it solve all the other edge cases you mentioned? Or do you see any problems with that simple solution?

It should solve them, I just need to make sure that key-swipes still get disallowed when the pop-up keys are shown to preserve the possibility of those coexisting. And also for the repeats.

In my D-pad prototype with a trackpad-swipe on the 'select' key, it's kinda nifty for that key to intercept a touch-drag from one of the arrows and initiate the cursor movement. But I don't think that's super necessary, and could really just be replicated in a more polished way by applying that trackpad gesture to all the cursor keys in that layout.

I also finagled the space bar to have a sliding-input peek to the d-pad as a vertical gesture option, so this fix may need to be re-worked a bit later on if we wish to have that behavior in the final version. An alternative is to place that 'peek' gesture option on the return key.

[devycarol]

Just spotted #566. I think some of those restrictions could be lifted in favor of requiring that the swiping key be the initially-touched one. For instance, accidental delete gestures are often triggered when 'delete' wasn't the initial key that was tapped.

[devycarol]

So now keyswipes only trigger when it was the initial key that was pressed, it hasn't started repeating, and a pop-up panel hasn't been shown. This in tandem with the pop-up/repeat patch in #932 means the app now gracefully handles swipers, repeaters, and keys with popups. The only incompatible pairing is the latter two, where popups will take precedence.

The timeout delay for the spacebar now applies to the delete swipe as well. Because of these things, I've allowed it to start more leniently. I've also made it so it only applies when the user is fast-typing—the original issue suggests that that's the main case where a timeout is warranted.

[devycarol]

While I'm at it, fixed a bug where keyswipes would freeze up when the finger moves above the keyboard. Was especially volatile for the delete swipe.

[Helium314]

Thanks!
Two (minor) things:

• Could you add a comment to keySwipe regarding the return value? It's not clear from the name alone.
• Space swipe is now always initiated, even if only one direction is enabled and the swipe was clearly in the other direction. I think this behavior can be rather unexpected. As far as I remember I intentionally wanted to avoid this.

[devycarol]

Oh, I have a better solution then. If you do e.g. a horizontal swipe and the setting is 'none', then switch off keySwipeAllowed and return false. That way it won't start a swipe action, but also won't lock the pointer out of further actions like gesture typing.

I'll try that and make sure it works :)

[devycarol]

Actually, I remember why I did that. When you have the combination of horizontal cursor movement with no vertical swipe, the gesture isn't as permissive as it believe should be. Before vertical swipes, you could do a little swipe up at the start of your cursor movement—this can be a lot more comfortable than trying to constrain your dragging within the y-bounds of the space bar.

It's your repo, I can certainly keep it as homebrew if you'd like me to change it back.

Also, turns out that method doesn't need to be a boolean after all. The idea was "you might want to have the keyswipe conditionally rejected, in which case you proceed with the rest of the move event logic." But I didn't wind up needing it, since mKeySwipeAllowed <- isSwiper() is pretty solid.

[devycarol]

Over all I'm really excited about this one! Now with these fixes, the delete swipe is actually something that I can realistically use—this one was a real killer 😂

I love open source 🫶🫶

[devycarol]

Really hoping I don't have to do that again. Will try and do these in a more one-at-a-time fashion.

Here's the overall changes:

• Keyswipes now only trigger when they were the initial key that was pressed.
• Keyswipe start-lag now applies to all swiper keys, but only during fast-typing.
• Because of those two things, their general start conditions are now a bit more permissive. This makes them smoother with less appearance of lag.
• Keyswipes and word-gestures are now mutually exclusive, hopefully fixing Glide typing: Suggested word stuck above the keys #539.
  • Error cases involving sliding input (starting a different kind of gesture while sliding) remain un-resolved. I think this PR is 'full,' so that would be addressed later.

[Helium314]

Thanks for summarizing again! It's already late here, so I'll review it tomorrow.

[Helium314]

Actually, I remember why I did that. When you have the combination of horizontal cursor movement with no vertical swipe, the gesture isn't as permissive as it believe should be. Before vertical swipes, you could do a little swipe up at the start of your cursor movement—this can be a lot more comfortable than trying to constrain your dragging within the y-bounds of the space bar.

I also find it more comfortable to swipe with the finger somewhere on the keyboard instead of precisely in the space bar. But the restriction is only for initiating the swipe, after that you can move as you like.

My reasoning for being restrictive with initiating the swipe was related to other functionality (currently only language switch, but might be extended).
So when you have language switch on left/right and nothing on up/down, accidental language switch should be avoided when the user e.g. misses the key above and swipes upward. With cursor movement it's not that bad though.

[devycarol]

But the restriction is only for initiating the swipe, after that you can move as you like.

The restriction forces you do to a little zig-zag at the start of your swipe, which can cause more movement than wanted in the case of horizontal cursor control. Removing the restriction (only in the case of no set gesture for the other direction) allows more precise movement by slowly traversing a diagonal.

Would you like to defer changes related to language switch until later? I'd like to add a toggle gesture for the d-pad once it's added, so the behavior of all layout change swipes could be fine-tuned then I think.

Is really is a lot less likely to accidentally language-swipe now that is won't intercept touches that started on other keys anymore.

[Helium314]

There are several issues I have with this change:

• it's changing behavior because of taste, but I think none of the two possibilities is clearly preferable in every case
• it doesn't fit with "fix erroneous keyswipes", and is not in the description (sorry if I really overlooked it, I checked again)
• since it's mentioned, when I notice it I can't know whether the change is intentional or a side effect / bug
  • (my experience with finding such "hidden-but-intended" changes far too often is why I'm asking for a description in the contribution guidelines)

For now I would like to keep the old behavior mostly for consistency. It may need to be adjusted anyway later when extending / generalizing the key swipe functionality, and I think this extension would be a better place to adjust the behavior.

[devycarol]

Sounds good! Rolling back.

[Helium314]

Thanks!