Introduce multi-touch drag strategies for DragGestureRecognizer
#136708
Conversation
github-actions
bot
added
framework
There was a problem hiding this comment.
If we pursue this, we will likely want to maintain the current behavior.
There was a problem hiding this comment.
I prefer to switch the default behavior to the same as Android, because I think this experience is better, even on the ios platform. What do you think?
If the developer really wants the previous behavior, it can be achieved by overriding ScrollBehavior.
There was a problem hiding this comment.
My first thought is, how would we clarify for developers the difference between this+DragGestureRecognizer and MultiDragGestureRecognizer
Good point! I was confused at the time, and then I went to study MultiDragGestureRecognizer.
They do have some confusion in terms of names and are actually completely different things.
multitouchDragStrategy means to use multiple fingers to complete one dragging task.
MultiDragGestureRecognizer is to distribute different finger events to different MultiDragPointerState instances to achieve the purpose of dragging multiple objects at the same time(see Draggable widget).
There was a problem hiding this comment.
Or we can change the name to DragMultitouchStrategy or DragMultiFingerStrategy ?
I am not sure :)
There was a problem hiding this comment.
I'm also leaning towards maintaining the current behavior (at least in DragGestureRecognizer). Some users might be depending on the current behavior in tests, and with DragGestureRecognizer being used frequently and having been around for so long then maybe we shouldn't break this expectation.
There was a problem hiding this comment.
Ok! I think we are ready to fix this! I chatted with @goderbauer 👍, and @Hixie 👍 , and I think @dkwingsmt also wanted to weigh in here.
I originally thought preserving the old behavior as the default was the right way to go in case it was a breaking change but I was wrong. Can you update this to have the default be the correct behavior? I can check to see if anything breaks, and if it does we can go from there with migration guidance and communicating the change more widely.
The way you have fashioned this is a pretty painless migration. Well done! 💯
There was a problem hiding this comment.
@dkwingsmt Hi, Could you take a look at this? : )
There was a problem hiding this comment.
Yes, I agree that we should provide the "correct" behavior out of the box, since dragging is such a fundamental feature. The current behavior should be considered a bug that we fix with this PR, while preserving an option for users to go back.
I have two more suggestions (sorry for posting them so late):
We should default to the iOS-style strategy on iOS in the future.As the PR description said, the iOS-style will be implemented in the future. For now, I agree that we can make iOS switch to the Android style since it's an improvement anyway.We might be better off with aI misunderstood the code. The current design agrees with my idea.InheritedWidgetthat configures the entire app in one go instead of having to provide this argument for every widget, especially since many of the dragging widgets are buried in the depth of other widgets.
There was a problem hiding this comment.
We might be better off with a InheritedWidget that configures the entire app in one go instead of having to provide this argument for every widget, especially since many of the dragging widgets are buried in the depth of other widgets.
I think that is the goal of using the ScrollBehavior here @dkwingsmt. It can be set for the entire app, and is inherited so all scrollables will use it. This change does not require every widget to be configured that way.
There was a problem hiding this comment.
Ah I see, I misunderstood the code. The current design makes sense!
There was a problem hiding this comment.
We should probably add another option that allows the user to revert to the old Flutter behavior, just in case anyone wants it.
There was a problem hiding this comment.
That is one of the options here. :)
There was a problem hiding this comment.
I see, I thought trackAllActivePointers referred to the iOS-style behavior (and hence my other misunderstanding). So if we decides to go with this name, how do we plan to name the iOS-style behavior?
Maybe rename it to sumAllPointers (for the old behavior) and averageAllPointers (for the iOS-style)?
Also, do we need the "active" word here? It doesn't seem needed to me here and I'd prefer shorter names if possible.
Piinks
left a comment
Piinks
left a comment
There was a problem hiding this comment.
The end of an era!
I'd like to add a guide to https://docs.flutter.dev/release/breaking-changes so folks know what to expect when this is released to stable.
xu-baolin
added
the
autosubmit
flutter/flutter@5e5b529...918e336 2023-11-30 [email protected] Move Impeller tests on Pixel 7 Pro from staging to prod (flutter/flutter#139280) 2023-11-30 [email protected] Introduce multi-touch drag strategies for `DragGestureRecognizer` (flutter/flutter#136708) 2023-11-30 [email protected] Use the correct recipe on fuchsia_precache. (flutter/flutter#139279) 2023-11-30 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Migration for the `sendTiming` events for `package:unified_analytics`" (flutter/flutter#139278) 2023-11-30 [email protected] Migrate fuchsia_precache to shard tests. (flutter/flutter#139202) 2023-11-29 [email protected] Fix chips `onDeleted` callback don't show the delete button when disabled (flutter/flutter#137685) 2023-11-29 [email protected] Roll Flutter Engine from 9a7e49d75411 to 35939ca8534f (5 revisions) (flutter/flutter#139259) 2023-11-29 [email protected] Refactor to use Apple system fonts (flutter/flutter#137275) 2023-11-29 [email protected] Dynamic view sizing (flutter/flutter#138648) 2023-11-29 [email protected] Roll dependencies (flutter/flutter#139203) 2023-11-29 [email protected] add sourceTimeStamp to ScaleUpdateDetails (flutter/flutter#135936) 2023-11-29 [email protected] Roll Flutter Engine from 222beb28a8eb to 9a7e49d75411 (1 revision) (flutter/flutter#139250) 2023-11-29 [email protected] Remove deprecated `PlatformMenuBar.body` (flutter/flutter#138509) 2023-11-29 [email protected] Migration for the `sendTiming` events for `package:unified_analytics` (flutter/flutter#138896) 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://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
|
Filed #139333 to follow up on the migration guide. I should be able to whip that up tomorrow. |
|
I think we should rename it to ChatGPT: |
_Description of what this PR is changing or adding, and why:_ Migration guide for flutter/flutter#136708 Fixes flutter/flutter#139333 ## Presubmit checklist - [x] This PR doesn’t contain automatically generated corrections (Grammarly or similar). - [x] This PR follows the [Google Developer Documentation Style Guidelines](https://developers.google.com/style) — for example, it doesn’t use _i.e._ or _e.g._, and it avoids _I_ and _we_ (first person). - [x] This PR uses [semantic line breaks](https://github.com/dart-lang/site-shared/blob/main/doc/writing-for-dart-and-flutter-websites.md#semantic-line-breaks) of 80 characters or fewer.
_Description of what this PR is changing or adding, and why:_ Migration guide for flutter/flutter#136708 Fixes flutter/flutter#139333 ## Presubmit checklist - [x] This PR doesn’t contain automatically generated corrections (Grammarly or similar). - [x] This PR follows the [Google Developer Documentation Style Guidelines](https://developers.google.com/style) — for example, it doesn’t use _i.e._ or _e.g._, and it avoids _I_ and _we_ (first person). - [x] This PR uses [semantic line breaks](https://github.com/dart-lang/site-shared/blob/main/doc/writing-for-dart-and-flutter-websites.md#semantic-line-breaks) of 80 characters or fewer.
…utter#136708) Fixes flutter#11884 As flutter#38926 pointed out, the current Flutter implementation of multi-finger drag behavior is different from iOS and Android. This change introduces the `MultitouchDragStrategy` attribute, which implements the Android behavior and can be controlled through `ScrollBehavior`, while retaining the ability to extend iOS behavior in the future.
5 participants
Fixes #11884
As #38926 pointed out, the current Flutter implementation of multi-finger drag behavior is different from iOS and Android.
This change introduces the
MultitouchDragStrategyattribute, which implements the Android behavior and can be controlled throughScrollBehavior, while retaining the ability to extend iOS behavior in the future.