Skip to content

refactor(android/engine): Refactor KMSystemKeyboardWebViewClient 🛋#7993

Merged
darcywong00 merged 4 commits intomasterfrom
refactor/android/KMSystemKeyboardWebViewClient
Feb 1, 2023
Merged

refactor(android/engine): Refactor KMSystemKeyboardWebViewClient 🛋#7993
darcywong00 merged 4 commits intomasterfrom
refactor/android/KMSystemKeyboardWebViewClient

Conversation

@darcywong00
Copy link
Copy Markdown
Contributor

@darcywong00 darcywong00 commented Jan 9, 2023 •
edited
Loading

Follow-on to #7983 as part of #7881

This PR refactors KMManager.KMSystemKeyboardWebViewClient to use KMKeyboardWebViewClient class

  • Use keyboardType when there's differences between KeyboardType.KEYBOARD_TYPE_INAPP and KEyboardType.KEYBOARD_TYPE_SYSTEM

User Testing

Setup - Install the PR builds of the Android apps to an Android device/emulator

  • TEST_KEYMAN - Verifies InApp and System keyboard in Keyman app
  1. Launch Keyman for Android and dismiss the "Get Started" menu
  2. From the Keyman settings menu, search for and install xinaliq keyboard. This keyboard uses rota keys based on the context
  3. After Keyman installs xinaliq, verify the inapp keyboard switches to xinaliq
  4. Verify the globe key has a help tip "Tap here to change keyboard"
  5. Short-press the globe key to switch keyboard
  6. Verify the in-app keyboard switches to sil_euro_latin
  7. Verify the globe key help tip no longer appears
  8. Short-press the globe to switch to xinaliq
  9. In the Keyman app, type "the quick brown foxed jumped over the lazy dog". Note, if typing from a hardware keyboard, the w key outputs ü.
  10. In the app, move the cursor to between "th" in "the".
  11. In the xinaliq keyboard, press t multiple times
  12. Verify the t in the text changes through several rotations (Refer to the keyboard help)
  13. In the Keyman app menu, select "Clear Text".
  14. Verify the app clears the entire text.
  15. From Keyman settings, enable Keyman as system-wide keyboard and set Keyman as default keyboard
  16. Launch a separate app (e.g. Google Chrome)
  17. Select a text area to start typing
  18. With xinaliq selected, type "the quick brown fox jumped over the lazy dog"
  19. In the text area, move the cursor between "th" in "the".
  20. In the xinaliq keyboard, press t multiple times
  21. Verify the t in the text area changes through several rotations
  22. From the system status bar, press the back/down arrow
  23. Verify the Keyman system keyboard is dismissed (hidden)
  • TEST_FV - Verifies FV Android system keyboard

  1. Launch FV for Android

  2. On the "Setup" menu, add the keyboard --> BC Coast --> Sencoten

  3. Enable the keyboard and wait for the Sencoten keyboard and dictionary to install

  4. On the "Setup" menu, enable First Voices as a system keyboard and set as default keyboard

  5. Launch a separate app (e.g. Google Chrome)

  6. Select a text area to start typing

  7. With the FV Sencoten keyboard selected, type

  8. Verify the selected keys are output

@darcywong00 darcywong00 added this to the A17S4 milestone Jan 9, 2023
@keymanapp-test-bot keymanapp-test-bot bot added the user-test-required User tests have not been completed label Jan 9, 2023
@keymanapp-test-bot
Copy link
Copy Markdown

keymanapp-test-bot bot commented Jan 9, 2023 •
edited
Loading

User Test Results

Test specification and instructions

  • ✅ TEST_KEYMAN (PASSED): Retested this as per darcy's suggestion in API 30 / Android 11 emulator and here is my observation: Yes, after I pressed the Back / down arrow button from the Mobile, the Keyman Keyboard was dismissed from the browser view. Seems to be working as expected. Thanks. (notes)
  • ✅ TEST_FV (PASSED): Installed First Voices Keyboard in Android 11 / API 30 emulator and it working as expected. (notes)

Test Artifacts

