[SuperEditor][SuperReader][Android] Fix long-press firing after scrolling and releasing the pointer (Resolves #1535) #1540
Conversation
…ling and releasing the pointer (Resolves superlistapp#1535)
|
@Jethro87 could you please try this PR? |
| @@ -324,6 +324,12 @@ class _AndroidDocumentTouchInteractorState extends State<AndroidDocumentTouchInt | |||
|
|
|||
| if (isScrolling) { | |||
| _isScrolling = true; | |||
|
|
|||
| // The long-press timer is cancelled if a pan gesture is detected. | |||
There was a problem hiding this comment.
This explanation is too difficult to understand. It's not clear which part is describing what's happening now, or what isn't happening now, or what might be happening now. Please try to describe the relevant details in a way that a reader would understand by opening this file and looking directly at this explanation, like I'm doing now.
For example, reading this without all the pre-existing conversation and investigation, I don't know what it means to say "The long-press timer is cancelled if a pan gesture is detected". Does that mean we're cancelling it now? Does that mean it's already cancelled? Does that mean this if-statement "detected a pan gesture"? Does it mean that sometime earlier a pan gesture was detected? The explanation creates far more questions than it answers.
There was a problem hiding this comment.
I simplified the comment. Do you think it's clear enough?
|
@angelosilvestre This looks good. I am not experiencing the issue on Android anymore. |
| @@ -304,6 +304,12 @@ class _ReadOnlyAndroidDocumentTouchInteractorState extends State<ReadOnlyAndroid | |||
|
|
|||
| if (isScrolling) { | |||
| _isScrolling = true; | |||
|
|
|||
| // The long-press timer is cancelled if a pan gesture is detected. | |||
There was a problem hiding this comment.
I'm gonna merge this because there's a critical long-press ticket from Superlist, but this comment is still ordered in a way that makes it unnecessarily difficult to understand what it's trying to say.
Generally, the first thing you should say in a comment is what the code is actually doing. Beginning with contextual qualifiers is confusing because you haven't event established what you're qualifying yet.
Here's a comment in reversed order:
// Cancel the long-press timer because we're scrolling, and scrolling is mutually
// independent from long-pressing.
//
// We also cancel the timer when a pan gesture starts. However, we won't receive
// a pan gesture event if we have an ancestor scrollable. Therefore, we cancel the
// timer here, too.
|
I cherry-picked this to unblock some other work: #1553 |
[SuperEditor][SuperReader][Android] Fix long-press firing after scrolling and releasing the pointer. Resolves #1535
When the editor or reader is inside a CustomScrollview, trying to drag is opening the software keyboard and changing the selection. This was partially solved in #1536. However one issue remained:
Starting a scroll gestures and the releasing the pointer is still causing the issue. The cause is that, when the timer fires, we aren't scrolling anymore.
To fix that, this PR cancels the long-press timer as soon as the user starts the scrolling gesture.
I kept the existing test, which simulates the user dragging and holding down the pointer, and added another one simulating the user starting a drag gesture and then releasing the pointer.