Skip to content
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): forgot to target es5 when minifying polyfilled worker #9409

Merged
merged 1 commit into from
Aug 4, 2023

Conversation

jahorton
Copy link
Contributor

@jahorton jahorton commented Aug 3, 2023

Fixes #9401.
Fixes #9408.
Fixes Sentry issue: KEYMAN-WEB-DS.

User Testing

TEST_ANDROID_5: Using an Android 5.0 (Lollipop) device with its originally-installed version of Chrome...

  1. Install and launch the Keyman app for Android
  2. Verify that no errors happen on startup
  3. Swap to sil_euro_latin for English if it is not already the active keyboard/language combination.
  4. Type app.
  5. Verify that the predictive-text banner is visible and shows reasonable suggestions.

This may be easiest to do through Android Studio emulation.

@jahorton jahorton added this to the A17S18 milestone Aug 3, 2023
@jahorton jahorton requested a review from mcdurdin as a code owner August 3, 2023 03:02
@keymanapp-test-bot keymanapp-test-bot bot added has-user-test user-test-required User tests have not been completed labels Aug 3, 2023
@keymanapp-test-bot
Copy link

keymanapp-test-bot bot commented Aug 3, 2023

User Test Results

Test specification and instructions

  • TEST_ANDROID_5 (PASSED): Tested with the attached PR build (Keyman 17.0.152-alpha-test-9409) in Android 5.1 Lollipop OS / API 22 Simulator and here is my observation: 1. I did not see the keyboard error message after opening the Keyman In-App. 2. Installed another keyboard using Settings option and I noticed that there is no keyboard error message after the new installation. 3. Typing 'app' in the text input screen shows the suggestion words fully visible on the banner. Seems to be fixed and working fine. Thanks. (notes)

Test Artifacts

Copy link
Contributor

@darcywong00 darcywong00 left a comment

Choose a reason for hiding this comment

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

lgtm

Is this the only minify that needed to specify the es5 target?

@jahorton
Copy link
Contributor Author

jahorton commented Aug 3, 2023

lgtm

Is this the only minify that needed to specify the es5 target?

The standard esbuild config setting already includes it; this lm-worker specific one doesn't reference that default, though.

@bharanidharanj
Copy link

bharanidharanj commented Aug 3, 2023

Test Results

  • TEST_ANDROID_5 (PASSED): Tested with the attached PR build (Keyman 17.0.152-alpha-test-9409) in Android 5.1 Lollipop OS / API 22 Simulator and here is my observation: 1. I did not see the keyboard error message after opening the Keyman In-App. 2. Installed another keyboard using Settings option and I noticed that there is no keyboard error message after the new installation. 3. Typing 'app' in the text input screen shows the suggestion words fully visible on the banner. Seems to be fixed and working fine. Thanks.

@keymanapp-test-bot keymanapp-test-bot bot removed the user-test-required User tests have not been completed label Aug 3, 2023
@jahorton jahorton merged commit 9bd12cb into master Aug 4, 2023
@jahorton jahorton deleted the fix/web/lm-worker-es5-minification branch August 4, 2023 02:16
@keyman-server
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants