Skip to content

Commit

Permalink
[SuperEditor] Prevent long-press to resolve when swiping to pop (Reso…
Browse files Browse the repository at this point in the history
…lves #1947) (#2064)
  • Loading branch information
angelosilvestre committed Jun 7, 2024
1 parent a212a35 commit 8b2255f
Show file tree
Hide file tree
Showing 7 changed files with 260 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -710,6 +710,11 @@ class _AndroidDocumentTouchInteractorState extends State<AndroidDocumentTouchInt
}
}

void _onTapCancel() {
_tapDownLongPressTimer?.cancel();
_tapDownLongPressTimer = null;
}

// Runs when a tap down has lasted long enough to signify a long-press.
void _onLongPressDown() {
_longPressStrategy = AndroidDocumentLongPressSelectionStrategy(
Expand Down Expand Up @@ -1251,6 +1256,7 @@ class _AndroidDocumentTouchInteractorState extends State<AndroidDocumentTouchInt
(TapSequenceGestureRecognizer recognizer) {
recognizer
..onTapDown = _onTapDown
..onTapCancel = _onTapCancel
..onTapUp = _onTapUp
..onDoubleTapDown = _onDoubleTapDown
..onTripleTapDown = _onTripleTapDown
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -586,6 +586,11 @@ class _IosDocumentTouchInteractorState extends State<IosDocumentTouchInteractor>
widget.focusNode.requestFocus();
}

void _onTapCancel() {
_tapDownLongPressTimer?.cancel();
_tapDownLongPressTimer = null;
}

void _onTapUp(TapUpDetails details) {
// Stop waiting for a long-press to start.
_globalTapDownOffset = null;
Expand Down Expand Up @@ -1295,6 +1300,7 @@ class _IosDocumentTouchInteractorState extends State<IosDocumentTouchInteractor>
(TapSequenceGestureRecognizer recognizer) {
recognizer
..onTapDown = _onTapDown
..onTapCancel = _onTapCancel
..onTapUp = _onTapUp
..onDoubleTapUp = _onDoubleTapUp
..onTripleTapUp = _onTripleTapUp
Expand Down
68 changes: 53 additions & 15 deletions super_editor/lib/src/infrastructure/multi_tap_gesture.dart
Original file line number Diff line number Diff line change
Expand Up @@ -198,14 +198,39 @@ class TapSequenceGestureRecognizer extends GestureRecognizer {
_trackers.remove(tracker.pointer);
tracker.entry.resolve(GestureDisposition.rejected);
_freezeTracker(tracker);

if (_firstTap == null && _firstTapDownDetails != null) {
// The user tapped down and then another recognizer won the arena. For example, in an app with both a
// TapSequenceGestureRecognizer and a HorizontalDragGestureRecognizer, when the user taps down and
// then drags horizontally, the onTapDown event is fired, and after that the HorizontalDragGestureRecognizer
// declares itself as the winner. Invoke onTapCancel to cancel the gesture.
_notifyListenersOfCancellation();
if (_trackers.isEmpty) {
_reset();
}
return;
}

if (tracker == _secondTap) {
// A double tap was registered and we were defeated on the gesture arena after that. Reset
// to clean up the tap trackers.
_reset();
return;
}

if (tracker == _firstTap) {
// A tap up was registered and we were defeated on the gesture arena after that. Reset
// to clean up the tap trackers.
_reset();
return;
}

if (_firstTap != null || _secondTap != null) {
if (tracker == _firstTap || tracker == _secondTap) {
// We have a single or double tap registered, but the tracker isn't related to any of them.
// It's not clear what this situation means.
_notifyListenersOfCancellation();
if (_trackers.isEmpty) {
_reset();
} else {
_checkCancel();
if (_trackers.isEmpty) {
_reset();
}
}
}
}
Expand Down Expand Up @@ -254,7 +279,7 @@ class TapSequenceGestureRecognizer extends GestureRecognizer {
_stopTapTimer();
if (_secondTap != null) {
if (_trackers.isNotEmpty) {
_checkCancel();
_notifyListenersOfCancellation();
}
// Note, order is important below in order for the resolve -> reject logic
// to work properly.
Expand All @@ -265,7 +290,7 @@ class TapSequenceGestureRecognizer extends GestureRecognizer {
}
if (_firstTap != null) {
if (_trackers.isNotEmpty) {
_checkCancel();
_notifyListenersOfCancellation();
}
// Note, order is important below in order for the resolve -> reject logic
// to work properly.
Expand All @@ -275,6 +300,8 @@ class TapSequenceGestureRecognizer extends GestureRecognizer {
GestureBinding.instance.gestureArena.release(tracker.pointer);
}
_clearTrackers();

_firstTapDownDetails = null;
}

void _registerFirstTap(PointerEvent event, _TapTracker tracker) {
Expand Down Expand Up @@ -371,15 +398,26 @@ class TapSequenceGestureRecognizer extends GestureRecognizer {
}
}

void _checkCancel() {
if (_firstTap == null && onTapCancel != null) {
invokeCallback<void>('onTapCancel', onTapCancel!);
void _notifyListenersOfCancellation() {
if (_secondTap != null) {
if (onTripleTapCancel != null) {
invokeCallback<void>('onTripleTapCancel', onTripleTapCancel!);
}
return;
}
if (_firstTap != null && _secondTap == null && onDoubleTapCancel != null) {
invokeCallback<void>('onDoubleTapCancel', onDoubleTapCancel!);

if (_firstTap != null) {
if (onDoubleTapCancel != null) {
invokeCallback<void>('onDoubleTapCancel', onDoubleTapCancel!);
}
return;
}
if (_secondTap != null && onTripleTapCancel != null) {
invokeCallback<void>('onTripleTapCancel', onTripleTapCancel!);

if (_firstTapDownDetails != null) {
if (onTapCancel != null) {
invokeCallback<void>('onTapCancel', onTapCancel!);
}
return;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,11 @@ class _ReadOnlyAndroidDocumentTouchInteractorState extends State<ReadOnlyAndroid
_tapDownLongPressTimer = Timer(kLongPressTimeout, _onLongPressDown);
}

void _onTapCancel() {
_tapDownLongPressTimer?.cancel();
_tapDownLongPressTimer = null;
}

// Runs when a tap down has lasted long enough to signify a long-press.
void _onLongPressDown() {
if (_isScrolling) {
Expand Down Expand Up @@ -1054,6 +1059,7 @@ class _ReadOnlyAndroidDocumentTouchInteractorState extends State<ReadOnlyAndroid
(TapSequenceGestureRecognizer recognizer) {
recognizer
..onTapDown = _onTapDown
..onTapCancel = _onTapCancel
..onTapUp = _onTapUp
..onDoubleTapDown = _onDoubleTapDown
..onTripleTapDown = _onTripleTapDown
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,11 @@ class _SuperReaderIosDocumentTouchInteractorState extends State<SuperReaderIosDo
_tapDownLongPressTimer = Timer(kLongPressTimeout, _onLongPressDown);
}

void _onTapCancel() {
_tapDownLongPressTimer?.cancel();
_tapDownLongPressTimer = null;
}

// Runs when a tap down has lasted long enough to signify a long-press.
void _onLongPressDown() {
final interactorOffset = interactorBox.globalToLocal(_globalTapDownOffset!);
Expand Down Expand Up @@ -970,6 +975,7 @@ class _SuperReaderIosDocumentTouchInteractorState extends State<SuperReaderIosDo
(TapSequenceGestureRecognizer recognizer) {
recognizer
..onTapDown = _onTapDown
..onTapCancel = _onTapCancel
..onTapUp = _onTapUp
..onDoubleTapUp = _onDoubleTapUp
..onTripleTapUp = _onTripleTapUp
Expand Down
87 changes: 75 additions & 12 deletions super_editor/test/infrastructure/multi_tap_gesture_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,7 @@ void main() {
),
);

TestGesture gesture =
await tester.startGesture(tester.getCenter(tapTargetFinder));
TestGesture gesture = await tester.startGesture(tester.getCenter(tapTargetFinder));
await tester.pump();
expect(tapDownCount, 1);
expect(tapCount, 0);
Expand Down Expand Up @@ -138,8 +137,7 @@ void main() {
}),
);

TestGesture gesture =
await tester.startGesture(tester.getCenter(tapTargetFinder));
TestGesture gesture = await tester.startGesture(tester.getCenter(tapTargetFinder));
await tester.pump();
expect(tapDownCount, 0);
expect(tapCount, 0);
Expand Down Expand Up @@ -215,8 +213,7 @@ void main() {
),
);

TestGesture gesture =
await tester.startGesture(tester.getCenter(tapTargetFinder));
TestGesture gesture = await tester.startGesture(tester.getCenter(tapTargetFinder));
await tester.pump();
expect(tapDownCount, 0);
expect(tapCount, 0);
Expand Down Expand Up @@ -269,8 +266,7 @@ void main() {
expect(timeoutCount, 1);
});

testWidgets("can ignore single tap and double tap gestures",
(tester) async {
testWidgets("can ignore single tap and double tap gestures", (tester) async {
final recognizer = TapSequenceGestureRecognizer(
supportedDevices: {PointerDeviceKind.touch},
reportPrecedingGestures: false,
Expand Down Expand Up @@ -311,8 +307,7 @@ void main() {
),
);

TestGesture gesture =
await tester.startGesture(tester.getCenter(tapTargetFinder));
TestGesture gesture = await tester.startGesture(tester.getCenter(tapTargetFinder));
await tester.pump();
expect(tapDownCount, 0);
expect(tapCount, 0);
Expand Down Expand Up @@ -374,6 +369,75 @@ void main() {

await tester.pumpAndSettle();
});

testWidgets("cancels tap if another recognizer wins after tap down", (tester) async {
int tapDownCount = 0;
int tapCancelCount = 0;
int tapUpCount = 0;
int dragUpdateCount = 0;

// Pump a tree with a tap recognizer and a drag recognizer to check if dragging
// after onTapDown was called causes the tap to be cancelled.
await tester.pumpWidget(
MaterialApp(
home: Center(
child: RawGestureDetector(
gestures: {
HorizontalDragGestureRecognizer: GestureRecognizerFactoryWithHandlers<HorizontalDragGestureRecognizer>(
() => HorizontalDragGestureRecognizer(),
(HorizontalDragGestureRecognizer recognizer) {
recognizer.onUpdate = (_) {
dragUpdateCount += 1;
};
},
),
TapSequenceGestureRecognizer: GestureRecognizerFactoryWithHandlers<TapSequenceGestureRecognizer>(
() => TapSequenceGestureRecognizer(),
(TapSequenceGestureRecognizer recognizer) {
recognizer
..onTapDown = (_) {
tapDownCount += 1;
}
..onTapUp = (_) {
tapUpCount += 1;
}
..onTapCancel = () {
tapCancelCount += 1;
};
},
),
},
child: Container(
key: const ValueKey('tap-target'),
width: 48,
height: 48,
color: Colors.red,
),
),
),
),
);

// Start the gesture, this should fire onTapDown.
final gesture = await tester.startGesture(tester.getCenter(tapTargetFinder));
await tester.pump(kTapMinTime);

// Trigger a horizontal drag.
await gesture.moveBy(Offset(20, 0));
await tester.pump();
await gesture.moveBy(Offset(20, 0));
await tester.pump();

// Release the gesture.
await gesture.up();
await tester.pump();

// Ensure that onTapCancel was called and onTapUp was not.
expect(tapDownCount, 1);
expect(tapCancelCount, 1);
expect(tapUpCount, 0);
expect(dragUpdateCount, 1);
});
});
}

Expand All @@ -391,8 +455,7 @@ Widget _buildGestureScaffold(
home: Center(
child: RawGestureDetector(
gestures: {
TapSequenceGestureRecognizer: GestureRecognizerFactoryWithHandlers<
TapSequenceGestureRecognizer>(
TapSequenceGestureRecognizer: GestureRecognizerFactoryWithHandlers<TapSequenceGestureRecognizer>(
() => recognizer,
(TapSequenceGestureRecognizer recognizer) {
recognizer
Expand Down
Loading

0 comments on commit 8b2255f

Please sign in to comment.