Skip to content

Commit bc49cd1

Browse files
authored
Allow long-press gestures to continue even if buttons change. (flutter#127877)
Previously, if you changed buttons during a long-press gesture, if it was before the gesture was accepted we would discard it, and if it was after the gesture was accepted we would silently end it without firing any of the relevant events. This silent cancelation behavior is terrible because it means there's no way for consumers to know what state they're in, so you end up with widgets that thing they're still being long-pressed even though nothing is happening. We could change the behavior in three ways, as far as I can tell: - we could send a cancel event when you change buttons. This would introduce a new kind of transition (start->cancel) which I don't think we currently require people to support. This would therefore not fix existing code and would make future code more complicated to handle a really obscure user action that it seems unlikely anyone cares about. - we could send an end event when you change buttons. This would mean the action commits, even though the user is still holding the mouse button down. This seems slightly better than the previous option but still not ideal as it means nudging the mouse button commits you even though you're still holding the button down. - we could ignore button changes after the long-press has been accepted. I implemented the last one in this PR.
1 parent 55b6f04 commit bc49cd1

File tree

3 files changed

+94
-4
lines changed

3 files changed

+94
-4
lines changed

packages/flutter/lib/src/gestures/debug.dart

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44

5-
65
import 'package:flutter/foundation.dart';
76

87
// Any changes to this file should be reflected in the debugAssertAllGesturesVarsUnset()

packages/flutter/lib/src/gestures/long_press.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -645,7 +645,7 @@ class LongPressGestureRecognizer extends PrimaryPointerGestureRecognizer {
645645
_initialButtons = event.buttons;
646646
_checkLongPressDown(event);
647647
} else if (event is PointerMoveEvent) {
648-
if (event.buttons != _initialButtons) {
648+
if (event.buttons != _initialButtons && !_longPressAccepted) {
649649
resolve(GestureDisposition.rejected);
650650
stopTrackingPointer(primaryPointer!);
651651
} else if (_longPressAccepted) {

packages/flutter/test/gestures/long_press_test.dart

Lines changed: 93 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -458,7 +458,7 @@ void main() {
458458
expect(recognized, <String>['end']);
459459
});
460460

461-
testGesture('Should cancel long press when buttons change after acceptance', (GestureTester tester) {
461+
testGesture('Should not cancel long press when buttons change after acceptance', (GestureTester tester) {
462462
// First press
463463
gesture.addPointer(down);
464464
tester.closeArena(down.pointer);
@@ -470,7 +470,7 @@ void main() {
470470
tester.route(moveR);
471471
expect(recognized, <String>[]);
472472
tester.route(up);
473-
expect(recognized, <String>[]);
473+
expect(recognized, <String>['end']);
474474
});
475475

476476
testGesture('Buttons change after acceptance should not prevent the next long press', (GestureTester tester) {
@@ -701,4 +701,95 @@ void main() {
701701
longPress.dispose();
702702
recognized.clear();
703703
});
704+
705+
testGesture('Switching buttons mid-stream does not fail to send "end" event', (GestureTester tester) {
706+
final List<String> recognized = <String>[];
707+
final LongPressGestureRecognizer longPress = LongPressGestureRecognizer()
708+
..onLongPressStart = (LongPressStartDetails details) {
709+
recognized.add('primaryStart');
710+
}
711+
..onLongPressEnd = (LongPressEndDetails details) {
712+
recognized.add('primaryEnd');
713+
};
714+
715+
const PointerDownEvent down4 = PointerDownEvent(
716+
pointer: 8,
717+
position: Offset(10, 10),
718+
);
719+
720+
const PointerMoveEvent move4 = PointerMoveEvent(
721+
pointer: 8,
722+
position: Offset(100, 200),
723+
buttons: kPrimaryButton | kSecondaryButton,
724+
);
725+
726+
const PointerUpEvent up4 = PointerUpEvent(
727+
pointer: 8,
728+
position: Offset(100, 200),
729+
buttons: kSecondaryButton,
730+
);
731+
732+
longPress.addPointer(down4);
733+
tester.closeArena(4);
734+
tester.route(down4);
735+
tester.async.elapse(const Duration(milliseconds: 1000));
736+
recognized.add('two seconds later...');
737+
tester.route(move4);
738+
tester.async.elapse(const Duration(milliseconds: 1000));
739+
recognized.add('two more seconds later...');
740+
tester.route(up4);
741+
tester.async.elapse(const Duration(milliseconds: 1000));
742+
expect(recognized, <String>['primaryStart', 'two seconds later...', 'two more seconds later...', 'primaryEnd']);
743+
longPress.dispose();
744+
});
745+
746+
testGesture('Switching buttons mid-stream does not fail to send "end" event (alternative sequence)', (GestureTester tester) {
747+
// This reproduces sequences seen on macOS.
748+
final List<String> recognized = <String>[];
749+
final LongPressGestureRecognizer longPress = LongPressGestureRecognizer()
750+
..onLongPressStart = (LongPressStartDetails details) {
751+
recognized.add('primaryStart');
752+
}
753+
..onLongPressEnd = (LongPressEndDetails details) {
754+
recognized.add('primaryEnd');
755+
};
756+
757+
const PointerDownEvent down5 = PointerDownEvent(
758+
pointer: 9,
759+
position: Offset(10, 10),
760+
);
761+
762+
const PointerMoveEvent move5a = PointerMoveEvent(
763+
pointer: 9,
764+
position: Offset(100, 200),
765+
buttons: 3, // add 2
766+
);
767+
768+
const PointerMoveEvent move5b = PointerMoveEvent(
769+
pointer: 9,
770+
position: Offset(100, 200),
771+
buttons: 2, // remove 1
772+
);
773+
774+
const PointerUpEvent up5 = PointerUpEvent(
775+
pointer: 9,
776+
position: Offset(100, 200),
777+
);
778+
779+
longPress.addPointer(down5);
780+
tester.closeArena(4);
781+
tester.route(down5);
782+
tester.async.elapse(const Duration(milliseconds: 1000));
783+
recognized.add('two seconds later...');
784+
tester.route(move5a);
785+
tester.async.elapse(const Duration(milliseconds: 1000));
786+
recognized.add('two more seconds later...');
787+
tester.route(move5b);
788+
tester.async.elapse(const Duration(milliseconds: 1000));
789+
recognized.add('two more seconds later still...');
790+
tester.route(up5);
791+
tester.async.elapse(const Duration(milliseconds: 1000));
792+
expect(recognized, <String>['primaryStart', 'two seconds later...', 'two more seconds later...', 'two more seconds later still...', 'primaryEnd']);
793+
longPress.dispose();
794+
});
704795
}

0 commit comments

Comments
 (0)