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

Avoid fullscreen text entry UI in landscape mode #1842

Merged
merged 1 commit into from
Feb 20, 2020

Conversation

mchowning
Copy link
Contributor

@mchowning mchowning commented Jan 30, 2020

Fixes #1829

Summary

On Android when we are editing a paragraph block in landscape mode and you press the done button, an "Enter" keypress is registered. Therfore, the block is split at the point of the cursor. @iamthomasbishop indicated that the "Done" button should just close the keyboard, but that he would prefer not have the "full screen" editing type view when in landscape mode. Conveniently enough, the preferred solution is actually easier to implement. 😄 I would like to get @iamthomasbishop 's sign-off on this first though to make sure he's ok with how the view is mostly taken up by the keyboard.

But What Does It Look Like?!?!?

I'm glad you asked!

Before After
Screen Shot 2020-01-30 at 11 42 06 AM Screen Shot 2020-01-30 at 11 39 35 AM
android_landscape_no_extract mp4 android_landscape_no_extract mp4

To Test

  1. Open post with paragraph block
  2. Select paragraph block so that keyboard displays
  3. Switch the phone to landscape mode
  4. Observe that the phone looks like the "After" picture ^above^ (i.e., there isn't a "Done" button).

PR Submission Checklist

  • I have considered adding unit tests where possible.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@mchowning mchowning added [OS] Android [Status] Needs Design Review Needs design review or sign-off before shipping bugfix labels Jan 30, 2020
@mchowning mchowning added this to the 1.22 milestone Jan 30, 2020
@iamthomasbishop
Copy link
Contributor

This feels much more natural, imo. One tiny thing (which may be solved by this proposal if we work on it) is that there seems to be a momentary "stutter" in adjusting the border styling upon rotation in your last gif above, where it attempts to switch from top/bottom-only borders (in portrait) to full borders w/ rounded corner (in landscape).

@iamthomasbishop
Copy link
Contributor

One other thing, and we might want to tackle it separately. Do we want to automatically hide the keyboard upon rotation? I'm not 100% sure, just think it'd make the effects more visible.

@mchowning
Copy link
Contributor Author

Thanks for the comments @iamthomasbishop !

One tiny thing (which may be solved by this proposal if we work on it) is that there seems to be a momentary "stutter" in adjusting the border styling upon rotation in your last gif above, where it attempts to switch from top/bottom-only borders (in portrait) to full borders w/ rounded corner (in landscape).

This stutter is pretty noticeable on Android whereas I don't really notice it iOS. I suspect it may be tricky to address, so IMO it would be better to address it separately.

One other thing, and we might want to tackle it separately. Do we want to automatically hide the keyboard upon rotation? I'm not 100% sure, just think it'd make the effects more visible.

We could certainly do that. A few questions... Would you want this on both platforms or just on Android? If both, on iOS currently I believe that dismissing the keyboard generally also removes focus from the block, so would you want to lose focus on rotation as well on iOS, or do you not have a strong preference? Also, do you have a strong preference on whether we also dismissed the keyboard when rotating from landscape to portrait? FWIW, making the keyboard dismiss on Android will be pretty easy. At this point, I'm not sure how difficult that would be on iOS.

One interesting and unexpected (to me at least) side effect of dismissing the keyboard on Android is that dismissing the keyboard on rotation creates enough "movement" on the screen during rotation that the stutter you mentioned is actually much less noticeable (to my eyes at least).

@iamthomasbishop
Copy link
Contributor

This stutter is pretty noticeable on Android whereas I don't really notice it iOS. I suspect it may be tricky to address, so IMO it would be better to address it separately.

Sounds good 👍

Would you want this on both platforms or just on Android? If both, on iOS currently I believe that dismissing the keyboard generally also removes focus from the block, so would you want to lose focus on rotation as well on iOS, or do you not have a strong preference?

That's a good point. After thinking about this again, I think we should probably just simplify the whole situation and leave it as it currently is (don't remove focus/hide keyboard). While hiding the keyboard would certainly clear space, removing the focus might feel like a bug in practice (and require the user to re-select the block).

Copy link
Contributor

@cameronvoell cameronvoell left a comment

Choose a reason for hiding this comment

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

Looks good! Nice improvement. Tested on Samsung S10, Galaxy Tab S3 on API 29 and Emulator Pixel 2 running API 22.

@@ -138,6 +139,7 @@ public void onSelectionChanged(int selStart, int selEnd) {
ReactAztecText.this.propagateSelectionChanges(selStart, selEnd);
}
});
this.setImeOptions(getImeOptions() | EditorInfo.IME_FLAG_NO_EXTRACT_UI);
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a great improvement! Read through IME options here and agree IME_FLAG_NO_EXTRACT_UI looks like the right choice.

@etoledom etoledom modified the milestones: 1.22, 1.23 Feb 10, 2020
@mchowning mchowning modified the milestones: 1.23, 1.24 Feb 20, 2020
@mchowning mchowning force-pushed the issue/1829_soft-keyboard-landscape branch from c8b7866 to 3723be0 Compare February 20, 2020 23:04
@mchowning mchowning merged commit 5041096 into develop Feb 20, 2020
@mchowning mchowning deleted the issue/1829_soft-keyboard-landscape branch February 20, 2020 23:52
@mchowning
Copy link
Contributor Author

mchowning commented Mar 9, 2020

Hey @iamthomasbishop ! I was looking at this in the WPAndroid app, and I think we may want to revert this change. With the app toolbar virtually no space is left above the keyboard for the post, which results is a pretty poor experience. 😞 WDYT?

landscape_tight_android mp4

@iamthomasbishop
Copy link
Contributor

Agreed that's not great, although tbh landscape w/ any sort of text editing is quite restrictive considering the height of the keyboard. In the long run, I think we might want to consider a more flexible/collapsing app bar — which would help a lot — but for the time being, we can probably revert.

mchowning added a commit that referenced this pull request Mar 9, 2020
…t-keyboard-landscape"

This reverts commit 5041096, reversing
changes made to ddb8b3f.
mchowning added a commit that referenced this pull request Mar 10, 2020
#1989)

* Revert "Merge pull request #1842 from wordpress-mobile/issue/1829_soft-keyboard-landscape"

This reverts commit 5041096, reversing
changes made to ddb8b3f.

* Remove release note for reverted landscape mode change
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix [OS] Android [Status] Needs Design Review Needs design review or sign-off before shipping
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Android landscape mode - soft keyboard done/next button adds new line/block
4 participants