change(android,web) Use web-based popup key longpresses #9591

darcywong00 · 2023-09-19T04:20:35Z

Addresses part of #9573 and fixes #9499

This updates Keyman Engine for Android to match iOS using KeymanWeb for handling longpresses.
The longpress keys will be limited by the top of the banner/OSK.

KeymanWeb changes

Remove Android-specific portions of buildEmbeddedGestureConfig and pendingLongpress

Keyman Engine for Android changes

Remove native Java code for popup, preview, and longpresses keys, as it's handled in KeymanWeb.

Note: this means banner will always be visible on Android, no option to switch off banner any more.

This portion of #9573 will be addressed on a separate PR. Android will possibly need some image banners to display when predictions are disabled.

User Testing

Setup - Install the PR build of Keyman for Android on an Android device emulator. In the Keyman app, also install the IPA (SIL) keyboard

TEST_IPA_RENDERING - Verifies bug(android): Special character '<' rendering problem with IPA (SIL) keyboard #9499 is fixed

Install the PR build of Keyman for Android on an Android device/emulator
Open Keyman In-App and dismiss the "Get Started" menu
Switch to IPA (SIL) keyboard in the Keyman App
Click the Number (123) key
Long press [ key
From the longpress options, select < special character
Verify the < symbol appears on the screen.

TEST_ANDROID_INAPP - Verifies longpresses and globe key work in the Keyman app

Install the PR build of Keyman for Android on an Android device/emulator
Open Keyman In-App and select sil_eurolatin keyboard
On the default sil_eurolatin keyboard, briefly press a top row key and release
Verify the key preview briefly appears, and the selected key is output
Longpress on a top row key and hold
Verify the longpress keys appear (position limited to the top of the banner)
Select a longpress key and release
Verify the selected key is output
Short-press the globe key
Verify the Keyman keyboard switches to IPA (SIL) keyboard
Long-press the globe key and release
Verify the keyboard picker menu appears.
Select a keyboard
Verify Keyman switches to the selected keyboard

TEST_ANDROID_SYSTEM_LONGPRESSES - Verifies longpresses and globe key work as a system keyboard

Install the PR build of Keyman for Android on an Android device/emulator
Open Keyman In-App and select the "Get Started" menu
Enable Keyman as a system keyboard and set as default keyboard
Launch a separate app (e.g. Chrome browser) and select a text area
With default Keyman sil_eurolatin keyboard, briefly press a top row key and release
Verify the key preview briefly appears, and the selected key is output
Longpress on a top row key and hold
Verify the longpress keys appear (position limited to the top of the banner)
Select a longpress key and release
Verify the selected key is output
Short-press the globe key
Verify the Keyman keyboard switches to IPA (SIL) keyboard
Long-press the globe key and release
Verify the keyboard picker menu appears.
Select a keyboard
Verify Keyman switches to the selected keyboard

TEST_IOS_INAPP - Verifies longpresses key works in the Keyman for iPhone and iPad app

Install the PR build of Keyman for iPhone and iPad on an iOS device/emulator
Open Keyman In-App and select sil_eurolatin keyboard
On the default sil_eurolatin keyboard, briefly press a top row key and release
Verify the key preview briefly appears, and the selected key is output
Longpress on a top row key and hold
Verify the longpress keys appear (position limited to the top of the banner)
Select a longpress key and release
Verify the selected key is output

Match from styles/colors.xml https://github.com/keymanapp/keyman/blob/31fd6ea42afef1dddd7d883fc31c5086fe9ae9b2/android/KMEA/app/src/main/res/values/colors.xml#L11

keymanapp-test-bot · 2023-09-19T04:20:39Z

User Test Results

Test specification and instructions

✅ TEST_IPA_RENDERING (PASSED): Tested in Android 12.0 / API31 emulator and here is my observation: 1. Installed the attached PR build 17.0.176-alpha-test-9591 on an Android emulator. 2. Switched to IPA (SIL) keyboard in the Keyman In-App. 3. Verified that the < symbol appears on the text input screen after selecting it from the long-press options. Seems to be working as expected. (notes)
✅ TEST_ANDROID_INAPP (PASSED): Tested in Android 12.0 / API31 emulator and here is my observation: 1. Installed the attached PR build 17.0.176-alpha-test-9591 on an Android emulator. 2. Verified that the long press key on the top row is working as expected for sil_eurolatin keyboard. 3. Verified that the keyboard has been changed to IPA (SIL) after short-press and releasing the globe key. 4. Also, verified that the keyboard picker menu appears, after long press and release of the globe key. Able to switch to some other keyboard from the Keyboard picker menu. Seems to be working as expected. (notes)
✅ TEST_ANDROID_SYSTEM_LONGPRESSES (PASSED): Tested in Android 12.0 / API31 emulator and here is my observation: 1. Installed the attached PR build 17.0.176-alpha-test-9591 on an Android emulator. 2. Enabled Keyman as the default keyboard using Get started dialog. Opened Chrome browser. Verified that the long press key on the top row is working as expected for the sil_eurolatin keyboard. 3. Verified that the keyboard has been changed to IPA (SIL) after short-press and releasing the globe key. 4. Also, verified that the keyboard picker menu appears, after long press and release of the globe key. Able to switch to some other keyboard from the Keyboard picker menu. Seems to be working as expected. (notes)
✅ TEST_IOS_INAPP (PASSED): Tested in iPhone 13 Pro Max / iOS 15.0 simulator and here is my observation: 1. Installed the attached PR build 17.0.176-alpha-test-9591 on iPhone Simulator. 2. Switched to sil_eurolatin keyboard in the Keyman In-App. 3. Verified that the key preview briefly appears after briefly pressing a top row key and release. 4. Verified that the selected key appeared on the screen after selecting it from the longpress options. Seems to be working as expected. (notes)

Test Artifacts

Android
Developer
- Keyman Developer - build failure
- Compiler Regression Tests - build failure
- kmcomp.zip - build failure
iOS
- Keyman for iOS (simulator image)
- FirstVoices Keyboards for iOS (simulator image)
- TestFlight internal PR build version - 17.0.176 (0.9591.9207)
Keyboards
- Test Keyboards - build failure
Web
- KeymanWeb Test Home
Windows
- Keyman for Windows - build failure
- FirstVoices Keyboards for Windows - build failure
- Text Editor (32 bit) - build failure
- Text Editor (64 bit) - build failure

darcywong00 · 2023-09-19T06:20:50Z

web/src/resources/osk/kmwosk.css

@@ -490,7 +490,7 @@ div.android div.kmw-keytip-cap {
 .tablet.ios #kmw-popup-keys .kmw-key{border:none;}
 .phone.ios  #kmw-popup-keys .kmw-key{border:none;}

-.phone.android #kmw-popup-keys {border:none; border-radius: 2px; background-color:#ccc; padding:5px 5px 0 0;}
+.phone.android #kmw-popup-keys {border:none; border-radius: 2px; background-color:#333333; padding:5px 5px 0 0;}


Longpress background styling to match the current background from

keyman/android/KMEA/app/src/main/res/values/colors.xml

Line 11 in 31fd6ea

<color name="key_preview_bg2">#ff333333</color>

jahorton · 2023-09-19T07:24:12Z

web/src/app/webview/src/osk/pendingLongpress.ts

-
-  public resolve() {
-    if(this.resolver) {
-      this.resolver(new SubkeyDelegator(this.vkbd, this.baseKey));


SubkeyDelegator may be removed as well; it's used exclusively as a follow-up to this class.

ah, I still had to keep SubkeyDelegator for

keyman/web/src/app/webview/src/keymanEngine.ts

Line 109 in 9fbe5f6

let gesture = osk.vkbd.subkeyGesture as SubkeyDelegator;

Right, which is within the executePopupKey method. Which is no longer being called, correct?

... which means we could probably eliminate the whole executePopupKey method, while we're at it.

I thought both iOS and Android will still need KeymanWeb to handle executePopupKey.

gotcha. I was able to remove popVisible (it had the same gesture)

I thought both iOS and Android will still need KeymanWeb to handle executePopupKey.

Ah, I get that. Yeah, we did leave the Swift connections to it in place, since back when we moved iOS to rely on the WebView, we were hoping Apple would fix the motivating bug so that we could revert to the same strategy Android's been using. Didn't happen, though, and we never revisited to clean out the old code. Probably worth making into a cleanup issue.

web/src/app/webview/src/keymanEngine.ts

Per review comments

jahorton · 2023-09-19T08:06:33Z

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

-function executePopupKey(keyID, keyText) {
-  // KMW only needs keyID to process the popup key. keyText merely logged to console
-  //window.console.log('executePopupKey('+keyID+'); keyText: ' + keyText);
-  keyman.executePopupKey(keyID);
-}
-


Re: a recent comment in one of the other threads.

darcywong00 · 2023-09-19T08:17:52Z

@bharanidharanj - please hold off on user testing until reviewers approve changes. Thanks

jahorton

I'm satisfied with the fixes to my requests for changes at this point.

mcdurdin

Changes LGTM. Minor suggestions.

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

mcdurdin · 2023-09-19T23:28:36Z

web/src/app/webview/src/oskConfiguration.ts

@@ -46,34 +45,5 @@ export function buildEmbeddedGestureConfig(device: DeviceSpec) {
    }
  }

-  if(device.OS == DeviceSpec.OperatingSystem.Android) {
-    embeddedGestureConfig.createKeyTip = (vkbd) => {


createKeyTip and startLongpress should be removed from EmbeddedGestureConfig.

I also think that the globe hint should be reimplemented in KeymanWeb, so we don't need any of this glue, and so it can be themed later. (This can be a separate pull)

createKeyTip is used in oskView and visualKeyboard
startLongpress is used in visualKeyboard

@jahorton good with removing these?

globe hint split into separate issue #9596

createKeyTip and startLongpress should be removed from EmbeddedGestureConfig.

EmbeddedGestureConfig cleanup in 98660fe

Co-authored-by: Marc Durdin <[email protected]>

Remove createKeyTip and startLongpress

bharanidharanj · 2023-09-21T07:50:50Z

TEST_ANDROID_INAPP (PASSED): Tested in Android 12.0 / API31 emulator and here is my observation: 1. Installed the attached PR build 17.0.176-alpha-test-9591 on an Android emulator. 2. Verified that the long press key on the top row is working as expected for sil_eurolatin keyboard. 3. Verified that the keyboard has been changed to IPA (SIL) after short-press and releasing the globe key. 4. Also, verified that the keyboard picker menu appears, after long press and release of the globe key. Able to switch to some other keyboard from the Keyboard picker menu. Seems to be working as expected.

bharanidharanj · 2023-09-21T08:10:13Z

TEST_ANDROID_SYSTEM_LONGPRESSES (PASSED): Tested in Android 12.0 / API31 emulator and here is my observation: 1. Installed the attached PR build 17.0.176-alpha-test-9591 on an Android emulator. 2. Enabled Keyman as the default keyboard using Get started dialog. Opened Chrome browser. Verified that the long press key on the top row is working as expected for the sil_eurolatin keyboard. 3. Verified that the keyboard has been changed to IPA (SIL) after short-press and releasing the globe key. 4. Also, verified that the keyboard picker menu appears, after long press and release of the globe key. Able to switch to some other keyboard from the Keyboard picker menu. Seems to be working as expected.

bharanidharanj · 2023-09-21T08:17:50Z

TEST_IPA_RENDERING (PASSED): Tested in Android 12.0 / API31 emulator and here is my observation: 1. Installed the attached PR build 17.0.176-alpha-test-9591 on an Android emulator. 2. Switched to IPA (SIL) keyboard in the Keyman In-App. 3. Verified that the < symbol appears on the text input screen after selecting it from the long-press options. Seems to be working as expected.

bharanidharanj · 2023-09-21T09:57:25Z

Test Results

TEST_IOS_INAPP (PASSED): Tested in iPhone 13 Pro Max / iOS 15.0 simulator and here is my observation: 1. Installed the attached PR build 17.0.176-alpha-test-9591 on iPhone Simulator. 2. Switched to sil_eurolatin keyboard in the Keyman In-App. 3. Verified that the key preview briefly appears after briefly pressing a top row key and release. 4. Verified that the selected key appeared on the screen after selecting it from the longpress options. Seems to be working as expected.

keyman-server · 2023-09-22T18:04:37Z

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

darcywong00 added 4 commits September 19, 2023 06:49

change(web): Remove Android pendingLongpress

f969c13

change(android/engine): Use web-based longpress

584053a

chore(web): Update background color for Android popup keys

f58cd50

Match from styles/colors.xml https://github.com/keymanapp/keyman/blob/31fd6ea42afef1dddd7d883fc31c5086fe9ae9b2/android/KMEA/app/src/main/res/values/colors.xml#L11

chore(android): Remove popup code

250b8a6

keymanapp-test-bot bot added has-user-test user-test-required User tests have not been completed labels Sep 19, 2023

darcywong00 added this to the A17S22 milestone Sep 19, 2023

github-actions bot added web/ android/ android/engine/ labels Sep 19, 2023

chore(android/engine): Remove key preview layout

d1c12c4

darcywong00 commented Sep 19, 2023

View reviewed changes

darcywong00 marked this pull request as ready for review September 19, 2023 06:27

darcywong00 requested review from jahorton, mcdurdin and sgschantz as code owners September 19, 2023 06:27

jahorton requested changes Sep 19, 2023

View reviewed changes

fix(web): Revert buildEmbeddedGestureConfig

e76936f

Per review comments

jahorton reviewed Sep 19, 2023

View reviewed changes

chore(web): More cleanup

8bf0ac0

jahorton approved these changes Sep 19, 2023

View reviewed changes

mcdurdin approved these changes Sep 19, 2023

View reviewed changes

darcywong00 mentioned this pull request Sep 20, 2023

refactor(android,ios,web): Move globe hint implementation from mobile platforms into Web #9596

Open

darcywong00 and others added 2 commits September 20, 2023 13:51

Update android/KMEA/app/src/main/java/com/keyman/engine/KMKeyboard.java

5a0c1a3

Co-authored-by: Marc Durdin <[email protected]>

chore(web): Cleanup embeddedGestureConfig

98660fe

Remove createKeyTip and startLongpress

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

darcywong00 merged commit 9e71322 into master Sep 22, 2023
2 checks passed

darcywong00 deleted the change/android/move-longpress-kmw branch September 22, 2023 05:32

This was referenced Sep 26, 2023

chore(web): feature-gestures merge with master (mid A17S22) 🐵 #9625

Merged

fix(web): early cherrypick of bugfixes in gesture-staging dependencies, minor cleanup 🐵 #9599

Merged

refactor(web): matched gesture-component abstraction 🐵 #9525

Merged

darcywong00 mentioned this pull request Oct 6, 2023

fix(android): Always display HTML banner when suggestions aren't available #9696

Merged

3 tasks

darcywong00 linked an issue Nov 17, 2023 that may be closed by this pull request

change(android): Web-based popup key longpressses #9573

Closed

darcywong00 mentioned this pull request Nov 17, 2023

change(android): Web-based popup key longpressses #9573

Closed

mcdurdin mentioned this pull request Apr 17, 2024

feat(android/engine): Use web host for key previews and longpress #6276

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

change(android,web) Use web-based popup key longpresses #9591

change(android,web) Use web-based popup key longpresses #9591

darcywong00 commented Sep 19, 2023 •

edited

Loading

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

edited

Loading

darcywong00 Sep 19, 2023

jahorton Sep 19, 2023

darcywong00 Sep 19, 2023

jahorton Sep 19, 2023 •

edited

Loading

darcywong00 Sep 19, 2023

darcywong00 Sep 19, 2023

jahorton Sep 19, 2023 •

edited

Loading

jahorton Sep 19, 2023

darcywong00 commented Sep 19, 2023

jahorton left a comment

mcdurdin left a comment

mcdurdin Sep 19, 2023

darcywong00 Sep 20, 2023

darcywong00 Sep 20, 2023

bharanidharanj commented Sep 21, 2023 •

edited

Loading

bharanidharanj commented Sep 21, 2023

bharanidharanj commented Sep 21, 2023

bharanidharanj commented Sep 21, 2023

keyman-server commented Sep 22, 2023

change(android,web) Use web-based popup key longpresses #9591

change(android,web) Use web-based popup key longpresses #9591

Conversation

darcywong00 commented Sep 19, 2023 • edited Loading

KeymanWeb changes

Keyman Engine for Android changes

User Testing

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

User Test Results

Test Artifacts

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jahorton Sep 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jahorton Sep 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

darcywong00 commented Sep 19, 2023

jahorton left a comment

Choose a reason for hiding this comment

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

bharanidharanj commented Sep 21, 2023 • edited Loading

bharanidharanj commented Sep 21, 2023

bharanidharanj commented Sep 21, 2023

bharanidharanj commented Sep 21, 2023

Test Results

keyman-server commented Sep 22, 2023

darcywong00 commented Sep 19, 2023 •

edited

Loading

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

edited

Loading

jahorton Sep 19, 2023 •

edited

Loading

jahorton Sep 19, 2023 •

edited

Loading

bharanidharanj commented Sep 21, 2023 •

edited

Loading