Wordpress-android: WordPress Stories : Story Title UI & Keyboard Issues

[jd-alexander]

• [x] Currently, the Story Title is not persisted as the Post Title
• [x] The story title is not being shown on the bottom sheet when the user has set it.
• [x] The story title disappears when the user navigates to a different section of the bottom sheet
• [x] The story title should have a default value of Untitled once the user doesn't set a title.
• [x] The keyboard should be shown when the bottom sheet is presented.
• [x] Ensure that the keyboard gets dismissed appropriately when the bottom sheet is dismissed etc.

[mzorz]

Just coming here to keep things documented - I'm documenting here some possibilities explored and the result of analyzing the code and related PRs.

As mentioned here https://github.com/wordpress-mobile/WordPress-Android/pull/12486

We are currently using a style that makes the window full screen but that interferes with the keyboard's ability to adjust the view. i.e adjustResize and the fullScreen styles don't play well together.

Already discussed this elsewhere but wanted to leave my findings written here I've been trying a few things re: adjustResize and windowFullScreen being in combination as a possible cause of the problem, and was able to zero that out.

• For example, if we force the EditPostActivity (which uses adjustResize and is not windowFullScreen) to open the bottom sheet for the story format (which contains the story_title EditText component), you'll see we have the same problem about the keyboard overlapping the bottom sheet (see this gist to try, also I uploaded a video here https://cloudup.com/c9b6YwnYO7G).

• Just in case the EditPostActivity was introducing some more noise (maybe also because we have other objects such as EditText->Aztec that grab focus and show the keyboard here), I made yet another test with this gist here, which just shows the prepublishing bottom sheet when you tap the FAB on WPMainActivity, also added adjustResize attribute to WPMainActivity in the manifest, and still can see the overlapping problem there.

So, with that zeroed out, the problem of the overlapping keyboard seems to be lying elsewhere. In this sense, I think we're in the right direction by coping with that with the KeyboardResizeViewUtil util class, introduced in https://github.com/wordpress-mobile/WordPress-Android/pull/12328.

Also found some interesting reading in this post, but I think the current solution with KeyboardResizeUtil is good enough.

On the publish button at the bottom being cut off

This problem is mentioned here https://github.com/wordpress-mobile/WordPress-Android/pull/12486 and was also observed in the alpha2 branch in WPAndroid.

Been trying this: in smaller screen sizes (tried on a Samsung J2) the RecyclerView works ok and lets you scroll, so the "cut off" part doesn't feel as weird. But, on bigger screens, the content is cut off and there's no possibility to scroll content within the bottom sheet limits nor push the bottom sheet all the way up whatsoever. I've seen other solutions for this (when content exceeds the space available for the bottom sheet by default) by using a NestedScrollView, but I don't think this is going to work for us given we already have a RecyclerView within the content space. I also did some random tests with NestedScrollView without luck.

I can see we're already having a way to ensure a minimum height for the bottom sheet as per this prepublishing_fragment_container_min_height, while this doesn't seem very portable / long term solution at first, I think we could add a new value there to ensure this new UI for Stories doesn't get cut as well for now. I think you suggested this as a fallback solution elsewhere, but writing it here in lieu of documenting since I also went into checking this as well.

On focus handling

In general, I was a bit hesitant to have a RecyclerView with EditText inside and then handling focus there, I remember it being a common source of problems in the past (for example in Mobile Gutenberg, adding several paragraph blocks one after the other is nothing else but a list of EditText struggling for focus).
I was curious to know whether reconverting this to a normal layout that adds sections as needed would simplify things and get rid of these problems, but it may not be a trivial refactor to make right now. What we could do though, is test a simple bottom sheet with an EditText, and see if this is a problem there as well. So I tried just having an EditText alone, replacing the RecyclerView in post_prepublishing_home_fragment, ~and the focus issues remain to be there (I had to use the same artifacts as requesting focus and signaling the keyboard to show up to make it appear).~

EDIT: _Developing..._ one new thing here: the very basic act of tapping on the EditText makes it both grab the focus and make the keyboard appear, so here we have a difference with the situation when the EditText is contained as an item in the RecyclerView. Will continue down this path to see if there's anything else we can spot there 👍 - gist on top of #12486 here

So for now, I think things are in a decent spot, with https://github.com/wordpress-mobile/WordPress-Android/pull/12486 being the next thing to check (I'm planning to get to that soon)

EDIT 2:, checked having an EditText on top of the RecyclerView, with the RecyclerView having another EditText. In this video we can see the first EditText grabs and holds focus while the keyboard goes up. Tapping in the second EditText (contained as an item in the RecyclerView), focus is lost https://cloudup.com/cMGF2oXpwKx

Preliminar conclusion

I think you're on the right direction here, even though it's difficult to untangle the series of issues that come with focus handling, this is probably as good as this gets 👍

Let's try and single out those other remaining issues and see if we can fix them - also depending on how bad they are, we may as well decide to defer for later. Thanks for these efforts @jd-alexander 🙏

[jd-alexander]

Thanks for this indepth investigation @mzorz this helps a lot 🙏

I was curious to know whether reconverting this to a normal layout that adds sections as needed would simplify things and get rid of these problems, but it may not be a trivial refactor to make right now.

If this solution works for us, then it would be worth the refactor for sure.

Let's try and single out those other remaining issues and see if we can fix them - also depending on how bad they are, we may as well decide to defer for later. Thanks for these efforts @jd-alexander 🙏

