Option to disable start lag for gestures during fast typing

[devycarol]

My first open source contribution!

There's a functionality that prevents gesture input from starting during "fast typing." This is to prevent accidental gesture input if the user is, say, multi-hand typing on a tablet. But this is often frustrating and unnecessary if you intermix taps with gestures, especially in one-handed typing. I've added a setting to disable this behavior, which is off by default.

I've only written setting strings for English. The only thing I am unsure of is whether the settings reads are done in a "proper" way—there was no Settings import beforehand.

Resolves #882.

[Helium314]

I tested this and actually would (for myself) prefer something in between on and off. So my suggestion would be to change this setting into a slider e.g. from 0 to 150 or 200%, and use the value as a multiplier for mDynamicThresholdDecayDuration when creating GestureStrokeRecognitionParams. Setting 0% should then effectively be the same as mGestureAlwaysStartEnabled.

You can check how to implement the slider in PreferencesSettingsFragment.setupKeypressSoundVolumeSettings or in a bunch of other places. Probably you only need to copy the code and slightly adjust it. (and the setting name should be adjusted too in this case) Is this ok for you, or do you want it as on/off switch only for some reason?

[Helium314]

I've only written setting strings for English. The only thing I am unsure of is whether the settings reads are done in a "proper" way—there was no Settings import beforehand.

You only need to add English strings, translations are done on Weblate after the PR is merged.

The setting should be read correctly. By default it's true because this is the default value in prefs.getBoolean(Settings.PREF_GESTURE_ALWAYS_START, true). I see you have the default as false in the xml file, so when not touching the setting, it will be on, but shown as off. This should be aligned to avoid confusion.

[devycarol]

Oh whoops! Thanks for pointing that out 😅

A slider seems very reasonable, and might even be more conceptually legible than a simple toggle. I'll make those changes.

[devycarol]

Oh, what about this line? https://github.com/Helium314/HeliBoard/blob/4aac81391e9e505724a315585d4de13b55767c0a/app/src/main/res/values/config-common.xml#L63

It seems to be hard-coded in 2 different places. Are you meant to read settings values into that xml file? Do we just remove that and handle the value in settings?

This is the context where that attribute is retrieved: https://github.com/Helium314/HeliBoard/blob/4aac81391e9e505724a315585d4de13b55767c0a/app/src/main/java/helium314/keyboard/keyboard/internal/GestureStrokeRecognitionParams.java#L63-L65

I might be misinterpreting things.

[Helium314]

My idea was simply putting the multiplier where the value is retrieved, like

mDynamicThresholdDecayDuration = mainKeyboardViewAttr.getInt( 
         R.styleable.MainKeyboardView_gestureDynamicThresholdDecayDuration, 
-        DEFAULT.mDynamicThresholdDecayDuration);
+        DEFAULT.mDynamicThresholdDecayDuration) * Settings....;

(and cast to int)

[devycarol]

Okay, I'll have a go at that. The constructor above also needs to be changed, yes?

[Helium314]

As far as I see, the other constructor is only used for DEFAULT, which is only used to provide default values. So I don't think it needs to be adjusted.

[devycarol]

I've run into some issues. First, "what should the setting do?" is a bit more complex when we make it a slider. Should it reduce the break-out threshold? Or should it reduce the window where we consider the user to be fast-typing? We could simply reduce the ms where that "safety mode" is active, and that way we could have it configured as an exact number of milliseconds and it would be more intuitive that adjusting the "decay threshold" I think.

In that case it would just read from settings, no multiplier involved.

Or if you prefer the other approach, the setting would be something like "Accidental gesture prevention: normal, slight, off."

The second issue is that those parameters are assigned as final, so the setting doesn't update the behavior until a full app restart (i.e. force stop) has occurred. Maybe I could just re-run PointerTracker.init() when the setting is updated? Not sure how feasible that is.

[devycarol]

One solution would be to sidestep the GestureStrokeRecognitionParams class entirely and always read those "cooldown delay" milliseconds straight from settings. Might be worse for performance though, I'm not sure.

This would be required for configurable cooldown millis, since "static time threshold after fast typing" is used for bogus-detection purposes other than the main fast-typing cooldown.

[Helium314]

"what should the setting do?"

My idea was reducing the mDynamicThresholdDecayDuration. It could also reduce the mGestureDynamicDistanceThresholdFrom if you think it's a good / necessary thing.

We could simply reduce the ms where that "safety mode" is active, and that way we could have it configured as an exact number of milliseconds and it would be more intuitive that adjusting the "decay threshold" I think.

We could, but then we would skip the way of doing it dynamically, which I assume was implemented for a reason.

Or if you prefer the other approach, the setting would be something like "Accidental gesture prevention: normal, slight, off."

I would prefer a slider, because it means better customizability, and fewer strings to translate.

the setting doesn't update the behavior until a full app restart (i.e. force stop) has occurred

Isn't it enough if you call KeyboardSwitcher.forceUpdateKeyboardTheme like it's done in similar need-to-reload situations? This triggers creation of a new MainKeyboardView, which I think should do the necessary reload.

[devycarol]

I've decided that decreasing the "safety millis" is the best approach. That's a static value, and I've implemented it without affecting other bogus-detection methods or needing to do a keyboard reset.

[Helium314]

Thanks, this should be fine

[devycarol]

I've set the params-class "testing" default to 500 and config_gesture_static_time_threshold_after_fast_typing (500) as the setting default.

[Helium314]

Thanks!

Just one thing: could you please avoid force-pushing in the future? This makes it difficult to see the actual changes. when trying to compare to the previous version. For the final (sqash) commit it doesn't matter anyway.

[devycarol]

Sure thing! I realized after the fact that it was probably not super helpful 😅