@darcywong00 darcywong00 linked an issue Jan 9, 2023 that may be closed by this pull request
refactor(android/engine): Merge SystemKeyboard and InappKeyboard in KMManager 🛋 #7881
Closed
@darcywong00 darcywong00 marked this pull request as ready for review January 9, 2023 06:29
@darcywong00 darcywong00 requested a review from sgschantz as a code owner January 9, 2023 06:29
@bharanidharanj
Copy link
Copy Markdown

bharanidharanj commented Jan 9, 2023

Test Results

  • TEST_KEYMAN (FAILED): Tested with the attached PR build (keyman 17.0.24-alpha-test-7993) in Android 11 / API 30 emulator and here is my observation: 1. Able to see the 'Tap here to change keyboard" help tip appears on the globe key, after installing xinaliq keyboard. 2. Verified the keyboard switches between sil_euro_latin and xinaliq happens correctly. 3. Able to see the letter 'w' appears as 'ü' after I typed the text as it mentioned in the User Testing Steps. 4. Verified the letter 't' changes through several rotations, If I press the 't' letter multiple times. 5. However, If I press the back / down arrow button from the system keyboard, the Keyman System keyboard still remains in the browser view.

..Tool tip

.. w letter changes

..Keyman Keyboard remains after pressing the Back/down arrow

@bharanidharanj
Copy link
Copy Markdown

bharanidharanj commented Jan 9, 2023

Test Results

  • TEST_FV (PASSED): Installed First Voices Keyboard in Android 11 / API 30 emulator and it working as expected.

@darcywong00
Copy link
Copy Markdown
Contributor Author

darcywong00 commented Jan 9, 2023

..Keyman Keyboard remains after pressing the Back/down arrow

@bharanidharanj - The green circle below shows the down arrow to press to dismiss the keyboard. Did you try this button?
down arrow

@keymanapp-test-bot retest TEST_KEYMAN

@darcywong00 darcywong00 mentioned this pull request Jan 10, 2023
Merged
@bharanidharanj
Copy link
Copy Markdown

bharanidharanj commented Jan 10, 2023

Test Results

  • TEST_KEYMAN (PASSED): Retested this as per darcy's suggestion in API 30 / Android 11 emulator and here is my observation: Yes, after I pressed the Back / down arrow button from the Mobile, the Keyman Keyboard was dismissed from the browser view. Seems to be working as expected. Thanks.

@keymanapp-test-bot keymanapp-test-bot bot removed the user-test-required User tests have not been completed label Jan 10, 2023
@darcywong00 darcywong00 requested a review from mcdurdin January 20, 2023 01:14
@darcywong00 darcywong00 modified the milestones: A17S4, A17S5 Jan 23, 2023
@darcywong00
chore(android): Merge branch 'refactor/KMKeyboardWebViewClient' into …
ea3d859 
…refactor/android/KMSystemKeyboardWebViewClient
mcdurdin
mcdurdin approved these changes Jan 24, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@mcdurdin mcdurdin left a comment

Choose a reason for hiding this comment

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

I like the targeted refactors, moving from working->working in each PR. This may be a little slower than an all-at-once approach, but incremental improvements are much easier to review and test. Good work.

Base automatically changed from refactor/KMKeyboardWebViewClient to master February 1, 2023 07:20
@darcywong00 darcywong00 merged commit 4604b0d into master Feb 1, 2023
@darcywong00 darcywong00 deleted the refactor/android/KMSystemKeyboardWebViewClient branch February 1, 2023 07:21
@keyman-server
Copy link
Copy Markdown
Collaborator

keyman-server commented Feb 1, 2023

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

darcywong00 added a commit that referenced this pull request Mar 2, 2023
@darcywong00
chore(android): Revert "Merge pull request #7993 from keymanapp/refac…
80abad4 
…tor/android/KMSystemKeyboardWebViewClient"

This reverts commit 4604b0d.
@darcywong00 darcywong00 mentioned this pull request Mar 2, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mcdurdin mcdurdin mcdurdin approved these changes

@sgschantz sgschantz Awaiting requested review from sgschantz

Assignees

No one assigned

Labels

android/engine/ android/ has-user-test refactor

Projects

None yet

Milestone

A17S5

Development

Successfully merging this pull request may close these issues.

refactor(android/engine): Merge SystemKeyboard and InappKeyboard in KMManager 🛋

4 participants

@darcywong00 @bharanidharanj @keyman-server @mcdurdin