Skip to content

Commit

Permalink
[SuperEditor][SuperReader][Android] Fix long-press firing after scrol…
Browse files Browse the repository at this point in the history
…ling and releasing the pointer (Resolves #1535) (#1540)
  • Loading branch information
angelosilvestre authored Oct 24, 2023
1 parent 6ed5bd5 commit 813f404
Show file tree
Hide file tree
Showing 4 changed files with 130 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,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
Expand Down Expand Up @@ -497,13 +502,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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
// 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
Expand Down
61 changes: 60 additions & 1 deletion super_editor/test/super_editor/supereditor_scrolling_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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));
Expand Down
60 changes: 59 additions & 1 deletion super_editor/test/super_reader/super_reader_scrolling_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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));
Expand Down

0 comments on commit 813f404

Please sign in to comment.