Unverified Commit bc49cd1b authored by Ian Hickson's avatar Ian Hickson Committed by GitHub

Allow long-press gestures to continue even if buttons change. (#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.
parent 55b6f049
...@@ -2,7 +2,6 @@ ...@@ -2,7 +2,6 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
import 'package:flutter/foundation.dart'; import 'package:flutter/foundation.dart';
// Any changes to this file should be reflected in the debugAssertAllGesturesVarsUnset() // Any changes to this file should be reflected in the debugAssertAllGesturesVarsUnset()
......
...@@ -645,7 +645,7 @@ class LongPressGestureRecognizer extends PrimaryPointerGestureRecognizer { ...@@ -645,7 +645,7 @@ class LongPressGestureRecognizer extends PrimaryPointerGestureRecognizer {
_initialButtons = event.buttons; _initialButtons = event.buttons;
_checkLongPressDown(event); _checkLongPressDown(event);
} else if (event is PointerMoveEvent) { } else if (event is PointerMoveEvent) {
if (event.buttons != _initialButtons) { if (event.buttons != _initialButtons && !_longPressAccepted) {
resolve(GestureDisposition.rejected); resolve(GestureDisposition.rejected);
stopTrackingPointer(primaryPointer!); stopTrackingPointer(primaryPointer!);
} else if (_longPressAccepted) { } else if (_longPressAccepted) {
......
...@@ -458,7 +458,7 @@ void main() { ...@@ -458,7 +458,7 @@ void main() {
expect(recognized, <String>['end']); expect(recognized, <String>['end']);
}); });
testGesture('Should cancel long press when buttons change after acceptance', (GestureTester tester) { testGesture('Should not cancel long press when buttons change after acceptance', (GestureTester tester) {
// First press // First press
gesture.addPointer(down); gesture.addPointer(down);
tester.closeArena(down.pointer); tester.closeArena(down.pointer);
...@@ -470,7 +470,7 @@ void main() { ...@@ -470,7 +470,7 @@ void main() {
tester.route(moveR); tester.route(moveR);
expect(recognized, <String>[]); expect(recognized, <String>[]);
tester.route(up); tester.route(up);
expect(recognized, <String>[]); expect(recognized, <String>['end']);
}); });
testGesture('Buttons change after acceptance should not prevent the next long press', (GestureTester tester) { testGesture('Buttons change after acceptance should not prevent the next long press', (GestureTester tester) {
...@@ -701,4 +701,95 @@ void main() { ...@@ -701,4 +701,95 @@ void main() {
longPress.dispose(); longPress.dispose();
recognized.clear(); recognized.clear();
}); });
testGesture('Switching buttons mid-stream does not fail to send "end" event', (GestureTester tester) {
final List<String> recognized = <String>[];
final LongPressGestureRecognizer longPress = LongPressGestureRecognizer()
..onLongPressStart = (LongPressStartDetails details) {
recognized.add('primaryStart');
}
..onLongPressEnd = (LongPressEndDetails details) {
recognized.add('primaryEnd');
};
const PointerDownEvent down4 = PointerDownEvent(
pointer: 8,
position: Offset(10, 10),
);
const PointerMoveEvent move4 = PointerMoveEvent(
pointer: 8,
position: Offset(100, 200),
buttons: kPrimaryButton | kSecondaryButton,
);
const PointerUpEvent up4 = PointerUpEvent(
pointer: 8,
position: Offset(100, 200),
buttons: kSecondaryButton,
);
longPress.addPointer(down4);
tester.closeArena(4);
tester.route(down4);
tester.async.elapse(const Duration(milliseconds: 1000));
recognized.add('two seconds later...');
tester.route(move4);
tester.async.elapse(const Duration(milliseconds: 1000));
recognized.add('two more seconds later...');
tester.route(up4);
tester.async.elapse(const Duration(milliseconds: 1000));
expect(recognized, <String>['primaryStart', 'two seconds later...', 'two more seconds later...', 'primaryEnd']);
longPress.dispose();
});
testGesture('Switching buttons mid-stream does not fail to send "end" event (alternative sequence)', (GestureTester tester) {
// This reproduces sequences seen on macOS.
final List<String> recognized = <String>[];
final LongPressGestureRecognizer longPress = LongPressGestureRecognizer()
..onLongPressStart = (LongPressStartDetails details) {
recognized.add('primaryStart');
}
..onLongPressEnd = (LongPressEndDetails details) {
recognized.add('primaryEnd');
};
const PointerDownEvent down5 = PointerDownEvent(
pointer: 9,
position: Offset(10, 10),
);
const PointerMoveEvent move5a = PointerMoveEvent(
pointer: 9,
position: Offset(100, 200),
buttons: 3, // add 2
);
const PointerMoveEvent move5b = PointerMoveEvent(
pointer: 9,
position: Offset(100, 200),
buttons: 2, // remove 1
);
const PointerUpEvent up5 = PointerUpEvent(
pointer: 9,
position: Offset(100, 200),
);
longPress.addPointer(down5);
tester.closeArena(4);
tester.route(down5);
tester.async.elapse(const Duration(milliseconds: 1000));
recognized.add('two seconds later...');
tester.route(move5a);
tester.async.elapse(const Duration(milliseconds: 1000));
recognized.add('two more seconds later...');
tester.route(move5b);
tester.async.elapse(const Duration(milliseconds: 1000));
recognized.add('two more seconds later still...');
tester.route(up5);
tester.async.elapse(const Duration(milliseconds: 1000));
expect(recognized, <String>['primaryStart', 'two seconds later...', 'two more seconds later...', 'two more seconds later still...', 'primaryEnd']);
longPress.dispose();
});
} }
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment