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(web): add and enable auto-correction 📚 #11866

Open
wants to merge 5 commits into
base: change/web/track-prediction-base-corrections
Choose a base branch
from

Conversation

jahorton
Copy link
Contributor

@jahorton jahorton commented Jun 25, 2024

This establishes something I've been waiting a long time to enable: when one prediction is sufficiently more likely than all alternatives, it will be automatically selected for application when tapping the space bar. In short: auto-correct.

Current conditions for auto-correct:

  • The pre-existing prefix for the correction/prediction is not zero-length. (We don't want to auto-select 'the' from an empty string with SIL EuroLatin, after all.)
  • Any "correction" applied to generate the suggestion must be the highest-probability correction that leads to a valid suggestion.
    • It doesn't have to be the most likely keystroke - just the most likely keystroke that can reasonably produce a word from the model.
  • The suggestion to be auto-selected must have at least twice the probability of all other generated suggestions.
    • I started off with a harsher threshold, but auto-selection was pretty rare then. It could probably be more aggressive... but that does raise the risk of false positives.
    • This is not part-of-speech aware in any sense - if "book" and "books" are about as likely as each other, "boo" would not auto-select either "book" or "books"; neither would win, even if they were the only words in the model, as there's no clear probability winner.

These conditions may, of course, be further tweaked... but this seemed to be a reasonable initial set to use for testing.

One interaction that should probably be reviewed for correctness + user-friendliness: While the banner is receiving any mouse-based or touch-based interaction, the auto-selected suggestion will be deselected - even if just to scroll before releasing the banner. If it was a scrolling interaction, the original auto-selected suggestion will be reselected and rehighlighted. If a suggestion was applied, however, there will be no reselection or re-highlighting.

Animated gif of the interaction and its variations


Notes for review:

This PR's changes utilize existing plumbing that was prepared all the way back with #1747 during 12.0-alpha. We prevented it from actually auto-applying anything at the time, but most of the design work was done back then.

#3702 was added a while back in order to swallow a single whitespace tap after applying a suggestion via banner UI. We wish to maintain that here. However, we don't swallow the whitespace after application if it was triggered by pressing the spacebar... because we'd have already tapped the spacebar once.


User Testing

TEST_LOOK_AND_FEEL: Using Keyman for Android, play around with predictive text and report back on how the interactions feel.

  • When it presents an automatically-selected suggestion, do banner interactions feel intuitive?
    • Be sure to try scrolling the banner when testing this.
    • As might be noticed by the recording above, I find tempta to be a good "prompt" input for this. Do try others, though.
    • Do you see any banner "flicker" during these interactions? Things should feel as smooth as they usually do.
  • When it presents an automatically-selected suggestion, do keyboard interactions feel intuitive?
    • Just in case, be sure to hit the spacebar at least twice in a row when an automatically-selected suggestion is selected (before the first tap).
    • Also, try selecting a different suggestion from the banner, then tapping spacebar.
    • Also, try selecting the auto-selected suggestion from the banner, then tapping spacebar.
  • If the answer to either question above is "no", FAIL the test and report what felt wrong.
  • Do NOT fail this test if you expect something to be automatically selected, but it isn't.
  • ... but DO fail this test if a suggestion isn't auto-selected when it's the only one available.

TEST_POST_APPLY_WHITESPACE: Using Keyman for Android...

  • With an auto-selected suggestion available, press the spacebar twice in a row.
    • There should be a total of two spaces after the applied suggestion.
  • With an auto-selected suggestion available, press the suggestion, then tap the spacebar twice in a row.
    • There should be a total of two spaces after the applied suggestion.

TEST_POST_APPLY_BACKSPACE: Using Keyman for Android...

  • With an auto-selected suggestion available, press the spacebar to apply it.
  • Then, press backspace.
  • You should see a "suggestion" that restores the word as it was before you applied the suggestion.
    • The "suggestion" should not be auto-selected.

@keymanapp-test-bot keymanapp-test-bot bot added the user-test-missing User tests have not yet been defined for the PR label Jun 25, 2024
@keymanapp-test-bot
Copy link

keymanapp-test-bot bot commented Jun 25, 2024

User Test Results

Test specification and instructions

  • TEST_LOOK_AND_FEEL (PASSED): I tested this issue with the attached "Keyman-18.0.60-alpha-test-11866" build on the Android 14 Physical devices. Here, I have given my observation. (notes)
  • TEST_POST_APPLY_WHITESPACE (PASSED) (notes)
  • TEST_POST_APPLY_BACKSPACE (PASSED) (notes)

Test Artifacts

@keymanapp-test-bot keymanapp-test-bot bot added this to the A18S5 milestone Jun 25, 2024
@jahorton jahorton changed the base branch from master to change/web/track-prediction-base-corrections June 26, 2024 08:19
@jahorton jahorton changed the title feat(web): add and enable auto-correction feat(web): add and enable auto-correction 📚 Jun 26, 2024
@keymanapp-test-bot 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 Jun 27, 2024
@@ -559,6 +568,53 @@ export default class ModelCompositor {
return suggestions;
}

private predictionAutoSelect(suggestionDistribution: CorrectionPredictionTuple[]) {
Copy link
Contributor Author

@jahorton jahorton Jun 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potential followup:

Note that this method does not actually reference anything in the class. We could make it static... or even better, we could make it a standalone method.

Why this point is significant: it's isolated in a manner great for unit testing. No need to mock inputs or anything! I haven't actually defined any unit tests for it yet, though.

... I'd be more proactive about unit testing this if the effects weren't already easy to see during actual KMW use.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the unit test is probably a good idea for future maintenance.

@jahorton jahorton marked this pull request as ready for review June 27, 2024 05:38
@dinakaranr
Copy link

Test Results

  • TEST_LOOK_AND_FEEL (PASSED): I tested this issue with the attached "Keyman-18.0.60-alpha-test-11866" build on the Android 14 Physical devices. Here, I have given my observation.
  1. Installed the "Keyman-18.0.60.apk" file and gave all permissions to the application.
  2. Checked the "Enable Keyman as system-wide keyboard" and set the keyboard as the default keyboard box on the settings page.
  3. Open the Keyman app. Enable the "Predictions" and install the "Dictionary."
  4. Added a sentence to the notepad. 
    Results:
    Enter a word, and it will be observed that the word is highlighted on the banner.
    Verify that I can scroll the banner words. 
    Auto-suggestion and auto-correction feature work after pressing the "spacebar" key.
    There is no flicker on the banner while selecting the word.
  • TEST_POST_APPLY_WHITESPACE (PASSED): 
  1. Installed the "Keyman-18.0.60.apk" file and gave all permissions to the application.
  2. checked the "Enable Keyman as system-wide keyboard" and set the keyboard as the default keyboard box on the settings page.
  3. Open the Keyman app. Enable the "Predictions" and install the "Dictionary."
  4. Added a sentence to the notepad.
  5. Enter a word and the observed that the word is highlighted on the banner. 
  6. Press the "space bar" two times.
    Results: Here, The word is added. Two spaces appeared after the word.
  • TEST_POST_APPLY_BACKSPACE (PASSED): 
  1. Installed the "Keyman-18.0.60.apk" file and gave all permissions to the application.
  2. Checked the "Enable Keyman as system-wide keyboard" and set the keyboard as the default keyboard box on the settings page.
  3. Open the Keyman app. Enable the "Predictions" and install the "Dictionary."
  4. Added a sentence to the notepad.
  5. Enter a word and observe that the word is highlighted on the banner. 
  6. Press the "Spacebar" for two times, and then press the "Backspace" key.
    Results: The suggestion word appeared in the double quotes, and it is not auto-selected. 
    This feature feels good and works well. Thank you.

@keymanapp-test-bot keymanapp-test-bot bot removed the user-test-required User tests have not been completed label Jun 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

4 participants