No problem! Yes, let's work on resolving the other issues while we come up with a solution for this.

[mzorz]

If this solution works for us, then it would be worth the refactor for sure.

👋 @jd-alexander I've been thinking about it and I'm leaning towards going down this path 👍 . I don't think the RecyclerView is needed here: there's just a few elements / sections, all of them are different to each other, and none of them are going to get recycled given all are shown on the screen at the same time so it's really not honoring any of its purposes to have this component here. And to add to that it also brings the focus problems, which the more I think about it we're trying to deal with focus handling by approaching smaller issues one by one, in the end adding workarounds that don't seem to play well with one another and causing regressions or introducing new problems like the IllegalArgumentException mentioned in #12486 .
These workarounds IIRC may also be more complex to deal with other IME devices such as hardware keyboards.

Here's what I have in mind for a plan, let me know if you agree:

• remove these focus handling bits in PrepublishingHomeViewHolder:

            val focusStoryTitleEditTextAndShowKeyboard = {
                storyTitle.focusAndShowKeyboard()
                storyTitle.postDelayed({ storyTitle.requestFocus() }, STORY_TITLE_EDIT_TEXT_REQUEST_FOCUS_DELAY)
            }

and

            storyTitle.postDelayed({
                focusStoryTitleEditTextAndShowKeyboard()
            }, STORY_TITLE_EDIT_TEXT_REQUEST_FOCUS_DELAY)

(I think these are all we need to remove but let's double check). We still need the KeyboardResizeViewUtil class.
This leaves things in a way where the Story title is not focused right away, but at least the focus doesn't do weird things. Also, if you tap on the EditText within the RecyclerView, even if the caret will blink once and then disappear, you can still type and text will be input in the EditText (because the InputConnection that bridges the keyboard is still established on the same EditText there).

• remove the RecyclerView form the bottom sheet
  This will let us check the 3 remaining checkboxes in this issue with using the focusAndShowKeyboard() method in onBind():

• [ ] The keyboard should be shown when the bottom sheet is presented.

This can be done by requesting the focus / showing the keyboard with the focusAndShowKeyboard() method 👍

• [ ] Ensure that the keyboard gets dismissed appropriately when the bottom sheet is dismissed etc

This one would work by default when the EditText is unfocused and destroyed when the bottom sheet fragment gets detached from the Activity and therefore from the InputConnection is dropped as well

• [ ] Add a flag to the Tags Fragment that tells it not to close the keyboard once the keyboard is open on the home screen of the prepublishing bottom sheet.

For this one I'm not sure how this will work out but in any case I wouldn't mind if the keyboard is hidden and then shown back again while navigating to the Tags Fragment (it's more predictable than having the other timing / focus related issues).

These could be two separate PRs, let me know what you think 🙏

Also adding a final thing here: the handling of back needs to get revisited, apparently we have to tap on back arrow many times to dismiss the bottom sheet, see this video here https://cloudup.com/c6QLZVPHZ4r (adding here because it's related to focus handling)

Thanks a lot 🙏

[jd-alexander]

Thanks for this detailed breakdown @mzorz 🙏

👋 @jd-alexander I've been thinking about it and I'm leaning towards going down this path 👍 . I don't think the RecyclerView is needed here: there's just a few elements / sections, all of them are different to each other, and none of them are going to get recycled given all are shown on the screen at the same time so it's really not honoring any of its purposes to have this component here.

This is a good discovery. I am thinking that we can try the approach of removing the StoryTitle layout from the RecyclerView and it outside the component so we don't have the same focus issues. I will work on doing this tomorrow so that we can get this resolved.

remove these focus handling bits in PrepublishingHomeViewHolder:

Yes, I am looking forward to removing these approaches as they weren't as optimal and were necessary due to the nature of the EditText being in the RecyclerView

For this one I'm not sure how this will work out but in any case I wouldn't mind if the keyboard is hidden and then shown back again while navigating to the Tags Fragment (it's more predictable than having the other timing / focus related issues).

Good point, I don't mind this either, so I will give this a try once the focus issues are resolved and share what the experience is like.

Also adding a final thing here: the handling of back needs to get revisited, apparently we have to tap on back arrow many times to dismiss the bottom sheet, see this video here https://cloudup.com/c6QLZVPHZ4r (adding here because it's related to focus handling)

This has been resolved in develop here so once we merge develop into the Stories base branch, we will be up to date.

So the action items are to resolve the points you mentioned in the comments. The only deviation will be that we won't remove the entire RecyclerView component since we can try utilize a layout above it to accomplish the desired outcome.

Again, thank you so much for assisting with this. Nothing like two pair of eyes on one of the most troubling parts of the Android API :)

[mzorz]

This is a good discovery. I am thinking that we can try the approach of removing the StoryTitle layout from the RecyclerView and it outside the component so we don't have the same focus issues. I will work on doing this tomorrow so that we can get this resolved.

This sounds like a nice way to handle it!

So the action items are to resolve the points you mentioned in the comments. The only deviation will be that we won't remove the entire RecyclerView component since we can try utilize a layout above it to accomplish the desired outcome.

Sounds good! Thanks for documenting the decision here 🙇

Again, thank you so much for assisting with this. Nothing like two pair of eyes on one of the most troubling parts of the Android API :)

🙌🏻 😃 glad to help! Great team work here so far 💯