Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes wordpress-mobile/WordPress-Android#10492
Note: only one reviewer required
Description
The issue described turned out to be a problem with the text selection having a zero-width representation on the end of a paragraph or line and as such, when the drag and drop mechanism wanted to create the shadow image to be dragged around, it would then fail because you can't create an image with zero dimensions.
Narrowing down
In Aztec, we're using
EndOfParagraphMarker
span to mark the end of a paragraph, same happens with a newline.For example, this is an actual "\n" without it being end of paragraph:
And here we have an
EndOfParagraphMarker
:What the solution does
This solution makes use of the precedence in which events are handled. The Drag listener is only called when a drag operation has been determined to have started. For that, we first need to detect a long press gesture. Once we detect it, we figure out whether the long press happened within the boundaries of selected text (*). Then, if the selected is exactly 1 character and such spanned has either an
EndOfParagraphMarker
or aAztecVisualLinebreak
span, then we just mark the event as handled to prevent the regular View's handler from choking with it.I decided to apply this patchonly for the platforms where this behavior has been observed (Android 9 for 99% of users, and 10 as I was able to observe all of the times I tested on an API 29 emulator).
Other
I also observed this, and also (as can be seen up in the comments) the placement of the cursor is far left/top away from the actual touch points (where user's finger is touching).
(*) A note on the selection: it's not perfect, because we calculate a box starting on the top-left corner of the first line that has selected text, and the box ends on the right-bottom end of the last line containing selected text (this means you could long tap outside the selected text but still on one of the lines on the extremes and still trigger the listener code checks) but it should be enough. In practice, if you have code selected and you long tap around the edges it should be OK to assume you wanted to tap on the selection. And if that was not the case, then a single tap should unselect and get you ready to go. I don't expect troubles with this, but wanted to mention so the reviewer knows how I came about this.
Alternative solution
I first explored the solution of implementing our own drag & drop mechanism as can be seen here by mimicking what TextView does, and actually providing a non-zero dimensioned shadow view when it's the case that the selection has a zero-width representation.
This works well. However, I thought this was going too far, as we can just use the same long press detection mechanism and then just omit the action altogether (after all, who would want to drag a newline only? Seems like a gesture glitch more than anything).
In the case the current solution in this PR does not cover all the cases, we can always go ahead with the other branch solution (moving it here to AztecText instead of using AztecEditorFragment of course) and just make it work for every case. 👍
To test
Aztec demo app:
AztecVisualLinebreak
is identified as having been selected and as such we signal the listener as handled, to avoid having a crash.WPAndroid:
Should look like this:
(side note: on WPAndroid I tried this branch which implements pretty much the same things on AztecEditorFragment, to make sure whether there was a difference with where we'd set the long click listener, but found none, so it's better to have it here in AztecText directly and also save the problem for Gutenberg as well).
PR submission checklist:
RELEASE-NOTES.txt
if necessary.