[SuperEditor][Android] Hide expanded drag handles after deleting the selected text (Resolves #1936) #1951
Conversation
…selected text (Resolves #1936)
| @@ -655,7 +656,38 @@ class _AndroidDocumentTouchInteractorState extends State<AndroidDocumentTouchInt | |||
| widget.dragHandleAutoScroller.value!.ensureOffsetIsVisible(extentOffsetInViewport); | |||
| } | |||
|
|
|||
| void _onDocumentChange(_) { | |||
| void _onDocumentChange(DocumentChangeLog changeLog) { | |||
There was a problem hiding this comment.
My initial thought is that this a backwards place to recognize such a situation. We have various Android-specific widgets and configurations for the UI. The UI is where the user's interaction originates. Is there not a much simpler location in the Android-specific UI where we can recognize this situation, rather than try to reconstruct our knowledge of it after the fact?
There was a problem hiding this comment.
I don't think so, even though we can hide the controls in the cut handler, for the cases where the user presses the backspace button, or type when the selection is expanded, the IME will generate deltas that will modify the document, without any user interaction with the app. The user interaction in this case is only with the software keyboard.
There was a problem hiding this comment.
When the handles rebuild for the new collapsed selection, doesn't the Android interactor know that the user isn't currently dragging a handle, and therefore knows that we should collapse the handles?
There was a problem hiding this comment.
I'm not sure if I understood where you are suggesting to write this code...
There was a problem hiding this comment.
@angelosilvestre can you please frame that as a question? Where are you getting confused?
There was a problem hiding this comment.
@matthew-carroll I'll take a look at the existing code to frame a question.
|
@angelosilvestre are you waiting for me on this? It looks like I had an open question. |
|
@matthew-carroll I modified the implementation as you suggested. |
| @@ -1363,6 +1370,23 @@ class SuperEditorAndroidControlsOverlayManagerState extends State<SuperEditorAnd | |||
| @visibleForTesting | |||
| bool get wantsToDisplayMagnifier => _controlsController!.shouldShowMagnifier.value; | |||
|
|
|||
| void _onDocumentChange(_) { | |||
There was a problem hiding this comment.
It looks like this only moved the problem I identified before. The problem is that you're trying to identify a user interaction through a document change. My original question, which is still my question, is why can't you identify the user interaction when the user interaction happens, instead of after the fact?
This issue says its about "deleting the selected text". I assume that involves an expanded selection, then some kind of keyboard interaction, then a collapsed selection.
Doesn't this widget already know when the selection is expanded, and when the selection is collapsed? Why can't the widget recognize that the selection has collapsed and then hide the expanded handles, without worrying about document changes at all?
…selected text (Resolves superlistapp#1936) (superlistapp#1951)
[SuperEditor][Android] Hide expanded drag handles after deleting the selected text. Resolves #1936
On Android with an expanded selection, performing any action that causes text to be deleted, such as cutting, deleting, pasting or typing to replace the text, keeps the expanded handles visible. Trying to drag those handles throws an exception:
The expanded handles are kept visible because of #1754, where we changed the expanded handles to be allowed to be visible even with a collapsed selection. However, this should happen when the user is dragging the handles, not when the selection collapses due to a deletion.
Looking at other Android apps, it seems that the expected behavior is that no handle should be visible after deleting the selected text, not even the collapsed drag handle.
This PR changes the Android touch interactor to hide the expanded handles and any other controls when text is deleted, causing the selection to collapse.
We could also implement this by adding a listener directly to the editor and then listening for selection change events. If a better solution would be to find a way to make
shouldShowExpandedHandlesto befalseif the user is not dragging then I can dig into that.On iOS this issue doesn't happen, because the expanded handles are never visible when the selection is collapsed. I added the same test I added for android.