Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cherry Pick: [SuperEditor][SuperReader][Android] Fix long-press firing after scrolling and releasing the pointer (Resolves #1535) (#1540) #1553

Merged
merged 1 commit into from
Oct 24, 2023
Merged

Cherry Pick: [SuperEditor][SuperReader][Android] Fix long-press firing after scrolling and releasing the pointer (Resolves #1535) (#1540) #1553

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 5 additions & 7 deletions super_editor/lib/src/default_editor/document_gestures_touch_android.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
6 changes: 6 additions & 0 deletions super_editor/lib/src/super_reader/read_only_document_android_touch_interactor.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
61 changes: 60 additions & 1 deletion super_editor/test/super_editor/supereditor_scrolling_test.dart
View file
Open in desktop
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
View file
Open in desktop
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