fix(android/engine): Fix OSK widths#10442
Conversation
User Test ResultsTest specification and instructions
Test Artifacts |
|
Sigh - this needs more investigation. I also notice when rotating to landscape, the menu options don't display the landscape layout 😖 |
8f634d6 to
4718b5b
Compare
|
The refactoring of detecting the device orientation seems to work now. @keymanapp-test-bot retest |
|
|
||
| // onConfigurationChanged() only triggers when device is rotated while app is in foreground | ||
| // This handles when device is rotated while app is in background | ||
| // using KMManager.getOrientation() since getConfiguration().orientation is unreliable #10241 |
There was a problem hiding this comment.
getResources().getconfiguration() was sometimes reporting PORTRAIT when the device was in LANDSCAPE orientation.
| Configuration newConfig = this.getResources().getConfiguration(); | ||
| if (newConfig != null && newConfig.orientation != lastOrientation) { | ||
| lastOrientation = newConfig.orientation; | ||
| int newOrientation = KMManager.getOrientation(context); | ||
| if (newOrientation != lastOrientation) { | ||
| lastOrientation = newOrientation; | ||
| KMManager.onConfigurationChanged(newConfig); | ||
| } |
There was a problem hiding this comment.
This retrieves newConfig but never tests it. Given it may have a wrong orientation, is this going to cause grief when passing it to onConfigurationChanged().
Also, I would move line 271 ... newConfig = ... inside the if() { statement before line 275 in order to keep its scope limited
There was a problem hiding this comment.
I retested the scenarios from #8704 and they seemed fine (restarting the app after rotating the device while the app is in the background).
lastOrientation is undefined so the app goes into this branch and refreshes with the correct dimensions. Maybe getOrientation() is the only buggy part?
|
Answer @mcdurdin 's question from standup:
I tested with my physical device locked in PORTRAIT and then locked in LANDSCAPE. Good news is the app and OSK remain locked in the orientation. |
android/KMAPro/kMAPro/src/main/java/com/tavultesoft/kmapro/AdjustKeyboardHeightActivity.java
Outdated
Show resolved
Hide resolved
| if (orientation == Configuration.ORIENTATION_PORTRAIT) { | ||
| return prefs.getInt(KMManager.KMKey_KeyboardHeightPortrait, defaultHeight); | ||
| } else if (rotation == Surface.ROTATION_90 || rotation == Surface.ROTATION_270) { | ||
| } else if (orientation == Configuration.ORIENTATION_LANDSCAPE) { |
There was a problem hiding this comment.
What do we do if ORIENTATION_UNDEFINED is returned? Seems like we fall into a hole?
There was a problem hiding this comment.
The defaultHeight initializes in l. 2023 so that value gets returned in l. 2033. I think typically that will be the portrait dimensions
Co-authored-by: Marc Durdin <marc@durdin.net>
|
Changes in this pull request will be available for download in Keyman version 17.0.252-alpha |
Follows #10373 and fixes #10241
This applies two things to fix OSK width to be full screen
User Testing
Setup - Install the PR build of Keyman for Android on an Android 12.0 device /emulator (SDK 31)