[SuperEditor][SuperReader][Android] Fix scrolling with an ancestor Sc…

…rollable (Resolves #1535) (#1536)
superlistapp · Oct 20, 2023 · d10f528 · d10f528
1 parent 83bcc6f
commit d10f528
Show file tree

Hide file tree

Showing 4 changed files with 123 additions and 0 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
@@ -499,6 +499,13 @@ 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
@@ -482,6 +482,13 @@ class _ReadOnlyAndroidDocumentTouchInteractorState extends State<ReadOnlyAndroid
 
   // Runs when a tap down has lasted long enough to signify a long-press.
   void _onLongPressDown() {
+    if (_isScrolling) {
+      // When the reader 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/test/super_editor/supereditor_scrolling_test.dart b/super_editor/test/super_editor/supereditor_scrolling_test.dart
@@ -581,6 +581,60 @@ void main() {
         expect(caretOffset.dy, greaterThanOrEqualTo(screenSizeWithKeyboard.height - trailingBoundary));
       });
 
+      testWidgetsOnMobile('scrolling 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();
+        }
+
+        // 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));
+        await dragGesture.up();
+        await dragGesture.removePointer();
+
+        // 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);
+      });
+
       group('respects horizontal scrolling', () {
         testWidgetsOnAllPlatforms('inside a TabBar', (tester) async {
           final tabController = TabController(length: 2, vsync: tester);

diff --git a/super_editor/test/super_reader/super_reader_scrolling_test.dart b/super_editor/test/super_reader/super_reader_scrolling_test.dart
@@ -312,6 +312,61 @@ void main() {
         variant: _scrollDirectionVariant,
       );
     });
+
+    group("with ancestor scrollable", () {
+      testWidgetsOnMobile('scrolling doesn\'t change selection', (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();
+        }
+
+        // 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));
+        await dragGesture.up();
+        await dragGesture.removePointer();
+
+        // Ensure we scrolled and didn't change the selection.
+        expect(scrollController.offset, greaterThan(0));
+        expect(SuperReaderInspector.findDocumentSelection(), isNull);
+      });
+    });
   });
 }