feat(android): Add controls for auto-correct #12443

darcywong00 · 2024-09-19T06:55:28Z

Follows #12442 and addresses much of #12203 for adding "auto-correct" control on the language settings menu.

From the design meeting decisions, we replace the current "Prediction" and "Correction" toggles with a group of 4 radio-buttons. This has options:

TODO for follow-on PR: add KeymanWeb API to apply auto-correct control

User Testing

Setup - Install the PR build of Keyman for Android on a device/emulator

TEST_SUGGESTIONS_RADIO_BUTTONS

Launch Keyman for Android.
From the "Get Started" menu, set Keyman as the default system keyboard
In the Keyman app, verify the default sil_euro_latin keyboard displays suggestions
Launch Chrome and select a text area to type with Keyman as the default keyboard
Verify the default sil_euro_latin keyboard displays suggestions
Return to the Keyman app --> Keyman settings --> Installed languages --> English to bring up the "English Settings" menu
Verify the default suggestion radio selection (at the bottom) is "Predictions with corrections"
Select "Predictions only"
Verify "Predictions only" is the only selected option
Select "Disable suggestions"
Verify "Disable suggestions" is the only selected option
Exit the menu
In the Keyman app, verify the suggestion banner is now disabled (replaced with Keyman banner)
In Chrome select a text field to type in
Verify the Keyman suggestion banner is disabled (replaced with Keyman banner)

keymanapp-test-bot · 2024-09-19T06:55:36Z

User Test Results

Test specification and instructions

✅ TEST_SUGGESTIONS_RADIO_BUTTONS (PASSED) (notes)

dinakaranr · 2024-09-19T10:58:28Z

Test Results

I tested this issue with the attached "Keyman 18.0.115-alpha-test-12443" build on the Android 14 mobile. Here is my observation.

TEST_SUGGESTIONS_RADIO_BUTTONS (Passed):

Installed the "Keyman-18.0.115.apk" file from the PR build. and gave all permissions to the application.
Checked the "Enable Keyman as system-wide keyboard" on the settings page. Checked the "Set Keyman as default keyboard.".
Open the Keyman app. Enter some text in the keyman's notepad.
Verified the default "sil_euro_latin" keyboard displays suggestions.
Launch the Chrome browser. Navigate the "editpad.org" website and enter some text on the page.
Verified the default "sil_euro_latin" keyboard displays suggestions.
Return to the Keyman app --> Keyman settings --> Installed languages --> English to bring up the "English Settings" menu
Verified the "Predictions with corrections" is set as the default suggestion radio selection.
Select "Predictions only"
Verified "Predictions only" is the only selected option
Select "Disable suggestions"
Verified "Disable suggestions" is the only selected option.
Exit the menu and return to Keyman Notepad.
Verified the suggestion banner is now disabled and it is showing the Keyman banner.
Launch the Chrome browser. Navigate the "editpad.org" website and enter some text on the page.
Verified the suggestion banner is now disabled and it is showing the Keyman banner.
It works as per the given steps. Thanks.

I have seen another behavior on Keyman's Notepad and Chrome browser (editpad.org) when using the "Predictions with auto-corrections" option.

Launch the Chrome browser.
Navigate to the "editpad.org" website.
Enter the wrong words in the text area.
Actual Results: Here, the word does not turn into the correct word if entering the wrong word. Instead of showing the correct word on the banner with highlights (I am expecting the word to auto-correction),
Please refer to the screenshot below

mcdurdin

This functionally looks good. Can you:

try and avoid using magic numbers and use the enum values everywhere possible
remove the mayPredict option from the javascript interface as it is redundant

Thanks!

mcdurdin · 2024-09-27T00:41:06Z

android/KMAPro/kMAPro/src/main/java/com/tavultesoft/kmapro/LanguageSettingsActivity.java

+    // Initialize radio button group
+    switch(maySuggest) {
+      case 1:
+        radioButton = (RadioButton) radioGroup.findViewById(R.id.suggestion_radio_1);
+        break;
+      case 2:
+        radioButton = (RadioButton) radioGroup.findViewById(R.id.suggestion_radio_2);
+        break;
+      case 3:
+        radioButton = (RadioButton) radioGroup.findViewById(R.id.suggestion_radio_3);
+        break;
+      case 0:
+      default:
+        radioButton = (RadioButton) radioGroup.findViewById(R.id.suggestion_radio_0);
+    }
+    radioGroup.clearCheck();


