[SuperEditor][Android] Fix toolbar being hidden when releasing a long press (Resolves #2481) #2552
Conversation
… press (Resolves #2481)
| @@ -985,7 +993,7 @@ class _AndroidDocumentTouchInteractorState extends State<AndroidDocumentTouchInt | |||
| ..hideMagnifier() | |||
| ..blinkCaret(); | |||
|
|
|||
| if (didTapOnExistingSelection && _isKeyboardOpen) { | |||
| if (didTapOnExistingSelection && widget.isImeConnected.value) { | |||
There was a problem hiding this comment.
This was "is keyboard open" now it's "is IME connected". These aren't the same thing. Which one do we actually want here?
There was a problem hiding this comment.
We actually want to know if the keyboard is open, but the method this is using isn't reliable. It will always report false if there is an ancestor Scaffold that resizes to avoid the bottom insets.
We already did this change on iOS.
| @@ -1168,7 +1166,7 @@ class _AndroidDocumentTouchInteractorState extends State<AndroidDocumentTouchInt | |||
| } | |||
|
|
|||
| void _onLongPressEnd() { | |||
| _longPressStrategy!.onLongPressEnd(); | |||
| _longPressStrategy?.onLongPressEnd(); | |||
There was a problem hiding this comment.
Why change to ?? How do we get here without a long press strategy?
There was a problem hiding this comment.
I was calling _onLongPressEnd at the wrong time. Fixed it.
| } | ||
|
|
||
| // Show the toolbar even the selection is collapsed, because the user might |
There was a problem hiding this comment.
I'm confused about the timing of the user interactions that we're solving with this PR. I thought the PR described the toolbar disappearing after it was already visible. But this code seems to be making the toolbar appear. What's the actual bug here?
There was a problem hiding this comment.
It seems we don't need this code. Just need the _onLongPressEnd to be called.
| // Ensure the toolbar is not visible. | ||
| expect(SuperEditorInspector.isMobileToolbarVisible(), isFalse); | ||
|
|
||
| // Long press to show the toolbar. |
There was a problem hiding this comment.
Shouldn't long press open the magnifier?
There was a problem hiding this comment.
On Android, it shouldn't. The magnifier is displayed when dragging a handle.
|
|
||
| // Release the finger. | ||
| await gesture.up(); | ||
| await tester.pumpAndSettle(); |
There was a problem hiding this comment.
Why do we need to settle?
There was a problem hiding this comment.
It seems pump is enough.
[SuperEditor][Android] Fix toolbar being hidden when releasing a long press (Resolves #2481).
On Android, long pressing and releasing the gesture on an empty paragraph, or on a whitespace, is closing the toolbar:
before.webm
This is caused because we are calling the same method that updates the overlay controls when the user just performs a single tap. That method hides the toolbar when the selection is collapsed.
This PR changes the tap up handling to call
_onLongPressEndif the user was performing a long press, and modify this method to show the toolbar. We also need to hide the toolbar when the document changes, so it hides after the user starts typing. This will be a breaking change if an app performs some document change when interacting the toolbar and expects it to remain open. However, we already do that on iOS, so it might not be a problem.Also, this PR makes a change, that we already made in iOS, to avoid checking if the keyboard is open by looking for bottom inserts, because that is unreliable. Instead, we are now checking for an IME connection. This doesn't guarantee that the keyboard is visible, but we don't have an ideal alternative yet.
after.webm
On iOS, this seems to be already working as expected, so this PR adds a test for it.