-
-
Notifications
You must be signed in to change notification settings - Fork 112
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
fix(web): modular app/webview KMW hosted within Android app 🧩 #8544
fix(web): modular app/webview KMW hosted within Android app 🧩 #8544
Conversation
…s Android app functional
User Test ResultsTest specification and instructions 🟥 SUITE_ANDROID_MENU_FUNCTIONALITY: Menu Functionality Tests
✅ SUITE_ANDROID_KEYBOARD_FUNCTIONALITY: Keyboard Tests42 tests in 3 groups PASSED
✅ SUITE_ANDROID_KNOWN_BUGS: drawn from a list of bugs15 tests in 3 groups PASSED
🟧 SUITE_IOS_MENU_FUNCTIONALITY: Menu Functionality Tests, in-app##
✅ SUITE_IOS_SYSTEM_KEYBOARD:12 tests in 4 groups PASSED
Retesting Template
Test Artifacts
|
Re: the iOS build failure:
Looks like recent updates to the build agents caused the artifact upload to fall over? At least the build completed fine up 'til then. |
@@ -16,13 +16,13 @@ | |||
--> | |||
<script src="es6-shim.min.js"></script> | |||
<script src="keymanandroid.js"></script> | |||
<script src="sentry.min.js"></script> | |||
<script src="keyman-sentry.js"></script> |
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.
Are we needing to include both sentry.min.js
and keyman-sentry.js
?
I thought keyman-sentry.js
was just the renamed component
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.
- In converting our Sentry-integration setup to modular form... well... turns out we can't quite directly
import
the Sentry libraries and directly integrate with them.
esbuild
falls over due to unsupported code transformation rules when targeting ES5, which we do b/c of our Android 5.0 support (and possibly other environments, too)- We need to import
KEYMAN_VERSION
stuff, which is now modularized, so... we'll either have to prepend bundled Sentry or include it in the asset bundles directly.- I opted for the latter and have made the necessary changes for both mobile apps.
All of that to say... yes, we need to include both. The TS build system doesn't support prepending sentry.min.js
on the module-based form of keyman-sentry.js
. Prepending was "easy" with the namespaced TS format, but that only works for builds that output to a single file - something that modular builds don't support without additional tooling (like a later esbuild
step).
If we prefer, I can gerry-rig something like I already did with the lm-worker
build to prepend it and do sourcemapping on the resulting setup. But, since we're only using it within the mobile apps anyway, and it's not needed within the WebWorker, I didn't think it worth the extra hassle.
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.
Yes, this is the appropriate answer -- there's no need to concatenate these files since they are used only within Keyman Engine for Android and Keyman Engine for iOS.
Pretty sure that's a separate issue entirely, though I can't quite find a perfect matching issue in our issue list. Edit: I have opened issue #8798 to track this. |
Test (Keyman - Developer) build failure:
Yep, that's pretty reasonable at this point. I haven't taken steps yet to reintegrate KMW with Developer. Getting the |
🎉 Big milestone! 🎉
With these changes in place, the Android app has been (lightly) tested to work with a fully-modular, bundled Keyman Engine for Web core!
Turns out there were a few loose threads here and there to mend and reconnect - it did take most of a day to debug the biggest issues and resolve things to the current state shown in this PR. But...
Other details:
<textarea>
element; the context is managed in pure TS / compiled JS.app/webview
format,keyman.context
serves that role now.app/webview
Web build product.import
the Sentry libraries and directly integrate with them.esbuild
falls over due to unsupported code transformation rules when targeting ES5, which we do b/c of our Android 5.0 support (and possibly other environments, too)KEYMAN_VERSION
stuff, so... we'll either have to prepend bundled Sentry or include it in the asset bundles directly.debug
variant oflm-worker
, which bloated the result past 1 MB.lm-worker
component it includes (without Worker sourcemaps at all) should be around 133 kB.minifyIdentifiers
flag, which was disabled for the result above.app/webview
build brings the size down from 461 kB to 378 kB.engine/main
(and otherengine/
components) are written in a manner that avoids the assumption that a consistent "singleton" variable / reference point exists.window['keyman']
is not assumed to exist for code at that level.app/
level components also avoid this assumption. This should be safe to relax (even long-term) forapp/webview
, though.🚧 Known issues 🚧:
setIsChiral
connections in favor of chasing down... everything else. I'm out of time to address that at the moment, unfortunately.User Testing
Since there has been a lot of changes - some small, some large - under the hood in pursuit of full modularization, we're going to want a near-full regression-test suite on this PR. At minimum. I may add other tests at a later point. But that should give us a good testing baseline to start with.
Gather Assets for Testing
Setup Steps
User Testing
SUITE_ANDROID_MENU_FUNCTIONALITY: Menu Functionality Tests
click to expand
Various versions of Androids
UI for these tests assume default English locale
TEST_GET_STARTED: This tests the "Get Started" menu from a fresh installation
Expand to see how to do and verify this
TEST_INSTALL_KD: This menu is accessed via Get Started menu or the Settings button-->"Install Keyboard or Dictionary"
TEST_KEYMAN_SETTTINGS:
Expand to see how to do and verify this
TEST_SHARE: Type and verify text can be shared to external app
TEST_KEYMAN_BROWSER: Keyman Browser ###
TEST_TEXT_SIZE: Verify text can be rescaled from Text Size 16 to 72
TEST_CLEAR_TEXT: Verify text can be cleared
TEST_INFO:
TEST_INSTALL_UPDATES: This menu option only appears when a language resource (keyboard or lexical-model) update is available
Expand to see how to do and verify this
TEST_CHANGE_DISPLAY_LANG: This tests changing the display language for the App and assumes starting with English locale
TEST_ADJUST_K_HEIGHT: This menu option allows the user to adjust the OSK height for the current orientation (portrait or landscape). The customized height is saved as a preference
Expand to see how to do and verify this
TEST_SPACEBAR_CAPTION: This menu option allows the user to adjust the label displayed on the keyboard spacebar
Expand to see how to do and verify this
SUITE_ANDROID_KEYBOARD_FUNCTIONALITY: Keyboard Tests
click to expand
Various versions of Androids
UI for these tests assume default English locale
In-app Keyboards
These tests are run in the context of typing in the Keyman app
click to expand
TEST_INAPP_LATIN_KEYBOARD_PORTRAIT: English(EuroLatin SIL)
In portrait orientation, verify OSK is visible and fills the width the bottom of the screen
q
key worksp
key worksSHIFT
123
1
key works0
key worksTEST_INAPP_LATIN_KEYBOARD_LANDSCAPE: English(EuroLatin SIL)
In portrait orientation, verify OSK is visible and fills the width the bottom of the screen
q
key worksp
key worksSHIFT
123
1
key works0
key worksTEST_INAPP_LONG_PRESS: English(EuroLatin SIL)
Type on the OSK using the following scenarios and verify expected output:
TEST_INAPP_NON_LATIN_KEYBOARD: Non-Latin script Keyboard
TEST_INAPP_ROTATE_P-TO-L:
TEST_INAPP_ROTATE_L-TO-P:
System Keyboards
These tests are run with Keyman selected as a system keyboard, and in the context of typing in an external app (like Google Keep)
click to expand
TEST_SYSTEM_LATIN_KEYBOARD_PORTRAIT: English (EuroLatin SIL)
In portrait orientation, verify OSK is visible and fills the width the bottom of the screen
q
key worksp
key worksSHIFT
123
1
key works0
key worksTEST_SYSTEM_LATIN_KEYBOARD_LANDSCAPE: English(EuroLatin SIL)
In portrait orientation, verify OSK is visible and fills the width the bottom of the screen
q
key worksp
key worksSHIFT
123
1
key works0
key worksTEST_SYSTEM_LONG_PRESS: English(EuroLatin SIL)
Type on the OSK using the following scenarios and verify expected output:
TEST_KEYBOARD_PICKER: Keyboard Picker menu to switch input method
This tests the Keyboard Picker menu which lists other system input methods at the bottom of the menu. These appear only in Keyman system keyboard
TEST_SYSTEM_ROTATE_P-TO-L:
TEST_SYSTEM_ROTATE_L-TO-P:
TEST_EXT_BLUETOOTH_KEYBOARD: External (Bluetooth) keyboard
This test is run with an external keyboard (USB or bluetooth) connected to the device
Expand to see how to do and verify this
caps lock
has no effect when typingtab
toggles to next fieldenter
adds a newlinebackspace
removes a characterTEST_EXT_AZERTY_KEYBOARD: External European AZERTY keyboard
This test is run with an external European AZERTY keyboard paired to the device. It should contain the 102nd key
<>
on the bottom row next to the left-shift.azerty
appears<
appears>
appearsSUITE_ANDROID_KNOWN_BUGS: drawn from a list of bugs
click to expand
Various versions of Androids
UI for these tests assume default English locale
TEST_GLOBE_KEY Holding backspace should not trigger a longpress on another random key (7155)
FAIL
this test if the longpress menu of the previous letter appearsTEST_SUGGESTION_NO_CAPITAL Initial suggestions when starting a new field are not always Initial cased (7165)
FAIL
this test.TEST_BROKEN_SUGGESTION selecting the "undo" suggestion breaks subsequent input (7167)
When we select the "undo" suggestion that becomes available with Backspace after accepting a suggestion, on the next keystroke Keyman inserts a broken suggestion.
Problem
from banner."Prob"
(undoing the last suggestion).Prob
is shown, PASS this test; otherwise ifProthe
orProblem
!TEST_SHIFT_KEY_STUCK At start of input, pressing globe key, going back, leads to stuck shift key (7169)
<
button on the top left of the screen or Android' back button at the bottom of the screenShift
key is stuck and unusable, FAIL this test.TEST_REDUCED_TOAST_NOISE - Verifies the number of Toast notifications are reduced (7170)
SUITE_IOS_MENU_FUNCTIONALITY: Menu Functionality Tests, in-app##
click to expand
Test Cases
TEST_GET_STARTED: Get Started
+
.TEST_ADD_NEW_KEYBOARD: Add New Keyboard
This menu is accessed via Get Started menu or Settings menu
TEST_ADD_NEW_KEYBOARD_FROM_FILE:
.kmp
file)TEST_SWITCH_KEYBOARD
TEST_TEXT_SIZE: Text Size
TEST_CLEAR_TEXT: Clear Text
TEST_INFO: Info
TEST_SETTINGS: Settings
Installed Languages ... Add Language
Select an installed language ... Add (another) Keyboard
Select an installed language ... Dictionary
Validate that when both predictions and corrections are off, banner is not visible
Add/Remove dictionaries - validate
If multiple dictionaries are available, test swapping between them
TEST_EUROLATIN:
English (Eurolatin)
in-appq
key worksk
key worksSHIFT
123
1
key works0
key worksNote that EuroLatin2, a highly customized keyboard explicitly designed for mobile, is not found; hence EuroLatin (SIL) is used for testing this instead.
US Basic
in-app+
123
key works, presenting options for a currency layer and a symbol layer.£
key.©
key.e
on the default layer and select a subkey.Settings > Languages > English > US Basic
menu, follow the "Help link" and ensure it displays appropriate help.Settings > Languages > English > US Basic
menu, scan the QR code with a phone and test that it links to the current version of that keyboard's public download page on keyman.com.SUITE_IOS_SYSTEM_KEYBOARD:
click to expand
Test the following after setting Keyman as a system keyboard.
Test Cases