Skip to content
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

moving the cursor does not reset spacebar multi-tap #242

Open
KraXen72 opened this issue May 24, 2023 · 25 comments · Fixed by #613
Open

moving the cursor does not reset spacebar multi-tap #242

KraXen72 opened this issue May 24, 2023 · 25 comments · Fixed by #613
Labels
bug Something isn't working

Comments

@KraXen72
Copy link
Contributor

KraXen72 commented May 24, 2023

Thumb-Key Version
1.3.0

Describe the bug
if you type space, move the cursor and type it again, it deletes the last letter and adds a comma. (i do not want to turn off spacebar multitaps, just want it to start from the beginnig of the character cycle if the cursor moves)

To Steps to reproduce the behavior:
| = cursor

  1. type something like 'noproblem|'
  2. realize misake, move cursor: 'no|problem'
  3. hit space
  4. move cursor: 'no problem|'
  5. hitting space to continue typing instead produces 'no proble, |'
@KraXen72 KraXen72 added the bug Something isn't working label May 24, 2023
@dessalines
Copy link
Owner

I can't replicate this for some reason.

@KraXen72
Copy link
Contributor Author

it seems to happen mostly when you grab the cursor by it's native android handle & move it. not that much when i move it with spacebar swipes

@dessalines
Copy link
Owner

I think I may have found a fix for this one.

@KraXen72
Copy link
Contributor Author

KraXen72 commented Jun 5, 2023

still not fixed in 1.6.0, happened to me 3 times in a row while i moved the cursor thorugh it's native drag handle. i'd suggest maybe checking the cursor position before proceeding with the multitap?

@dessalines dessalines reopened this Jun 5, 2023
@dessalines
Copy link
Owner

Not something I know how to do.

@KraXen72
Copy link
Contributor Author

KraXen72 commented Jun 6, 2023

this is what i mean causes it:
Screenshot_20230606-040457_GitHub.png

Screenshot_20230606-040449_GitHub.png

@dessalines
Copy link
Owner

PRs welcome.

@niccokunzmann
Copy link
Contributor

Another way to reproduce this is:

  • type 123 space space
  • (now, there should be 123, )
  • use the finger to set the cursor between 2 and 3
  • tab space
  • see . 3,

@0xFOSSMan
Copy link
Contributor

0xFOSSMan commented Sep 5, 2023

Another way to reproduce this is:

* type `123` space space

* (now, there should be `123, `)

* use the finger to set the cursor between 2 and 3

* tab space

* see `. 3, `

tried reproducing it exactly like you said, didn't happen to me. Maybe it's fixed now?
#321 (comment) also this comment doesn't mention issues in that situation. But I am not sure since they are moving the cursor with a spacebar-swipe, not the android cursor controls.

@niccokunzmann
Copy link
Contributor

niccokunzmann commented Sep 5, 2023 via email

@0xFOSSMan
Copy link
Contributor

0xFOSSMan commented Sep 5, 2023 via email

@KraXen72
Copy link
Contributor Author

as far as i can tell, this has been fixed by #401 or #424. I can no longer reproduce it! Can someone else try to reproduce it? Thanks!

@0xFOSSMan
Copy link
Contributor

0xFOSSMan commented Sep 20, 2023 via email

@KraXen72
Copy link
Contributor Author

ah okay. i was testing it in the thumb-key input box, and i forgot that it doesen't happen there.

@jm355
Copy link
Contributor

jm355 commented Oct 15, 2023