This would be neater with an array but it's not critical.

mcdurdin · 2024-09-27T00:41:43Z

android/KMAPro/kMAPro/src/main/java/com/keyman/android/SystemKeyboard.java

-        KMManager.setBannerOptions(mayPredict);
+        int maySuggest = prefs.getInt(KMManager.getLanguageAutoCorrectionPreferenceKey(langId), KMManager.KMDefault_Suggestion);
+        // Enable banner if maySuggest is not SuggestionType.SUGGESTIONS_DISABLED (0)
+        KMManager.setBannerOptions(maySuggest > 0);


Can we avoid using a number 0 here and use SuggestionType.SUGGESTIONS_DISABLED?

android/KMEA/app/src/main/assets/android-host.js

mcdurdin · 2024-09-27T00:49:02Z

android/KMEA/app/src/main/java/com/keyman/engine/KMManager.java

+   * @param suggestType Radio button index 0 to 3
+   */
+  public static void setMaySuggest(String languageID, int suggestType) {
+    if (suggestType < 0 || suggestType > 3) {


Can we use SuggestionType.SUGGESTIONS_DISABLED.toInt() or something like that? Everywhere we have integer values in the java code?

android/KMEA/app/src/main/res/values/strings.xml

Co-authored-by: Marc Durdin <marc@durdin.net>

…t/android/auto-correct-radio-buttons # Keyman Conventional Commit suggestions: # # - Link to a Sentry issue with git trailer: # Fixes: _MODULE_-_ID_ # - Give credit to co-authors: # Co-authored-by: _Name_ <_email_> # - Use imperative, present tense ('attach' not 'attaches', 'attached' etc) # - Don't include a period at the end of the title # - Always include a blank line before trailers # - More: https://github.com/keymanapp/keyman/wiki/Pull-Request-and-Commit-workflow-notes

ready for re-review

mcdurdin · 2024-10-09T03:53:12Z

android/KMAPro/kMAPro/src/main/java/com/keyman/android/SystemKeyboard.java

-        KMManager.setBannerOptions(mayPredict);
+        int maySuggest = prefs.getInt(KMManager.getLanguageAutoCorrectionPreferenceKey(langId), KMManager.KMDefault_Suggestion);
+        // Enable banner if maySuggest is not SuggestionType.SUGGESTIONS_DISABLED (0)
+        KMManager.setBannerOptions(maySuggest > SuggestionType.SUGGESTIONS_DISABLED.toInt());


Suggested change

KMManager.setBannerOptions(maySuggest > SuggestionType.SUGGESTIONS_DISABLED.toInt());

KMManager.setBannerOptions(maySuggest <> SuggestionType.SUGGESTIONS_DISABLED.toInt());

Yeah same same but removes knowledge of actual values :)

yep. will use !=

mcdurdin · 2024-10-09T03:54:37Z

android/KMEA/app/src/main/java/com/keyman/engine/KMKeyboardWebViewClient.java

-        modelPredictionPref = prefs.getBoolean(KMManager.getLanguagePredictionPreferenceKey(KMManager.currentLexicalModel.get(KMManager.KMKey_LanguageID)), true);
+        modelPredictionPref = prefs.getInt(KMManager.getLanguageAutoCorrectionPreferenceKey(
+          KMManager.currentLexicalModel.get(KMManager.KMKey_LanguageID)), KMManager.KMDefault_Suggestion)
+          > SuggestionType.SUGGESTIONS_DISABLED.toInt();


Suggested change

> SuggestionType.SUGGESTIONS_DISABLED.toInt();

<> SuggestionType.SUGGESTIONS_DISABLED.toInt();

mcdurdin · 2024-10-09T03:55:10Z

android/KMEA/app/src/main/java/com/keyman/engine/KMManager.java

+      int modes[] = { 0, 1, 2, 3 };
+      return modes[this.ordinal()];


Why not just return this.ordinal();?

mcdurdin · 2024-10-09T03:55:38Z

android/KMEA/app/src/main/java/com/keyman/engine/KMManager.java

@@ -315,6 +346,8 @@ public enum EnterModeType {
  public static final int KMMinimum_LongpressDelay = 300;
  public static final int KMMaximum_LongpressDelay = 1500;

+  // Default prediction/correction setting - corresponds to SuggestionType.PREDICTIONS_WITH_CORRECTIONS
+  public static final int KMDefault_Suggestion = 2;


Still got a magic number here

keyman-server · 2024-10-09T18:01:26Z

Changes in this pull request will be available for download in Keyman version 18.0.124-alpha

darcywong00 added 2 commits September 19, 2024 13:16

feat(android): Add radio-button selections for auto-correct

4927922

chore(android): Disable debug log

9096105

darcywong00 requested a review from sgschantz as a code owner September 19, 2024 06:55

keymanapp-test-bot bot added the user-test-missing User tests have not yet been defined for the PR label Sep 19, 2024

keymanapp-test-bot bot added this to the A18S11 milestone Sep 19, 2024

github-actions bot added android/ android/engine/ android/app/ feat labels Sep 19, 2024

keymanapp-test-bot bot added has-user-test user-test-required User tests have not been completed and removed user-test-missing User tests have not yet been defined for the PR labels Sep 19, 2024

keymanapp-test-bot bot removed the user-test-required User tests have not been completed label Sep 19, 2024

Base automatically changed from fix/android/disable-banner-password to master September 25, 2024 02:40

mcdurdin previously requested changes Sep 27, 2024

View reviewed changes

darcywong00 and others added 3 commits September 27, 2024 10:46

Apply suggestions from code review

a6f9082

Co-authored-by: Marc Durdin <marc@durdin.net>

Merge branch 'master' into feat/android/auto-correct-radio-buttons

cc2bc23

fix(android): Use SuggestionType methods instead of magic numbers

6eba88c

darcywong00 modified the milestones: A18S11, A18S12 Sep 28, 2024

darcywong00 added 2 commits September 30, 2024 09:12

fix(android/engine): Remove extraneous braces

39f2d3e

darcywong00 requested a review from mcdurdin October 8, 2024 09:08

mcdurdin approved these changes Oct 9, 2024

View reviewed changes

fix(android): Apply review suggestions

15993ff

darcywong00 merged commit 9fa0670 into master Oct 9, 2024
3 checks passed

darcywong00 deleted the feat/android/auto-correct-radio-buttons branch October 9, 2024 10:25

darcywong00 mentioned this pull request Oct 10, 2024

feat(ios): add auto-correct toggle #12525

Open

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(android): Add controls for auto-correct #12443

feat(android): Add controls for auto-correct #12443

darcywong00 commented Sep 19, 2024 •

edited

Loading

keymanapp-test-bot bot commented Sep 19, 2024 •

edited

Loading

dinakaranr commented Sep 19, 2024

mcdurdin left a comment

mcdurdin Sep 27, 2024

mcdurdin Sep 27, 2024

mcdurdin Sep 27, 2024

mcdurdin Oct 9, 2024

darcywong00 Oct 9, 2024

mcdurdin Oct 9, 2024

mcdurdin Oct 9, 2024

mcdurdin Oct 9, 2024

keyman-server commented Oct 9, 2024

	KMManager.setBannerOptions(maySuggest > SuggestionType.SUGGESTIONS_DISABLED.toInt());
	KMManager.setBannerOptions(maySuggest <> SuggestionType.SUGGESTIONS_DISABLED.toInt());

	> SuggestionType.SUGGESTIONS_DISABLED.toInt();
	<> SuggestionType.SUGGESTIONS_DISABLED.toInt();

feat(android): Add controls for auto-correct #12443

feat(android): Add controls for auto-correct #12443

Conversation

darcywong00 commented Sep 19, 2024 • edited Loading

User Testing

keymanapp-test-bot bot commented Sep 19, 2024 • edited Loading

User Test Results

dinakaranr commented Sep 19, 2024

Test Results

mcdurdin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keyman-server commented Oct 9, 2024

darcywong00 commented Sep 19, 2024 •

edited

Loading

keymanapp-test-bot bot commented Sep 19, 2024 •

edited

Loading