fix(android): fix keyboard size after rotation and restore via onSizeChanged, after layout by jahorton · Pull Request #11722 · keymanapp/keyman

jahorton · 2024-06-06T07:08:46Z

This aims to fix the size and orientation issues we've been seeing for our touch-keyboards on Android.

Note that this PR adds to the adjustments made by ##11604. That does appear to properly ensure the correct "outer size" of the keyboard - that of the hosting WebView. The remaining issues appear to lie within the keyboard host-page inside of the WebView.

In order to properly handle this, ensuring proper layout for our WebView-internal keyboard, we split management of WebView's layout operations from the JS calls used to set the WebView-internal layout, the latter of which are based on the final size of the hosting WebView. To do so, we relocate the latter set of calls to onSizeChanged, overriding a superclass method that ought be triggered after onConfigurationChanged and its layout-update.

User Testing

TEST_KEYBOARD_REPEATED_RESTORE

Install the PR build of Keyman for Android on an Android device/emulator Android 13 (SDK 33)
Ensure the keyboard may be used as the system keyboard.
Pick any application and start editing text via the Keyman system keyboard (without changing orientation).
Dismiss the keyboard (by finishing your input and selecting something that doesn't need the keyboard).
Change your device's orientation: if in 'portrait', go to 'landscape' mode and vice-versa.
Re-activate the keyboard.
Repeat steps 3 through 6 at least five times, verifying that the OSK always displays with the correct width for the device's orientation.

TEST_GENERAL_USE: Test the system keyboard in general use, tossing in a few orientation changes from time to time. Verify that the keyboard always displays correctly for the current orientation.

…Changed, after layout Fixes: #10054

keymanapp-test-bot · 2024-06-06T07:08:51Z

User Test Results

Test specification and instructions

✅ TEST_KEYBOARD_REPEATED_RESTORE (PASSED): I tested this issue with the attached "Keyman 18.0.45-alpha-test-11722" build on an Android 13(SDK 33) emulator and physical environment: Here is my observation. (notes)
✅ TEST_GENERAL_USE (PASSED): I used the keyman as the system's keyboard in multiple applications. OSK always displays the correct device's orientation. It works correctly. Thank you.

Test Artifacts

Android

jahorton · 2024-06-06T07:15:17Z

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

+  @Override
  public void onConfigurationChanged(Configuration newConfig) {
    super.onConfigurationChanged(newConfig);


What set me on this path: we're calling methods that should be automatically called by the Android OS with parameters we're managing in order to maintain WebView and host page layout. That... feels odd. It does seem to be the correct time to adjust Android View layout data, at least.

The question this left me with, though: the API for Configuration suggests that it returns the "available" width and height. The thing is... the Configuration passed in as a parameter would have been computed before we set new layout parameters in place. So... is it actually right to expect the values to be current within this method? Or, do we need to wait for layout adjustments to take place first? If we're adjusting height, that could certainly be volatile... and made worse if we're also simultaneously flipping our width and height values due to orientation change.

mcdurdin · 2024-06-07T01:21:02Z

I am going to install this now

mcdurdin

Code changes look okay.

I know this is a bug fix, and we are trying to keep changes minimal. It seems like part of the problem here is the existing spaghetti is making it very difficult to reason about the orientation state.

One simple refactor I think we should try and apply is to reduce the number of loadJavascript calls:

Each call is costly
If we have a setDimensions function which is passed an object, then we can set as many of the dimensions as are passed in, in the specific order that they should be set, and without having to bubble that ordering detail all the up into the Java code.
It helps us see where we are only setting width or height or vice-versa.

mcdurdin · 2024-06-07T01:40:39Z

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

    loadJavascript(KMString.format("setBannerHeight(%d)", bannerHeight));
-    loadJavascript(KMString.format("setOskWidth(%d)", newConfig.screenWidthDp));
+    loadJavascript(KMString.format("setOskWidth(%d)", width));
+    // Must be last - it's the one that triggers a Web-engine layout refresh.
    loadJavascript(KMString.format("setOskHeight(%d)", oskHeight));


This smells like it should be a single function call, setDimensions({bannerHeight:%d,oskWidth:%d,oskHeight:%d}).

This may also be relevant:

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

Lines 426 to 436 in 91e91bf

public void onResume() {

DisplayMetrics dms = context.getResources().getDisplayMetrics();

int kbWidth = (int) (dms.widthPixels / dms.density);

// Ensure window is loaded for javascript functions

loadJavascript(KMString.format(

"window.onload = function(){ setOskWidth(\"%d\");"+

"setOskHeight(\"0\"); };", kbWidth));

if (this.getShouldShowHelpBubble()) {

this.showHelpBubbleAfterDelay(2000);

}

}

Calls to these functions (on master branch):

Only 1 call to setBannerHeight, right here.

4 calls to setOskHeight

2 calls to setOskWidth

dinakaranr · 2024-06-07T09:22:41Z

Test Results

TEST_KEYBOARD_REPEATED_RESTORE (PASSED): I tested this issue with the attached "Keyman 18.0.45-alpha-test-11722" build on an Android 13(SDK 33) emulator and physical environment: Here is my observation.

Installed the "Keyman 18.0.45.apk" file and gave all permissions to the application.
Checked the "Enable Keyman as system-wide keyboard" and set the keyboard as the default keyboard box on the settings page.

I have followed the acceptance steps. I tried to change the view from portrait to landscape and landscape to portrait. OSK always displays the correct width and height after the device's orientation. It works correctly after repeating steps more than 10 times. It works correctly. Thank you.

TEST_GENERAL_USE (PASSED): I used the keyman as the system's keyboard in multiple applications. OSK always displays the correct device's orientation. It works correctly. Thank you.

…Changed, after layout Fixes: #10054 Cherry-pick-of: #11722

keyman-server · 2024-06-10T18:04:56Z

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

mcdurdin · 2024-06-12T00:03:09Z

So far the test build has worked perfectly for me. I'll move to 18.0.53-alpha on my phone today.

fix(android): fix keyboard size after rotation and restore via onSize…

d868337

…Changed, after layout Fixes: #10054

jahorton requested review from darcywong00 and sgschantz as code owners June 6, 2024 07:08

keymanapp-test-bot bot added has-user-test user-test-required User tests have not been completed labels Jun 6, 2024

keymanapp-test-bot bot added this to the A18S3 milestone Jun 6, 2024

github-actions bot added android/ android/engine/ fix labels Jun 6, 2024

jahorton commented Jun 6, 2024

View reviewed changes

mcdurdin self-requested a review June 7, 2024 01:20

mcdurdin mentioned this pull request Jun 7, 2024

fix(android): check current orientation when redisplaying system keyboard #11604

Merged

mcdurdin requested changes Jun 7, 2024

View reviewed changes

jahorton mentioned this pull request Jun 7, 2024

refactor(android): merge layout JS calls into a single call #11734

Open

keymanapp-test-bot bot removed the user-test-required User tests have not been completed label Jun 7, 2024

mcdurdin modified the milestones: A18S3, A18S4 Jun 7, 2024

mcdurdin approved these changes Jun 10, 2024

View reviewed changes

jahorton added a commit that referenced this pull request Jun 10, 2024

fix(android): fix keyboard size after rotation and restore via onSize…

cef40eb

…Changed, after layout Fixes: #10054 Cherry-pick-of: #11722

jahorton mentioned this pull request Jun 10, 2024

fix(android): check current orientation, fix keyboard size after system keyboard rotations and resumes 🍒 🏠 #11747

Merged

Base automatically changed from fix/android/sys-keyboard-orientation to master June 10, 2024 05:15

jahorton merged commit b8943ea into master Jun 10, 2024

jahorton deleted the fix/android/split-layout-and-rescale branch June 10, 2024 05:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(android): fix keyboard size after rotation and restore via onSizeChanged, after layout#11722

fix(android): fix keyboard size after rotation and restore via onSizeChanged, after layout#11722
jahorton merged 1 commit intomasterfrom
fix/android/split-layout-and-rescale

jahorton commented Jun 6, 2024 •

edited

Loading

Uh oh!

keymanapp-test-bot bot commented Jun 6, 2024 •

edited

Loading

Uh oh!

jahorton Jun 6, 2024 •

edited

Loading

Uh oh!

mcdurdin commented Jun 7, 2024

Uh oh!

mcdurdin left a comment

Uh oh!

mcdurdin Jun 7, 2024

Uh oh!

dinakaranr commented Jun 7, 2024

Uh oh!

keyman-server commented Jun 10, 2024

Uh oh!

mcdurdin commented Jun 12, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	public void onResume() {
	DisplayMetrics dms = context.getResources().getDisplayMetrics();
	int kbWidth = (int) (dms.widthPixels / dms.density);
	// Ensure window is loaded for javascript functions
	loadJavascript(KMString.format(
	"window.onload = function(){ setOskWidth(\"%d\");"+
	"setOskHeight(\"0\"); };", kbWidth));
	if (this.getShouldShowHelpBubble()) {
	this.showHelpBubbleAfterDelay(2000);
	}
	}

Uh oh!

Conversation

jahorton commented Jun 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

User Testing

Uh oh!

keymanapp-test-bot bot commented Jun 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

User Test Results

Test Artifacts

Uh oh!

jahorton Jun 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mcdurdin commented Jun 7, 2024

Uh oh!

mcdurdin left a comment

Choose a reason for hiding this comment

Uh oh!

mcdurdin Jun 7, 2024

Choose a reason for hiding this comment

Uh oh!

dinakaranr commented Jun 7, 2024

Test Results

Uh oh!

keyman-server commented Jun 10, 2024

Uh oh!

mcdurdin commented Jun 12, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jahorton commented Jun 6, 2024 •

edited

Loading

keymanapp-test-bot bot commented Jun 6, 2024 •

edited

Loading

jahorton Jun 6, 2024 •

edited

Loading