One possible solution (i haven't looked at the code) would be to have a (configurable duration?) .5 second timeout for the double tap behavior

@jm355
Copy link
Contributor

jm355 commented Jan 9, 2024

I don't know how to add a configurable tap timeout, but this works-ish. If you go fast enough you can still hit the error state, and if you wait too long you lose the double tap. That's fine for double tap but kind of sucks for triple tap. Maybe you could hack it even more such that the timeout only gets checked when tapCount == 1?

diff --git a/app/src/main/java/com/dessalines/thumbkey/ui/components/keyboard/KeyboardKey.kt b/app/src/main/java/com/dessalines/thumbkey/ui/components/keyboard/KeyboardKey.kt
index 7b5e3ab..a1b4e66 100644
--- a/app/src/main/java/com/dessalines/thumbkey/ui/components/keyboard/KeyboardKey.kt
+++ b/app/src/main/java/com/dessalines/thumbkey/ui/components/keyboard/KeyboardKey.kt
@@ -118,6 +118,7 @@ fun KeyboardKey(
     val releasedKey = remember { mutableStateOf<String?>(null) }
 
     var tapCount by remember { mutableIntStateOf(0) }
+    var timeOfLastTap by remember { mutableLongStateOf(0L) }
     val tapActions =
         if (spacebarMultiTaps) {
             buildTapActions(key)
@@ -180,12 +181,13 @@ fun KeyboardKey(
                 // Set the last key info, and the tap count
                 val cAction = key.center.action
                 lastAction.value?.let { lastAction ->
-                    if (lastAction == cAction) {
+                    if (lastAction == cAction && (System.currentTimeMillis() - timeOfLastTap) < 500) {
                         tapCount += 1
                     } else {
                         tapCount = 0
                     }
                 }
+                timeOfLastTap = System.currentTimeMillis()
                 lastAction.value = cAction
 
                 // Set the correct action

In researching this, I came across InputConnection.requestCursorUpdates. I believe you could call that with requestCursorUpdates(CURSOR_UPDATE_MONITOR, CURSOR_UPDATE_FILTER_INSERTION_MARKER), and implement updateCursorAnchorInfo such that it would reset the tap count. That seems like the proper solution, but I couldn't figure out how to do that since I'm pretty unfamiliar with android dev. Anybody have any ideas or tips?

@jm355
Copy link
Contributor

jm355 commented Jan 9, 2024

@dessalines should I make a pull request with that change and a todo comment describing the better cursor updates solution, or would you prefer to wait for the cursor updates solution?

The cursor updates solution could also be used to avoid the case where you end a sentence (which capitalizes the keyboard), then move the cursor manually and accidentally type a capitalized key in the middle of a sentence. The logic on cursor update could include something like

if(!capslock){ 
    if(previousCharIsSpace && charBeforePreviousCharEndsSentence) {
        capitalizeKeyboard()
    } else {
        lowercaseKeyboard()
    } 
}

@dessalines
Copy link
Owner

In researching this, I came across InputConnection.requestCursorUpdates. I believe you could call that with requestCursorUpdates(CURSOR_UPDATE_MONITOR, CURSOR_UPDATE_FILTER_INSERTION_MARKER), and implement updateCursorAnchorInfo such that it would reset the tap count. That seems like the proper solution, but I couldn't figure out how to do that since I'm pretty unfamiliar with android dev. Anybody have any ideas or tips?

I tried this solution last weekend actually, but couldn't get it working. Its the right way to handle it, rather than a tap timeout. It needs someone other than me to take a crack at it.

@KraXen72
Copy link
Contributor Author

huge if true

@dessalines dessalines reopened this Jan 21, 2024
@dessalines
Copy link
Owner

Looks like this is still an issue on some text fields.

@jm355
Copy link
Contributor

jm355 commented Jan 21, 2024

Which ones? also there's a good chance the magic number of 15 that I used isn't ideal, or could be calculated based on the device, like pixel density of the screen or something

@dessalines
Copy link
Owner

dessalines commented Jan 22, 2024

A lot of them: Signal and Markor so far.

@jm355
Copy link
Contributor

jm355 commented Jan 22, 2024

what are you typing to reproduce it?

@dessalines
Copy link
Owner

Its very inconsistent. Occasionally if I go to those text fields, spacebar multitap doesn't work at all. Then other times they work, but the move cursor doesn't that character replacing bug again.

@jm355
Copy link
Contributor

jm355 commented Jan 22, 2024

Hmm, to me it sounds like the magic number of 15 needs to be figured out in a more device-specific way then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants