Allow long-press gestures to continue even if buttons change. #127877

Hixie · 2023-05-30T20:43:12Z

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.

Renzo-Olivares

I agree this is a very obscure user interaction, option 1 did sound reasonable to me and it looks like we do something similar by handling down (accepted) -> cancel in TapGestureRecognizer.

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

Lines 257 to 268 in f92f441

    
           @override 
        
           void resolve(GestureDisposition disposition) { 
        
             if (_wonArenaForPrimaryPointer && disposition == GestureDisposition.rejected) { 
        
               // This can happen if the gesture has been canceled. For example, when 
        
               // the pointer has exceeded the touch slop, the buttons have been changed, 
        
               // or if the recognizer is disposed. 
        
               assert(_sentTapDown); 
        
               _checkCancel(null, 'spontaneous'); 
        
               _reset(); 
        
             } 
        
             super.resolve(disposition); 
        
           }

But I agree with the point that it wouldn't fix current users. I wonder if in the future we want to provide the buttons in the gesture details object so a user could potentially handle this use-case by checking if the initial down buttons are the same as the end buttons.

Renzo-Olivares · 2023-06-02T20:00:57Z

packages/flutter/test/gestures/long_press_test.dart

From testing on macOS, after this first move4, when releasing the primary button, another move event is dispatched with the buttons set as 2.

Renzo-Olivares · 2023-06-02T20:02:16Z

packages/flutter/test/gestures/long_press_test.dart

This might be outside of the scope of this PR, but when testing on macOS and web with a local example app the buttons of this final PointerUpEvent is actually 0. Is this expected?

Hixie · 2023-06-06T18:03:33Z

Can you elaborate on what you're seeing on macOS?

In general I would hope that we normalize our inputs so the same physical inputs turn into the same events at the Flutter level but that may be optimistic.

I'm happy to add tests for other sequences, to make sure they all make sense. Is that what you had in mind?

Renzo-Olivares · 2023-06-06T18:45:58Z

Using this example app with the change in this PR.

import 'package:flutter/material.dart';

void main() {
  runApp(const MainApp());
}

class MainApp extends StatelessWidget {
  const MainApp({super.key});

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      home: Scaffold(
        body: Center(
          child: GestureDetector(
            onLongPressStart: (LongPressStartDetails details) {
              debugPrint('start');
            },
            onLongPressMoveUpdate: (LongPressMoveUpdateDetails details) {
              debugPrint('move');
            },
            onLongPressEnd: (LongPressEndDetails details) {
              debugPrint('end');
            },
            child: Container(
              color: Colors.green,
              height: 200,
              width: 200,
              child: const Text('Hello World!'),
            ),
          ),
        ),
      ),
    );
  }
}

and adding some logging in PrimaryPointerGestureRecognizer.addAllowedPointer and PrimaryPointerGestureRecognizer.handleEvent.

For the following click sequence.

long press left click.
sometime after long press also press the right click.
release the left click.
release the right click.

This is what I see in the logs for macOS native/and macOS chrome.

flutter: PrimaryPointerGestureRecognizer addAllowedPointer down _TransformedPointerDownEvent#b28e4(position: Offset(378.2, 287.3))
flutter: PrimaryPointerGestureRecognizer handleEvent _TransformedPointerDownEvent#a57f1(position: Offset(378.2, 287.3)) buttons 1
flutter: start
flutter: PrimaryPointerGestureRecognizer handleEvent _TransformedPointerMoveEvent#06cd2(position: Offset(378.2, 287.3)) buttons 3
flutter: move
flutter: PrimaryPointerGestureRecognizer handleEvent _TransformedPointerMoveEvent#fd781(position: Offset(378.2, 287.3)) buttons 2
flutter: move
flutter: PrimaryPointerGestureRecognizer handleEvent _TransformedPointerUpEvent#fad41(position: Offset(378.2, 287.3)) buttons 0
flutter: end

I was more or less wondering if we should test that extra move event when the left click is released. And if the pointer up event ending with buttons: 0 is expected. Looking at your test it looks like it should be buttons: 2.

Hixie · 2023-06-14T22:07:32Z

I can add some tests that check that sequence and others, sure.

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.

Hixie · 2023-06-15T04:59:22Z

@Renzo-Olivares PTAL

Renzo-Olivares

LGTM

Renzo-Olivares · 2023-06-21T22:10:21Z

I think the broken google testing may have been unrelated since it was having some issues around the time when this PR was approved.

…flutter/flutter#127877)

