Issue/342 image uploading scroll

[theck13]

Fix

View remains at scrolled position when uploading images as described in https://github.com/wordpress-mobile/AztecEditor-Android/issues/313 and https://github.com/wordpress-mobile/AztecEditor-Android/issues/342. The clearFocus() change in https://github.com/wordpress-mobile/AztecEditor-Android/pull/340 does clear the focus from the AztecText view, but the first focusable view (i.e. AztecText before this change) regains focus when the system returns from the media chooser. By allowing the parent view to be focusable, the AztecText view is not focused when the activity is resumed and the view is not scrolled when the image uploading overlay is updated.

Test

1. Place cursor near bottom of view.
2. Tap Media format button.
3. Choose image to insert.
4. Scroll to top of view.
5. Notice view does not scroll.
6. Place cursor near top of view.
7. Tap Media format button.
8. Choose image to insert.
9. Scroll to bottom of view.
10. Notice view does not scroll.

[khaykov]

Works nicely, but I noticed that the cursor still gets removed on upload progress. Also, I wonder, would it be a good idea to set focusable state of parent from within AztecText? This way we could make sure that it works with any parent.

[theck13]

I noticed that the cursor still gets removed on upload progress.

The "hack" of resetting the text content in the AztecText.refreshText method causes the view to scroll to the cursor if the view is in focus. As a hack for the hack, the focus was removed from the view, but that means the cursor is removed with the focus. If there is a way to remove the focus while still keeping the cursor, I'm up for it.

I wonder, would it be a good idea to set focusable state of parent from within AztecText?

We can get the parent of the AztecText view programmatically as with 0f9312c5dc01a5b42588ea67043ed48143390cc2, but the parent is a ViewParent which is not always a View. Even though AztecText is meant to have at least one parent view, it could be used as the root view in which case the scrolling issue would still occur.

[khaykov]

I tried to look into all this focus problem and AztecText.refreshText method and looks like we can apply the same fix as in disappearing paragraph's PR. We can replace refreshText with a method like this:

    fun updateMediaSpanProgress(span: AztecMediaSpan) {
        editableText.setSpan(span, text.getSpanStart(span), text.getSpanEnd(span), Spannable.SPAN_EXCLUSIVE_EXCLUSIVE)
    }

We probably want to pass only a relevant span to it. It's easy to do when we load initial images from Html (here) we might want to take this method out of post {} and pass span to it.

When adding media directly using LineBlockFormatter.insertMedia we might want to return span all the way back to place where we used to call refreshText (in case of demo app it will be here). I tested this method a bit, and looks like it works ok, but there might be some that I missed.

I hope it makes sense :)

[theck13]

Thanks for the tip, @khaykov! I tried the changes you suggested. That method works really well when adding media directly from the format toolbar. While testing media loaded from HTML, I found that the images overlap other images and text. Try pasting the example text below into the HTML view then toggling to visual view. Do you have any issues with your test code?

Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
<img src="http://goo.gl/i4Z69E">
<img src="http://goo.gl/dWbMtv">
Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur.
<img src="http://goo.gl/3urcsh">
<img src="http://goo.gl/8C4lZo">
Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.
<img src="http://goo.gl/dL6tOu">
<img src="http://goo.gl/FJQ3yw">
Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.
<img src="http://goo.gl/9fNoiH">
<img src="http://goo.gl/AbW6tw">
Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.

[khaykov]

Yep, it overlaps from time to time. I guess swapping drawable inside span does not work too well. I think we'll have to use refreshText() when downloading images for now. It's a part of initial loading, so it's less annoying.

Btw, so much OOM from those images!

[theck13]

The issue with overlapping images loaded from HTML is that we don't know the dimensions beforehand so the span size changes once the image is loaded. A possible fix would be to first fetch only the dimensions in order to render the placeholder image in the size of the image to be loaded and then do a second network call to load the final image. In the meantime, I applied the changes you suggested when adding media directly from the format toolbar with f9446e105efd05799d9d383d7c014fd97452b25c and kept refreshText() when loading media from HTML.

[khaykov]

Thanks for the changes! Looks good 👍