Automatically applying Scrollbars on desktop platforms with configurable ScrollBehaviors #78588

Piinks · 2021-03-19T00:39:39Z

Originally #75354

This change applies Raw/Cupertino/Scrollbars by default on the Web and Desktop platforms. I've done this in a similar way to how we apply the GlowingOverscrollIndicator, in each Widget/Material/Cupertino ScrollBehavior. This allows us to get the right Scrollbar for the given ScrollBehavior.

~~This current proposal requires a breaking change to ScrollBehavior.buildViewportChrome, adding an optional controller parameter as Scrollbar requires a ScrollController for full functionality.~~ Update: Instead I have deprecated buildViewportChrome in favor of buildViewportDecoration. Breaking the former was a really hard break, so I have introduced a new method, and added an opt-in flag (ScrollBehavior.useDecoration) so customers can be migrated gracefully.

This new method will use (also new) ScrollableDetails to ascertain the right decoration. Passing ScrollableDetails will be a bit more future-proof, where if needed we can add to the details rather than needing to change a function signature. The ScrollableDetails currently contains the required AxisDirection (used for the GlowingOverscrollIndicator decoration), and an optional controller. If a controller is not provided, no Scrollbar will be created.

Some instances of Scrollable are excluded from this including:

EditableText
ListWheelScrollView
PageView
NestedScrollView
Dropdown menus (they have their own ScrollBehavior that I have, for now, excluded scrollbars from)

~~To give users control over this, I have added the ability to toggle this feature on and off in ThemeData and CupertinoThemeData.~~ Update: This was a bit over the top, exposing ScrollBehaviors is a cleaner way for users to control defaults. That will be a prerequisite change, in #76739

~~Blocked by: #76739~~

Fixes: #40107
Fixes: #70866 (Last part)
Also related as part of ScrollBehavior overhaul: #75728

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I listed at least one issue that this PR fixes in the description above.
I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test exempt.
I updated/added relevant documentation (doc comments with ///).
I signed the CLA.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

…rollbar

packages/flutter/lib/src/material/app.dart

goderbauer · 2021-03-30T21:00:24Z

packages/flutter/lib/src/material/dropdown.dart


 // Do not use the platform-specific default scroll configuration.
 // Dropdown menus should never overscroll or display an overscroll indicator.
+// The default scrollbar platforms will apply.


It will use the scrollbars defined in ScrollBehavior, but it should be using the ones from the behavior in the context, right?

Ah I see what you mean, I will update the copyWith function to accommodate this.

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

goderbauer · 2021-03-30T21:02:38Z

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

        axisDirection: _isMultiline ? AxisDirection.down : AxisDirection.right,
        controller: _scrollController,
-        physics: widget.scrollPhysics,
+        physics: widget.scrollPhysics ?? widget.scrollBehavior?.getScrollPhysics(context),


Is this actually necessary? When widget.scrollPhysics is null, it should be the Scrollable that does this lookup?

Oh yeah, you are right. I guess that makes it a bit redundant here and in Scrollable.

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

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

goderbauer · 2021-03-30T21:17:26Z

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

+      'Temporary migration flag, do not use. '
+      'This feature was deprecated after v2.1.0-11.0.pre.'
+    )
+    bool useDecoration = false,


Do you think we could avoid this flag altogether by making buildOverscrollIndicator just call buildViewportChrome by default?

I am not sure what you mean. My plan is to remove buildViewportChrome, so if we are calling it, won't it then take more steps to remove it?
buildViewportChrome may not be just specific to the overscroll indicator for some of our customers currently. I may be misunderstanding. :)

Also, that would make it such that scrollbars will be built before folks have had a chance to migrate to their desired behavior..

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

goderbauer

LGTM assuming tests on PR and in google pass

🥳 Thanks for sticking with this!

goderbauer · 2021-04-02T17:52:47Z

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

        physics: widget.scrollPhysics,
        dragStartBehavior: widget.dragStartBehavior,
        restorationId: widget.restorationId,
+        scrollBehavior: widget.scrollBehavior ?? ScrollConfiguration.of(context).copyWith(scrollbars: _isMultiline),


Maybe only do the copy if _isMultiline is true? (This avoids a dependency on the ScrollConfig (which may trigger rebuilds) if we don't need it). If _isMultiline is false, we can just use null as scrollBehvaior to have the lower levels look it up.

I think it's the other way around, if _isMultiLine is true, we want the ScrollBehavior that will apply scrollbars. If it is false, then we copy to remove the scrollbar.

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

packages/flutter/lib/src/material/app.dart

Scrollbars are automatically applied on desktop since flutter/flutter#78588

DexEze · 2023-05-22T06:01:09Z

I have found an issue with this change and how it interacts with TabBarView Widgets within a NestedScrollView Widget:

https://stackoverflow.com/questions/76277687/flutter-nestedscrollview-scrollcontroller-is-currently-attached-to-more-than-one/76303348#76303348

Basically on desktop, it causes a scrollbar to be attached to each ScrollView in the tabs, this causes the scroll controller to be connected to more than one tab bar and throws an error.

Piinks added 25 commits February 3, 2021 13:56

Initial thoughts

9871cec

++

d6a48a5

Fix tests

d73f0a8

Apply suggestions from code review

68061e4

Merge branch 'master' of github.com:flutter/flutter into autoScrollbar

091d6be

Try deprecating viewportChrome

393fc52

++

5ef5041

Merge branch 'autoScrollbar' of github.com:Piinks/flutter into autoSc…

d3edf86

…rollbar

Clean up

d53723b

Merge branch 'master' of github.com:flutter/flutter into autoScrollbar

3806ed8

++

84dc966

LEss breaking

fba7709

Fix tests

0408509

Fix CustomScrollView

8b32be8

++

dd752ba

++

e2648de

Merge branch 'master' of github.com:flutter/flutter into autoScrollbar

3a2130e

Doc fix

2b8ae2d

Review feedback

3df82fa

Merge branch 'master' of github.com:flutter/flutter into autoScrollbar

945eae4

Nits

0cb359e

Updates from feedback

8d0d78c

Fix docs

eb8410c

Let's try something different

42e995f

No autoScrollbar flags

d4c2ea4

Piinks requested a review from goderbauer March 19, 2021 00:39

flutter-dashboard bot added f: cupertino flutter/packages/flutter/cupertino repository f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. labels Mar 19, 2021

google-cla bot added the cla: yes label Mar 19, 2021

Piinks added 2 commits March 29, 2021 17:55

Merge branch 'master' of github.com:flutter/flutter into autoScrollbar2

4a068bf

Fix tests

823555b

Piinks changed the title ~~WIP Configured ScrollBehaviors for decorating viewports~~ Automatically applying Scrollbars on desktop platforms with configurable ScrollBehaviors Mar 29, 2021

Piinks added 3 commits March 29, 2021 18:30

Fix rude IDE import

912e1c7

Fix doc references and smoke test

6acc435

Fix more docs

317637a

Piinks requested a review from goderbauer March 30, 2021 16:08

goderbauer reviewed Mar 30, 2021

View reviewed changes

Piinks added 3 commits March 31, 2021 15:58

Review feedback

de2a522

Merge branch 'master' of github.com:flutter/flutter into autoScrollbar2

638760b

Fix docs reference

78b97ec

Piinks requested a review from goderbauer April 1, 2021 19:09

Updates from meeting

edf8441

goderbauer approved these changes Apr 2, 2021

View reviewed changes

Clean up and docs

ee8e141

Piinks added the waiting for tree to go green label Apr 2, 2021

Piinks merged commit e40610d into flutter:master Apr 2, 2021

Piinks mentioned this pull request Apr 22, 2021

[web] On Master and beta, multiple scrollbars are displayed if widget is placed between scrollbar and scrollable widget. #80757

Closed

creativecreatorormaybenot mentioned this pull request May 25, 2021

[web] Precise scrolling impossible by default #83368

Closed

jpnurmi added a commit to jpnurmi/ubuntu-desktop-installer that referenced this pull request Jan 19, 2022

Clean up explicit scrollbars that are no longer needed

de2b6d4

Scrollbars are automatically applied on desktop since flutter/flutter#78588

jpnurmi mentioned this pull request Jan 19, 2022

Clean up explicit scrollbars that are no longer needed canonical/ubuntu-desktop-installer#623

Merged

jpnurmi added a commit to canonical/ubuntu-desktop-installer that referenced this pull request Jan 21, 2022

Clean up explicit scrollbars that are no longer needed (#623)

8d65ad7

Scrollbars are automatically applied on desktop since flutter/flutter#78588

Piinks mentioned this pull request Sep 16, 2022

Remove deprecated ScrollBehavior.buildViewportChrome #111715

Merged

8 tasks

DexEze mentioned this pull request May 22, 2023

Automatically applying Scrollbars on desktop platforms causes NestedScrollView/TabBarView bug #127299

Closed

2 tasks

Automatically applying Scrollbars on desktop platforms with configurable ScrollBehaviors #78588

Automatically applying Scrollbars on desktop platforms with configurable ScrollBehaviors #78588

Uh oh!

Conversation

Piinks commented Mar 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

goderbauer 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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

DexEze commented May 22, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Piinks commented Mar 19, 2021 •

edited

Loading