flutter/flutter@35085c3...bc49cd1 2023-07-06 [email protected] Allow long-press gestures to continue even if buttons change. (flutter/flutter#127877) 2023-07-06 [email protected] Enable unreachable_from_main lint - it is stable now!!1 (flutter/flutter#129854) 2023-07-06 [email protected] Update labeler to new label names (flutter/flutter#130040) 2023-07-05 [email protected] MergeableMaterial: Fix adding a slice and separating it (flutter/flutter#128804) 2023-07-05 [email protected] Update infrastructure issue template for new priority scheme (flutter/flutter#129741) 2023-07-05 [email protected] Fix typo in canvas example (flutter/flutter#129879) 2023-07-05 [email protected] Reland Fix AnimatedList & AnimatedGrid doesn't apply MediaQuery padding #129556 (flutter/flutter#129860) 2023-07-05 [email protected] Change from "created via performance template" to "from: performance template" (flutter/flutter#130035) 2023-07-05 [email protected] Removes deprecated APIs from v2.6 in `binding.dart` and `widget_tester.dart` (flutter/flutter#129663) 2023-07-05 [email protected] Add new hot reload case string (flutter/flutter#130008) 2023-07-05 [email protected] Manual roll Flutter Engine from 987b621eac4e to bd2e42b203e1 (32 revisions) (flutter/flutter#130023) 2023-07-05 [email protected] Add simple unit tests for annotations.dart file (flutter/flutter#128902) 2023-07-05 [email protected] fix a bug when android uses CupertinoPageTransitionsBuilder... (flutter/flutter#114303) 2023-07-05 [email protected] Add .env file support for option `--dart-define-from-file` (flutter/flutter#128668) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…flutter/flutter#127877)

flutter-dashboard bot added f: gestures flutter/packages/flutter/gestures repository. framework flutter/packages/flutter repository. See also f: labels. labels May 30, 2023

github-actions bot removed f: gestures flutter/packages/flutter/gestures repository. framework flutter/packages/flutter repository. See also f: labels. labels May 30, 2023

goderbauer added framework flutter/packages/flutter repository. See also f: labels. f: gestures flutter/packages/flutter/gestures repository. labels Jun 1, 2023

Renzo-Olivares reviewed Jun 2, 2023

View reviewed changes

Hixie force-pushed the longpress branch from fcbd13b to 47d6cac Compare June 14, 2023 22:41

Renzo-Olivares approved these changes Jun 15, 2023

View reviewed changes

Merge branch 'master' into longpress

8d8196d

Hixie added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 5, 2023

auto-submit bot merged commit bc49cd1 into flutter:master Jul 6, 2023

engine-flutter-autoroll mentioned this pull request Jul 6, 2023

Roll Flutter from 35085c394dda to bc49cd1bcab5 (14 revisions) flutter/packages#4387

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 6, 2023

bc49cd1 Allow long-press gestures to continue even if buttons change. (…

17c3b17

…flutter/flutter#127877)

HansMuller mentioned this pull request Jul 7, 2023

Flutter Framework Team: Notable Commits #121415

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 16, 2023

bc49cd1 Allow long-press gestures to continue even if buttons change. (…

9d637cd

…flutter/flutter#127877)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 17, 2023

bc49cd1 Allow long-press gestures to continue even if buttons change. (…

e74a8f3

…flutter/flutter#127877)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 17, 2023

bc49cd1 Allow long-press gestures to continue even if buttons change. (…

d9908d2

…flutter/flutter#127877)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 17, 2023

bc49cd1 Allow long-press gestures to continue even if buttons change. (…

19277a4

…flutter/flutter#127877)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Allow long-press gestures to continue even if buttons change. #127877

Allow long-press gestures to continue even if buttons change. #127877

Uh oh!

Hixie commented May 30, 2023

Uh oh!

Renzo-Olivares left a comment •

edited

Loading

Uh oh!

Renzo-Olivares Jun 2, 2023

Uh oh!

Renzo-Olivares Jun 2, 2023

Uh oh!

Hixie commented Jun 6, 2023

Uh oh!

Renzo-Olivares commented Jun 6, 2023 •

edited

Loading

Uh oh!

Hixie commented Jun 14, 2023

Uh oh!

Hixie commented Jun 15, 2023

Uh oh!

Renzo-Olivares left a comment

Uh oh!

Renzo-Olivares commented Jun 21, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	@override
	void resolve(GestureDisposition disposition) {
	if (_wonArenaForPrimaryPointer && disposition == GestureDisposition.rejected) {
	// This can happen if the gesture has been canceled. For example, when
	// the pointer has exceeded the touch slop, the buttons have been changed,
	// or if the recognizer is disposed.
	assert(_sentTapDown);
	_checkCancel(null, 'spontaneous');
	_reset();
	}
	super.resolve(disposition);
	}

Allow long-press gestures to continue even if buttons change. #127877

Allow long-press gestures to continue even if buttons change. #127877

Uh oh!

Conversation

Hixie commented May 30, 2023

Uh oh!

Renzo-Olivares left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Renzo-Olivares Jun 2, 2023

Choose a reason for hiding this comment

Uh oh!

Renzo-Olivares Jun 2, 2023

Choose a reason for hiding this comment

Uh oh!

Hixie commented Jun 6, 2023

Uh oh!

Renzo-Olivares commented Jun 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Hixie commented Jun 14, 2023

Uh oh!

Hixie commented Jun 15, 2023

Uh oh!

Renzo-Olivares left a comment

Choose a reason for hiding this comment

Uh oh!

Renzo-Olivares commented Jun 21, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Renzo-Olivares left a comment •

edited

Loading

Renzo-Olivares commented Jun 6, 2023 •

edited

Loading