From 1dcce4330ae4f22abffc83f066de1121cdb0cfff Mon Sep 17 00:00:00 2001
From: Angelo Silvestre <angelosilvestre.ccp@gmail.com>
Date: Tue, 24 Oct 2023 15:18:37 -0300
Subject: [PATCH] [SuperEditor][SuperReader][Android] Fix long-press firing
 after scrolling and releasing the pointer (Resolves #1535) (#1540)

---
 .../document_gestures_touch_android.dart      | 12 ++--
 ...nly_document_android_touch_interactor.dart |  6 ++
 .../supereditor_scrolling_test.dart           | 61 ++++++++++++++++++-
 .../super_reader_scrolling_test.dart          | 60 +++++++++++++++++-
 4 files changed, 130 insertions(+), 9 deletions(-)

diff --git a/super_editor/lib/src/default_editor/document_gestures_touch_android.dart b/super_editor/lib/src/default_editor/document_gestures_touch_android.dart
index ae9deb92ef..d78f33f3ac 100644
--- a/super_editor/lib/src/default_editor/document_gestures_touch_android.dart
+++ b/super_editor/lib/src/default_editor/document_gestures_touch_android.dart
@@ -326,6 +326,11 @@ class _AndroidDocumentTouchInteractorState extends State<AndroidDocumentTouchInt
 
     if (isScrolling) {
       _isScrolling = true;
+
+      // The user started to scroll.
+      // Cancel the timer to stop trying to detect a long press.
+      _tapDownLongPressTimer?.cancel();
+      _tapDownLongPressTimer = null;
     } else {
       onNextFrame((_) {
         // Set our scrolling flag to false on the next frame, so that our tap handlers
@@ -499,13 +504,6 @@ class _AndroidDocumentTouchInteractorState extends State<AndroidDocumentTouchInt
 
   // Runs when a tap down has lasted long enough to signify a long-press.
   void _onLongPressDown() {
-    if (_isScrolling) {
-      // When the editor has an ancestor scrollable, dragging won't trigger a pan gesture
-      // is this widget. Because of that, the timer still fires after the timeout.
-      // Do nothing to let the user scroll.
-      return;
-    }
-
     _longPressStrategy = AndroidDocumentLongPressSelectionStrategy(
       document: widget.document,
       documentLayout: _docLayout,
diff --git a/super_editor/lib/src/super_reader/read_only_document_android_touch_interactor.dart b/super_editor/lib/src/super_reader/read_only_document_android_touch_interactor.dart
index 9bb4ff68cb..423d6079ad 100644
--- a/super_editor/lib/src/super_reader/read_only_document_android_touch_interactor.dart
+++ b/super_editor/lib/src/super_reader/read_only_document_android_touch_interactor.dart
@@ -306,6 +306,12 @@ class _ReadOnlyAndroidDocumentTouchInteractorState extends State<ReadOnlyAndroid
 
     if (isScrolling) {
       _isScrolling = true;
+
+      // The long-press timer is cancelled if a pan gesture is detected.
+      // However, if we have an ancestor scrollable, we won't receive a pan gesture in this widget.
+      // Cancel the timer as soon as the user started scrolling.
+      _tapDownLongPressTimer?.cancel();
+      _tapDownLongPressTimer = null;
     } else {
       onNextFrame((_) {
         // Set our scrolling flag to false on the next frame, so that our tap handlers
diff --git a/super_editor/test/super_editor/supereditor_scrolling_test.dart b/super_editor/test/super_editor/supereditor_scrolling_test.dart
index 8a527dca19..f794675db5 100644
--- a/super_editor/test/super_editor/supereditor_scrolling_test.dart
+++ b/super_editor/test/super_editor/supereditor_scrolling_test.dart
@@ -581,7 +581,7 @@ void main() {
         expect(caretOffset.dy, greaterThanOrEqualTo(screenSizeWithKeyboard.height - trailingBoundary - 2));
       });
 
-      testWidgetsOnMobile('scrolling doesn\'t cause the keyboard to open', (tester) async {
+      testWidgetsOnMobile('scrolling and holding the pointer doesn\'t cause the keyboard to open', (tester) async {
         final scrollController = ScrollController();
 
         // Pump an editor inside a CustomScrollView without enough room to display
@@ -626,8 +626,67 @@ void main() {
         // The editor supports long press to select.
         // Wait long enough to make sure  this gesture wasn't confused with a long press.
         await tester.pump(kLongPressTimeout + const Duration(milliseconds: 1));
+
+        // Ensure we scrolled, didn't changed the selection and didn't attach to the IME.
+        expect(scrollController.offset, greaterThan(0));
+        expect(SuperEditorInspector.findDocumentSelection(), isNull);
+        expect(tester.testTextInput.hasAnyClients, isFalse);
+
+        // Release the pointer.
+        await dragGesture.up();
+        await dragGesture.removePointer();
+      });
+
+      testWidgetsOnMobile('scrolling and releasing the pointer doesn\'t cause the keyboard to open', (tester) async {
+        final scrollController = ScrollController();
+
+        // Pump an editor inside a CustomScrollView without enough room to display
+        // the whole content.
+        await tester
+            .createDocument() //
+            .withLongTextContent()
+            .withCustomWidgetTreeBuilder(
+              (superEditor) => MaterialApp(
+                home: Scaffold(
+                  body: ConstrainedBox(
+                    constraints: const BoxConstraints(maxHeight: 200),
+                    child: CustomScrollView(
+                      controller: scrollController,
+                      slivers: [
+                        SliverToBoxAdapter(
+                          child: superEditor,
+                        ),
+                      ],
+                    ),
+                  ),
+                ),
+              ),
+            )
+            .pump();
+
+        // Ensure the scrollview didn't start scrolled.
+        expect(scrollController.offset, 0);
+
+        final scrollableRect = tester.getRect(find.byType(CustomScrollView));
+
+        const dragFrameCount = 10;
+        final dragAmountPerFrame = scrollableRect.height / dragFrameCount;
+
+        // Drag from the bottom all the way up to the top of the scrollable.
+        final dragGesture = await tester.startGesture(scrollableRect.bottomCenter - const Offset(0, 1));
+        for (int i = 0; i < dragFrameCount; i += 1) {
+          await dragGesture.moveBy(Offset(0, -dragAmountPerFrame));
+          await tester.pump();
+        }
+
+        // Stop the scrolling gesture.
         await dragGesture.up();
         await dragGesture.removePointer();
+        await tester.pump();
+
+        // The editor supports long press to select.
+        // Wait long enough to make sure  this gesture wasn't confused with a long press.
+        await tester.pump(kLongPressTimeout + const Duration(milliseconds: 1));
 
         // Ensure we scrolled, didn't changed the selection and didn't attach to the IME.
         expect(scrollController.offset, greaterThan(0));
diff --git a/super_editor/test/super_reader/super_reader_scrolling_test.dart b/super_editor/test/super_reader/super_reader_scrolling_test.dart
index 1241fb47a9..ad40a74b43 100644
--- a/super_editor/test/super_reader/super_reader_scrolling_test.dart
+++ b/super_editor/test/super_reader/super_reader_scrolling_test.dart
@@ -314,7 +314,7 @@ void main() {
     });
 
     group("with ancestor scrollable", () {
-      testWidgetsOnMobile('scrolling doesn\'t change selection', (tester) async {
+      testWidgetsOnMobile('scrolling and holding the pointer doesn\'t change selection', (tester) async {
         final scrollController = ScrollController();
 
         // Pump a reader inside a CustomScrollView without enough room to display
@@ -359,8 +359,66 @@ void main() {
         // The reader supports long press to select.
         // Wait long enough to make sure  this gesture wasn't confused with a long press.
         await tester.pump(kLongPressTimeout + const Duration(milliseconds: 1));
+
+        // Ensure we scrolled and didn't change the selection.
+        expect(scrollController.offset, greaterThan(0));
+        expect(SuperReaderInspector.findDocumentSelection(), isNull);
+
         await dragGesture.up();
         await dragGesture.removePointer();
+      });
+
+      testWidgetsOnMobile('scrolling and releasing the pointer doesn\'t change selection after gesture ended',
+          (tester) async {
+        final scrollController = ScrollController();
+
+        // Pump a reader inside a CustomScrollView without enough room to display
+        // the whole content.
+        await tester
+            .createDocument() //
+            .withLongTextContent()
+            .withCustomWidgetTreeBuilder(
+              (superReader) => MaterialApp(
+                home: Scaffold(
+                  body: ConstrainedBox(
+                    constraints: const BoxConstraints(maxHeight: 200),
+                    child: CustomScrollView(
+                      controller: scrollController,
+                      slivers: [
+                        SliverToBoxAdapter(
+                          child: superReader,
+                        ),
+                      ],
+                    ),
+                  ),
+                ),
+              ),
+            )
+            .pump();
+
+        // Ensure the scrollview didn't start scrolled.
+        expect(scrollController.offset, 0);
+
+        final scrollableRect = tester.getRect(find.byType(CustomScrollView));
+
+        const dragFrameCount = 10;
+        final dragAmountPerFrame = scrollableRect.height / dragFrameCount;
+
+        // Drag from the bottom all the way up to the top of the scrollable.
+        final dragGesture = await tester.startGesture(scrollableRect.bottomCenter - const Offset(0, 1));
+        for (int i = 0; i < dragFrameCount; i += 1) {
+          await dragGesture.moveBy(Offset(0, -dragAmountPerFrame));
+          await tester.pump();
+        }
+
+        // Stop the scrolling gesture.
+        await dragGesture.up();
+        await dragGesture.removePointer();
+        await tester.pump();
+
+        // The reader supports long press to select.
+        // Wait long enough to make sure  this gesture wasn't confused with a long press.
+        await tester.pump(kLongPressTimeout + const Duration(milliseconds: 1));
 
         // Ensure we scrolled and didn't change the selection.
         expect(scrollController.offset, greaterThan(0));