search

keymanapp / keyman

Keyman cross platform input methods system running on Android, iOS, Linux, macOS, Windows and mobile and desktop web
https://keyman.com/
Other
403 stars 112 forks source link

feat(android/app): add auto-correct toggle #12203

Open jahorton opened 3 months ago

jahorton commented 3 months ago

Is your feature request related to a problem? Please describe.

Now that we support auto-correct, it'd be wise to add a toggle allowing users to disable it if desired.

It may be best to make this a language-specific setting, right alongside the other enable/disable toggles.

Describe the solution you'd like

No response

Describe alternatives you've considered

No response

Related issues

No response

Keyman apps

Keyman version

No response

Operating system

No response

Device

No response

Target application

No response

Browser

No response

Keyboard name

No response

Keyboard version

No response

Language name

No response

Additional context

No response

darcywong00 commented 3 months ago

It may be best to make this a language-specific setting, right alongside the other enable/disable toggles.

So we've always had this language-specific toggle, and I never saw a noticeable change in predictions. Can you confirm what the KeymanWeb API is for enabling/disabling?

corrections toggle

jahorton commented 3 months ago

It may be best to make this a language-specific setting, right alongside the other enable/disable toggles.

So we've always had this language-specific toggle, and I never saw a noticeable change in predictions. Can you confirm what the KeymanWeb API is for enabling/disabling?

corrections toggle

With enable corrections off, the engine shouldn't change any letters currently within the context. Granted, I haven't directly tested for that in a while... and upon deeper inspection, it doesn't seem to be affecting anything. So, yeah, that should be made into an issue and addressed.

https://github.com/keymanapp/keyman/blob/837667126b52839390ea8314e08ceb74cbc64ea2/web/src/engine/osk/src/visualKeyboard.ts#L932-L937

Note lines 935-937 there - that would have originally "turned off" fat-finger data when .mayCorrect is set to false. Looks like I never patched that up in 17.0 work.

darcywong00 commented 3 months ago

I think this can be closed (the may correct toggle is already in the Keyman for Android language settings).

With the default sil_euro_latin keyboard on English, the toggle defaults to "on". And I verified keyman.core.languageProcessor.mayCorrect was true

After disabling the toggle, keyman.core.languageProcessor.mayCorrect is false

jahorton commented 3 months ago

This is not the same thing. It believe it reasonable to want auto-correct disabled while still allowing for corrections to be presented as options.

darcywong00 commented 3 months ago

Can you confirm what the KeymanWeb API is for enabling/disabling?

So what's the corresponding KeymanWeb call to this auto-correct toggle?

jahorton commented 3 months ago

It doesn't exist yet, but it's not too tricky to handle.

On the suggestions returned by the worker, the suggestion to auto-select for auto-correction is annotated with property .autoAccept == true.

https://github.com/keymanapp/keyman/blob/89d65adad04127873c0513bc2135abafd77f0567/web/src/engine/interfaces/src/prediction/predictionContext.ts#L321-L323

The first suggestion with that set to true is then saved by this.selected (to hold the currently-selected suggestion) and applied when appropriate. The easiest way to block auto-correct is simply to block this condition from passing - that is, to ignore the autoAccept flag. Say, if(this.allowsAutoCorrect && !s.autoAccept && !this.selected).

From there, the question is what API to provide from Web to allow host apps to toggle that allowsAutoCorrect flag.

darcywong00 commented 3 months ago

Since Keyman Engine for Android is currently toggling: keyman.core.languageProcessor.mayPredict keyman.core.languageProcessor.mayCorrect

Could we just add one more for keyman.core.languageProcessor.mayAutoCorrect ?

mcdurdin commented 3 months ago

A slider so it's only one setting? no predictive text | offer predictions only | offer corrections too | auto-correct

darcywong00 commented 3 months ago

A slider so it's only one setting? no predictive text | offer predictions only | offer corrections too | auto-correct

Does this mean we change the public API from two separate toggles into a single enum? https://help.keyman.com/developer/engine/android/17.0/KMManager/getLanguagePredictionPreferenceKey https://help.keyman.com/developer/engine/android/17.0/KMManager/getLanguageCorrectionPreferenceKey

Or does the slider correspond to 3 separate toggles internally? (prediction, correction, auto-correct)

mcdurdin commented 3 months ago

Some design discussion needed?

darcywong00 commented 2 months ago

Decisions from A18S10 Design Meeting

DW to do work on Android (both FV + KM) SGS to do work on iOS (both FV + KM)

darcywong00 commented 2 months ago

Consolidate the 4 options (No predictive text, Offer predictions only, Offer corrections too, Auto-correct) into RADIO BUTTONS

Preliminary implementation of the radio buttons

radio buttons

darcywong00 commented 1 month ago

Remaining TODOs (may get split to separate PRs)

The Android radio button layout didn't allow for additional description info

darcywong00 commented 1 month ago

From A18S14 sprint planning meeting, we'll defer this till A18S19 when @jahorton returns and can finalize the API involved