Skip to content

refactor(android/engine): Split KMInAppKeyboardWebViewClient to new class 🛋#7983

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

refactor(android/engine): Split KMInAppKeyboardWebViewClient to new class 🛋#7983
darcywong00 merged 4 commits intomasterfrom
refactor/KMKeyboardWebViewClient

Conversation

@darcywong00
Copy link
Copy Markdown
Contributor

@darcywong00 darcywong00 commented Jan 6, 2023 •
edited
Loading

Starts to address #7881

This extracts KMManager.KMInAppKeyboardWebViewClient to a separate KMKeyboardWebViewClient class.
A follow-on PR will refactor KMManager.KMSystemKeyboardWebViewClient to use this class too.

The first commit was moving the class to a separate file. Diffs can be seen in the remaining commits.

Some changes in KMManger:

  • KMManager.InAppKeyboardLoaded refactored to use InAppKeyboardWebViewClient.getKeyboardLoaded() and InAppKeyboardWebViewClient.setKeyboardLoaded()

User Testing

Setup - Load the PR build of Keyman for Android

  • TEST_KEYMAN - Verifies Keyman app functionality
  1. Launch Keyman
  2. Verify default sil_euro_latin appears
  3. From the app, change to the shift layer, longpress on a key and select one from the popup
  4. Verify the selected character is output
  5. From Keyman settings, search and install khmer_angkor keyboard
  6. Verify the in-app keybaord switches to khmer_angkor
  7. Longpress on a key and select one from the popup
  8. Verify the selected character is output

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

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

User Test Results

Test specification and instructions

  • ✅ TEST_KEYMAN (PASSED): Tested with the attached PR build (Keyman 17.0.24-alpha-test-7983) in API 30 /Android 11 emulator and here is my observation: 1. Long pressing a key shows popup. And selecting any subkey from the popup displays in the text area. Seems to be working as expected. (notes)

Test Artifacts

@darcywong00 darcywong00 marked this pull request as ready for review January 9, 2023 00:46
@darcywong00 darcywong00 requested a review from sgschantz as a code owner January 9, 2023 00:46
@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 mentioned this pull request Jan 9, 2023
Merged
@bharanidharanj
Copy link
Copy Markdown

bharanidharanj commented Jan 9, 2023

Test Results

  • TEST_KEYMAN (PASSED): Tested with the attached PR build (Keyman 17.0.24-alpha-test-7983) in API 30 /Android 11 emulator and here is my observation: 1. Long pressing a key shows popup. And selecting any subkey from the popup displays in the text area. Seems to be working as expected.

@keymanapp-test-bot keymanapp-test-bot bot removed the user-test-required User tests have not been completed label Jan 9, 2023
darcywong00
darcywong00 commented Jan 10, 2023
View reviewed changes
android/KMEA/app/src/main/java/com/keyman/engine/KMKeyboardWebViewClient.java
KMLog.LogError(TAG, "pageLoaded and InAppKeyboard null");
return;
}
KMManager.getKMKeyboard(keyboardType).keyboardSet = false;
Copy link
Copy Markdown
Contributor Author

@darcywong00 darcywong00 Jan 10, 2023

Choose a reason for hiding this comment

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

I refactored a lot of these KMManager.getKMKeyboard() calls in the follow-on #7993

@darcywong00 darcywong00 requested a review from mcdurdin January 20, 2023 01:15
@darcywong00 darcywong00 modified the milestones: A17S4, A17S5 Jan 23, 2023
Base automatically changed from refactor/android/engine-package to master February 1, 2023 07:19
@darcywong00 darcywong00 merged commit 6b5bd21 into master Feb 1, 2023
@darcywong00 darcywong00 deleted the refactor/KMKeyboardWebViewClient branch February 1, 2023 07:20
@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

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