-
-
Notifications
You must be signed in to change notification settings - Fork 111
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): Enhance how ENTER key is handled for FV and KMSample2 #12745
Conversation
User Test ResultsTest specification and instructions
Test Artifacts |
Additional info for reviewers: The bulk of functionality was done in the original PR in Keyman Engine for Android (main control unit is KMManager) KMManager would alter how the ENTER key was handled depending on what type of input field the cursor was at. The SystemKeyboard.java for the apps using Keyman Engine would call Hence this one makes the corresponding changes to FirstVoices for Android and the KMSample2 app (the sample system keyboard app we include with the Keyman Engine for Android libraries) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good -- just one suggestion to improve the code.
|
||
// Determine special handling for ENTER key | ||
KMManager.setEnterMode(attribute.imeOptions, inputType); | ||
|
||
// User switched to a new input field so we should extract the text from input field |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Though you are only adding a couple lines to the code here, it would be nice to break up the functionality of onStartInput
into several methods. It is about 50 lines long and is checking several conditions with discrete tasks. Rather than having blocks of code with nice comments, it would be better to extract these into small well-named methods. Even if the methods are not used anywhere else, it will be more readable and unit-testable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep - that's a good suggestion (maybe can do some in early 19.0).
A lot of the Android code could use refactoring (SystemKeyboard and the main culprit KMManager).
Changes in this pull request will be available for download in Keyman version 18.0.151-alpha |
Follows #12125 and fixes #12743
The ENTER key was enhanced in the Keyman system keyboard depending on the type of input field.
This ports the functionality to the FirstVoices for Android and KMSample2 apps.
Copying user tests from the original PR:
User Testing
Setup - On an Android device, install the PR build of FirstVoices for Android and the Android KMSample2 app. Preferably use a physical device instead of emulator because you'll also need additional Android apps installed:
For the corresponding groups, setup a keyboard and enable as the default corresponding keyboard.
On FirstVoices, can do Region --> BC Coast --> SENCOTEN keyboard.
GROUP_FV: Use FirstVoices for Android as system keyboard
GROUP_SAMPLE2: Use KMSample2 as system keyboard
TEST_KEEP - Verifies how ENTER key is handled in Keep app