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): unrevert #11258, leaving OSK hidden before instructed to display 🍒 🏠 #12058

Merged
merged 3 commits into from
Jul 30, 2024

Conversation

jahorton
Copy link
Contributor

@jahorton jahorton commented Jul 30, 2024

Cherry-pick-of: #12049

User Testing

TEST_REPRO: Attempt to reproduce #11962 using Keyman for Android.

  • Install the package provided within that issue.
  • Force-quit the app.
  • Relaunch the app.
    • If the issue is not properly fixed, an error notification will appear on app-relaunch.

TEST_ANDROID: Do simple general-use testing with Keyman for Android and report back if any odd behaviors occur.

TEST_IOS: Do simple general-use testing with Keyman for iOS and report back if any odd behaviors occur.

TEST_WEB: Do simple general-use testing with the "Test unminified Web" test page and report back if any odd behaviors occur.

TEST_DEV_SERVER: Use Keyman Developer's web-keyboard testing page and verify that it still works correctly. (We're mostly checking to see that this doesn't result in an immediate, obvious break.)

@jahorton jahorton requested a review from ermshiperete as a code owner July 30, 2024 01:17
@keymanapp-test-bot keymanapp-test-bot bot added the user-test-missing User tests have not yet been defined for the PR label Jul 30, 2024
@keymanapp-test-bot
Copy link

keymanapp-test-bot bot commented Jul 30, 2024

User Test Results

Test specification and instructions

@keymanapp-test-bot keymanapp-test-bot bot changed the title fix(web): unrevert #11258, leaving OSK hidden before instructed to display 🍒 fix(web): unrevert #11258, leaving OSK hidden before instructed to display 🍒 🏠 Jul 30, 2024
@github-actions github-actions bot added web/ cherry-pick Change already merged into another (stable) branch fix labels Jul 30, 2024
@keymanapp-test-bot keymanapp-test-bot bot added this to the A18S7 milestone Jul 30, 2024
@keymanapp-test-bot keymanapp-test-bot bot added has-user-test user-test-required User tests have not been completed and removed user-test-missing User tests have not yet been defined for the PR labels Jul 30, 2024
@dinakaranr
Copy link

Test Results

I tested this issue with the attached "Keyman 17.0.329" build on the Android 14 & iPhone, web(Chrome & Firefox), and keyboard developer. Here is my observation.

  • TEST_REPRO (passed):
  1. Install the "Keyman 17.0.329-test-12058" build.
  2. Relaunch the app after force quit.
  3. No error notification appears when forced to quit the app.
  • TEST_ANDROID (passed):
  1. Installed the "Keyman-17.0.329.apk" file and gave all permissions to the application.
  2. Checked the "Enable Keyman as system-wide keyboard" and set the keyboard as the default keyboard box on the settings page.
  3. Open the Keyman app. Enable the "Predictions" and install the "Dictionary."
  4. Added a sentence to the notepad and Chrome (search).
  5. Enter a word and observe that the words are suggested on the banner.
  6. The keyboard works on portrait and landscape viewport. Keyboard set as system keyboard.
  7. Installed/uninstalled multiple keyboards.
    It looks like it was expected.
  • TEST_IOS (passed):
  1. Installed the "Keyman-17.0.329" build from the testflight app and gave all permissions to the application.
  2. Checked the "Enable Keyman as system-wide keyboard" and set the keyboard as the default keyboard box on the settings page.
  3. Open the Keyman app. Enable the "Predictions" and install the "Dictionary."
  4. Added a sentence to the notepad and Chrome (search).
  5. Enter a word. Observe that the words are suggested on the banner.
  6. The keyboard works on portrait and landscape viewports. Keyboard set as system keyboard.
  7. Installed/uninstalled multiple keyboards.
    It looks like it was expected.
  • TEST_WEB (passed):
  1. Navigate to the "Tests unminified Keymanweb" --> "KeymanWeb Sample Page - Unminified Source" pages.
  2. "English - English": It works well in the text box and OSK keyboard.
  3. "Lao - Lao Basic": It works well in the text box and OSK keyboard.
  4. "English - Classic 10-key": OSK board appears but it is blank now. An alert window appears for very few seconds.
  5. It works in Khmer and French keyboards too. Thank you.

TEST_DEV_SERVER (passed):

  1. Added the Amharic & Cameroon keyboards loaded the .kmn, and pressed F7 to compile.
  2. SUITE_DEBUGGER: Test new core-based debugger - Compile and recompile the keyboard
  3. SUITE_SERVER: Test new Keyman Developer Server - Keyboard launched on the browser with OSK.
    I tested the above two test suites and work as per expected. Thanks.

@keymanapp-test-bot keymanapp-test-bot bot removed the user-test-required User tests have not been completed label Jul 30, 2024
Base automatically changed from change/web/cherry-pick/11962-revert-11174 to stable-17.0 July 30, 2024 23:14
Copy link
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.

LGTM

(process note: it looks like you had based this PR on HEAD of stable-17.0 and not on the parent PR branch originally -- which was based on HEAD~1, so I had to merge stable-17.0 in after merging parent to remove unrelated files from this PR history)

@mcdurdin mcdurdin merged commit 1dfefef into stable-17.0 Jul 30, 2024
3 checks passed
@mcdurdin mcdurdin deleted the fix/web/cherry-pick/unrevert-11258 branch July 30, 2024 23:18
@mcdurdin
Copy link
Member

4. "English - Classic 10-key": OSK board appears but it is blank now. An alert window appears for very few seconds.

I missed this before merging (I have not yet done the 17.0 stable build). Is this new behaviour? Or does it happen in the same way on 17.0.328? If you can reproduce this, can you share a video?

@keyman-server
Copy link
Collaborator

Changes in this pull request will be available for download in Keyman version 17.0.329

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick Change already merged into another (stable) branch fix has-user-test oem/fv/ oem/ stable web/
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants