[flutter] reject mouse drags by default in scrollables #81569

jonahwilliams · 2021-04-30T20:09:54Z

Currently flutter web and desktop apps allow scrolling by dragging with the mouse. This feels unnatural compared to other desktop applications, and makes it harder to use more complex mouse gestures on items that are contained within a scrollable and impossible to accurately select text (since the text selection drag competes with the scroll drag).

~~This is likely a breaking change, so the old scrollable behavior is configurable via scroll behavior: enableMouseDrag~~

Nothing broke!

Fixes #71322

jonahwilliams · 2021-04-30T20:12:12Z

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

+  ///
+  /// This is used by scrollables to force mouse interactions to go through the
+  /// scroll wheel or scrollbar, instead of allowing drag gestures to move them.
+  final bool rejectMousePointers;


This could be inverted to something like touchOnly?

I wonder should this be platform dependent? for example set it to true when platform is desktop or web. Do you know what is the android behavior it user uses mouse?

This isn't really a platform adaption, its an adaption based on how the user is interacting with the device. My impression of android/ios is that mouse interaction is uncommon, and if it is apps are likely to do the wrong thing anyway.

Maybe we can use a set of acceptedPointerDevices, so it's not mouse-specific, and put it in ScrollBehavior so we can configure it by platform like @chunhtai suggests.

Is the idea that when it is mouse, prevent drag scroll regardless of the platform?

Generally speaking scrolling with a mousewheel or scrollbar is more efficient that scrolling by a simulated touch gesture. By enabling mouse based drag in scrollables, flutter forces any gesture accepting widgets within those lists to compete with the scroll drag events. Disambiguation of these gestures isn't perfect and neither are the users movements - this leads to a harder to use application.

For example, try and drag to scroll to the right/left on the flutter dashboard. Because the left/right drags compete with the up/down drags its easy to get stuck accidentally. Of couse that also implies we need provide a way to keep this enabled, to support the nested scrollable with no scrollbar pattern.

I also don't think you should try to generalize this much. Its really unlikely that a gesture detector wants to disambiguate between, say, touch and inverse stylus. And if it does, it might be doing something entirely custom which would make extra configurability on our end a waste.

mouse and touch (stylus/inverse stylus) are really very separate kinds of interaction.

Piinks

This is awesome thank you!

I linked the issue this looks likely to resolve.

An earlier implementation of this broke DraggableScrollableSheet, but it looks like this isn't breaking anything 🎉 (still need to check g3, I've fired those tests off to see).

Piinks · 2021-04-30T21:58:05Z

packages/flutter/lib/src/widgets/scrollable.dart

          _gestureRecognizers = <Type, GestureRecognizerFactory>{
            HorizontalDragGestureRecognizer: GestureRecognizerFactoryWithHandlers<HorizontalDragGestureRecognizer>(
-              () => HorizontalDragGestureRecognizer(),
+              () => HorizontalDragGestureRecognizer(rejectMousePointers: true),


Originally we thought about putting this feature in ScrollBehavior. Down below here the _configuration is the ScrollBehavior setting up the velocityTrackerBuilder. That way folks can configure this as they like, or target subtrees by putting in another ScrollConfiguration.

Piinks · 2021-04-30T21:59:43Z

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

+  ///
+  /// This is used by scrollables to force mouse interactions to go through the
+  /// scroll wheel or scrollbar, instead of allowing drag gestures to move them.
+  final bool rejectMousePointers;


Maybe we can use a set of acceptedPointerDevices, so it's not mouse-specific, and put it in ScrollBehavior so we can configure it by platform like @chunhtai suggests.

chunhtai · 2021-04-30T22:01:16Z

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

+  ///
+  /// This is used by scrollables to force mouse interactions to go through the
+  /// scroll wheel or scrollbar, instead of allowing drag gestures to move them.
+  final bool rejectMousePointers;


Is the idea that when it is mouse, prevent drag scroll regardless of the platform?

chunhtai · 2021-04-30T22:02:32Z

packages/flutter/lib/src/widgets/scrollable.dart

          _gestureRecognizers = <Type, GestureRecognizerFactory>{
            VerticalDragGestureRecognizer: GestureRecognizerFactoryWithHandlers<VerticalDragGestureRecognizer>(
-              () => VerticalDragGestureRecognizer(),
+              () => VerticalDragGestureRecognizer(rejectMousePointers: true),


Does the developer have a way to override this behavior? I wonder whether this should be a theme-thing. cc @HansMuller

edit: I don't know there was ScrollConfiguration, I think this can be in ScrollConfiguration

ScrollConfiguration seems like a reasonable choice - assuming it is only created for lists and things? Like draggable sheets need to still be draggable, but that doesn't use a scrollable correct?

jonahwilliams · 2021-04-30T22:13:23Z

Assuming we put this in the scroll configuration, I imagine the breaking change might go something like:

expose the ability to turn of drags - breaking change announcement
default to false?

chunhtai · 2021-04-30T22:21:37Z

Assuming we put this in the scroll configuration, I imagine the breaking change might go something like:

expose the ability to turn of drags - breaking change announcement

default to false?

I vote for first, we should target the user experience

jonahwilliams · 2021-04-30T22:55:34Z

Next patch is a little WIP, but moves the configuration onto the scroll behavior and then defaults it to the current incorrect behavior

jonahwilliams · 2021-04-30T22:59:29Z

What is the over/under on adding a field to scroll behavior being a breaking change? i'm a bit concerned we're actually going to make this more breaking not less, to allow configuring a behavior that is IMO a bug.

jonahwilliams · 2021-04-30T22:59:49Z

i guess as long as people are extending ScrollBehavior it should be OK

chunhtai · 2021-04-30T23:26:34Z

Next patch is a little WIP, but moves the configuration onto the scroll behavior and then defaults it to the current incorrect behavior

Oh sorry I overlooked the original comment. I meant I vote for defaulting it to true to prevent mouse scrolling (or whatever you think the correct behavior is unless it break internal customer, then you probably need to do three phase transitions...), and I don't have opinion whether we expose and override or not. We can always expose it later if people want to change it. Not exposing it will however, probably get a lot of complain though

jonahwilliams · 2021-05-01T18:13:57Z

@Hixie considerations for naming? I think the correct name would be something like mouseDrag and not enableMouseDrag as I have it - but then maybe that should be mousedrag? Unsure 😄

Hixie · 2021-05-01T19:30:27Z

How about rather than a boolean here we just have a set of pointer kinds that accept drags? Then you can name it just "devices" and "dragDevices" or some such. (That would also let people decide if styluses should allow drags. I suspect there are UIs where you'd want a stylus to draw and a finger to drag.)

jonahwilliams · 2021-05-01T20:04:22Z

Yeah, that was the suggestion that @Piinks and @chunhtai made above 😅 - I'll give it a shot

Piinks · 2021-05-03T17:04:00Z

What is the over/under on adding a field to scroll behavior being a breaking change? i'm a bit concerned we're actually going to make this more breaking not less, to allow configuring a behavior that is IMO a bug.

I don't think it will be that breaking, we just made a bunch of changes to ScrollBehavior and I already migrated the implementing customers to land those changes. This was actually on my list for the next change to make before migrating customers back. :)

jonahwilliams · 2021-05-03T17:05:24Z

we just made a bunch of changes to ScrollBehavior and I already migrated the implementing customers to land those changes

Awesome!

Piinks

LGTM!

Piinks · 2021-05-03T17:15:01Z

packages/flutter/lib/src/widgets/scrollable.dart

+              () => VerticalDragGestureRecognizer(supportedDevices: _configuration.dragDevices),
              (VerticalDragGestureRecognizer instance) {
                instance
                  ..onDown = _handleDragDown


Nit: These could probably be chained to match the rest readability-wise. :)

Suggested change

() => VerticalDragGestureRecognizer(supportedDevices: _configuration.dragDevices),

(VerticalDragGestureRecognizer instance) {

instance

..onDown = _handleDragDown

() => VerticalDragGestureRecognizer(),

(VerticalDragGestureRecognizer instance) {

instance

..onDown = _handleDragDown

..supportedDevices = _configuration.dragDevices

chunhtai · 2021-05-03T17:30:59Z

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

+  /// The device types that this gesture recognizer will accept drags from.
+  ///
+  /// If not specified, defaults to all pointer kinds.
+  final List<PointerDeviceKind> supportedDevices;


Use a set instead?

chunhtai · 2021-05-03T17:33:01Z

packages/flutter/lib/src/widgets/scroll_configuration.dart

        || oldDelegate.overscrollIndicator != overscrollIndicator
        || oldDelegate.physics != physics
        || oldDelegate.platform != platform
+        || listEquals(oldDelegate.dragDevices, dragDevices)


we should probably use set otherwise the order difference may cause a rebuild.

also we should set the type? i.e. listEquals<PointerDeviceKind>

jonahwilliams · 2021-05-03T17:38:57Z

I wasn't sure whether to prefer a set or list, I mostly chose a list because I could use PointerDeviceKind.values as a default and since contains on small lists is generally faster (but that doesn't really matter here). Will update to a set

Piinks · 2021-05-03T18:40:37Z

Triggered a global test run manually, so it likely won't update here. Everything passed. 🎉 Will set the G check green manually.

jonahwilliams · 2021-05-03T22:02:38Z

Updated to use a set, and to set the supported devices in the gesture factory callback

jonahwilliams · 2021-05-03T22:03:02Z

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

 }

+const Set<PointerDeviceKind> _kAllPointerDeviceKinds = <PointerDeviceKind>{
+  ...PointerDeviceKind.values,


I didn't know we could do this!

talksik · 2022-11-02T11:42:12Z

I'm still having this problem. horizontal scroll within a vertical scroll. However, fluent-ui package has a HorizontalScollView that works. However, they don't show a scrollbar.

Piinks · 2022-11-02T16:44:45Z

@talksik can you please file a new issue with:

a minimal code sample that reproduces the issue
the output of flutter doctor -v

Thanks!

[flutter] reject mouse drag in scrollables

5efce5c

flutter-dashboard bot added a: tests "flutter test", flutter_test, or one of our tests framework flutter/packages/flutter repository. See also f: labels. labels Apr 30, 2021

google-cla bot added the cla: yes label Apr 30, 2021

jonahwilliams requested review from Piinks and goderbauer April 30, 2021 20:11

jonahwilliams commented Apr 30, 2021

View reviewed changes

Piinks reviewed Apr 30, 2021

View reviewed changes

chunhtai reviewed Apr 30, 2021

View reviewed changes

configure via scroll behavior

5085988

switch to correct default and rename to be positive

7b33429

drag devices

8e8954f

Piinks approved these changes May 3, 2021

View reviewed changes

chunhtai reviewed May 3, 2021

View reviewed changes

switch to set, use setter

774016b

jonahwilliams commented May 3, 2021

View reviewed changes

inceptusp mentioned this pull request May 20, 2021

Scrolling a list view with mouse gestures stopped working on desktop and web after flutter update - Channel dev, 2.3.0-12.1.pre #82982

Closed

TahaTesser mentioned this pull request May 21, 2021

Scroll views with horzontal scroll axis should scroll by default without pressing shift when on desktop #83065

Closed

This was referenced May 25, 2021

[desktop] Horizontal Scrolling Broken on windows and linux #83359

Closed

[web] Precise scrolling impossible by default #83368

Closed

This was referenced Jun 16, 2021

Scroll drag not working in Listview #84627

Closed

[web] - Horizontal Scrolling is not working anymore #84842

Closed

RefreshIndicator: Drag to refresh in Desktop (MacOS, Linux, Windows) doesn't work anymore. #85023

Closed

maheshj01 mentioned this pull request Jul 27, 2021

PageView widget mouse swipe/drag is not working on web #86959

Closed

WheelerCC mentioned this pull request Jul 28, 2021

Scroll issue in Web app MarcinusX/NumberPicker#118

Open

This was referenced Aug 4, 2021

Can't scroll horizontal listview #87621

Closed

Horizontal scrolling not working #87904

Closed

TahaTesser mentioned this pull request Aug 12, 2021

PaginatedDataTable horizontal scroll broken on web 2.5.0-6.0.pre.47 #88044

Closed

HansMuller mentioned this pull request Sep 13, 2021

Flutter 2.5: Scrolling in Web not working #89985

Closed

Piinks mentioned this pull request Sep 14, 2021

weird log for DraggableScrollableSheet #89681

Closed

darshankawar mentioned this pull request Sep 16, 2021

[windows][web] Horizontal and Vertical scrolling did not happen when scrolling through the external mouse #90169

Closed

maheshj01 mentioned this pull request Sep 16, 2021

[2.5] Recursion: Flutter app is not scrollable in Vysor #90177

Closed

This was referenced Sep 24, 2021

Flutter Web : Can't Scroll ListView with scrollDirection: Axis.horizontal #90660

Closed

horizontal list example is untestable with mouse on desktop flutter/website#7756

Closed

TahaTesser mentioned this pull request Oct 5, 2021

Scrolling doesn't work on [macOS] #91292

Closed

Piinks mentioned this pull request Oct 13, 2021

_WrappedScrollBehavior.copyWith does not pass on previous copyWith elements #91673

Closed

maheshj01 mentioned this pull request Nov 1, 2021

Scrolling doesn't work for Flutter Web on Win10, Chrome/Edge [HtmlKit+CanvasKit] #92730

Closed

MxSoul mentioned this pull request Dec 10, 2021

Fix the problem that the 'adb shell input swipe' command is invalid. #94535

Closed

8 tasks

maheshj01 mentioned this pull request Feb 25, 2022

Flutter app cannot be scrolled with TeamViewer #98974

Closed

bleroux mentioned this pull request Sep 21, 2023

PageView API doc sample fails on Desktop and Web #135188

Closed

HansMuller mentioned this pull request Sep 26, 2023

Update or document RefreshIndicator behavior on desktop #135510

Open

huycozy mentioned this pull request Oct 19, 2023

[Proposal]DraggableScrollableSheet should consume mouse events to expand and collapse on the Desktop #101903

Open

marcoedomingos mentioned this pull request Jan 31, 2024

Flutter Web: Scrolling doesn't work #142529

Closed

maheshj01 mentioned this pull request Feb 10, 2025

[Web] [Windows] Swipping by gestures in TabBarView not working #162894

Closed

[flutter] reject mouse drags by default in scrollables #81569

[flutter] reject mouse drags by default in scrollables #81569

Uh oh!

Conversation

jonahwilliams commented Apr 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chunhtai Apr 30, 2021 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Piinks left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chunhtai Apr 30, 2021 • 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!

Choose a reason for hiding this comment

Uh oh!

jonahwilliams commented Apr 30, 2021

Uh oh!

chunhtai commented Apr 30, 2021

Uh oh!

jonahwilliams commented Apr 30, 2021

Uh oh!

jonahwilliams commented Apr 30, 2021

Uh oh!

jonahwilliams commented Apr 30, 2021

Uh oh!

chunhtai commented Apr 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonahwilliams commented May 1, 2021

Uh oh!

Hixie commented May 1, 2021

Uh oh!

jonahwilliams commented May 1, 2021

Uh oh!

Piinks commented May 3, 2021

Uh oh!

jonahwilliams commented May 3, 2021

Uh oh!

Piinks left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chunhtai May 3, 2021 • 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!

jonahwilliams commented May 3, 2021

Uh oh!

Piinks commented May 3, 2021

Uh oh!

jonahwilliams commented May 3, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonahwilliams commented Apr 30, 2021 •

edited

Loading

chunhtai Apr 30, 2021 •

edited

Loading

chunhtai Apr 30, 2021 •

edited

Loading

chunhtai commented Apr 30, 2021 •

edited

Loading

chunhtai May 3, 2021 •

edited

Loading

talksik commented Nov 2, 2022 •

edited

Loading