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

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

Open
1 of 8 tasks
jahorton opened this issue Aug 16, 2024 · 14 comments
Open
1 of 8 tasks

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

jahorton opened this issue Aug 16, 2024 · 14 comments
Assignees
Milestone

Comments

@jahorton
Copy link
Contributor

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 for Android
  • Keyman for iPhone and iPad
  • Keyman for Linux
  • Keyman for macOS
  • Keyman for Windows
  • Keyman Developer
  • KeymanWeb
  • Other - give details at bottom of form

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
Copy link
Contributor

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
Copy link
Contributor Author

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.

// TODO: It'd be nice to optimize by keeping these off when unused, but the wiring
// necessary would get in the way of modularization at the moment.
// let keyman = com.keyman.singleton;
// if (!keyman.core.languageProcessor.mayCorrect) {
// return null;
// }

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
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor

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

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

@jahorton
Copy link
Contributor Author

jahorton commented Aug 26, 2024

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.

if(s.autoAccept && !this.selected) {
this.selected = s;
}

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
Copy link
Contributor

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
Copy link
Member

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

@darcywong00
Copy link
Contributor

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
Copy link
Member

Some design discussion needed?

@darcywong00
Copy link
Contributor

darcywong00 commented Sep 9, 2024

Decisions from A18S10 Design Meeting

  • Consolidate the 4 options (No predictive text, Offer predictions only, Offer corrections too, Auto-correct) into RADIO BUTTONS
  • Add a toggle for adding Contacts to suggestions
  • MD to add descriptions for the 4 options (English/Strings only). They'll get synced to Crowdin

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

@darcywong00 darcywong00 modified the milestones: A18S10, A18S11 Sep 14, 2024
@darcywong00
Copy link
Contributor

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 darcywong00 added this to the A18S12 milestone Sep 28, 2024
@darcywong00
Copy link
Contributor

darcywong00 commented Oct 10, 2024

Remaining TODOs (may get split to separate PRs)

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

@darcywong00 darcywong00 modified the milestones: A18S12, A18S13 Oct 11, 2024
@darcywong00 darcywong00 modified the milestones: A18S13, A18S14 Oct 26, 2024
@darcywong00
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

No branches or pull requests

3 participants