Still WIP here 🤕 😝

This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Tests were not as bad as they initially seemed, fixed now.

My primary concerns for adding this feature are that

• users will be annoyed by Scrollbars showing up where they are not wanted
• users that are already using Scrollbars on desktop/web will be broken by duped Scrollbars
• users implementing or extending buildViewportChrome will be broken
• creating scrollbars on mobile will require the user to add special handling when their app in on web/desktop, lest be affected by the duped scrollbar exception mentioned above
  • or they could turn off autoScrollbars all together and have no issue - which I think kind of nullifies the point of this feature.

From a framework user's perspective I think it's concerning that I have to worry about double scrollbars. If this feature is to be implemented I would assume that the framework will handle figuring out if I added a scrollbar explicitly or I am relying on automated scrollbar feature. If this detection is not possible then I would rather not have this feature. Just sharing my opinion, for what it's worth.

Thanks @kunit1, I agree. I'm gathering more feedback as well from the rest of the team.

I would assume that the framework will handle figuring out if I added a scrollbar explicitly or I am relying on automated scrollbar feature. If this detection is not possible then I would rather not have this feature.

I don't think we can automatically detect a Scrollbar. They can be placed anywhere in the tree, and even if we could track them all down, we would need to distinguish which were made automatically and which were user generated.
Also, there is already a pattern of use where the Scrollbar listens to more than one Scrollable. I've seen this most often with 2D scrolling.

In general, I try to avoid automatically making decisions for users like this.

The preceding change has landed, this has been updated and is ready for review. :)

Looks like the analyzer is not happy.

I thought about this a little more: It seems like we have cases where we want a scrollbar, but not glow (DropDown menu?) and cases where we want a glow, but no scrollbar (PageView). Do we need to disentangle the glow and the scrollbar stuff?

Here's an idea I had: We could change the ScrollBehavior class to represent something like the following:

class ScrollBehavior {
  
  @protected
  Widget buildViewportChrome(BuildContext context, Widget child, AxisDirection axisDirection) {
    // current implementation, unchanged, mainly used for glow
  }

  @protected
  Widget buildScrollbars(BuildContext context, Widget child, ScrollableDetails details,) {
    // wraps child in scrollbars (or returns the child directly)
  }

  // Called by the Scrollable instead of buildViewportChrome.
  Widget buildViewportDecorations(BuildContext context, Widget child, ScrollableDetails details,) {
    return buildViewportChrome(context, buildScrollbars(context, child, details), details.axisDirection);
  }

  // probably needs a different name?
  ScrollBehavior copyWith({bool chrome = true, bool = scrollbars = true}) {
    return _WrappedScrollBehavior(this, chrome: chrome, scrollbars: scrollbars);
  }

  // Other stuff in ScrollBehavior

}

The _WrappedScrollBehavior basically forwards all calls to the wrapped instance, except if chrome or scrollbars is false it returns the child unwrapped from buildScrollbars or buildViewportDecorations, respectively.

Furthermore, we change Scrollable to take an optional ScrollBehavior (if it is not provided, it looks one up in the tree like it does today). (This is similar to how it already takes an optional physics param.)

Now widgets that don't want scrollbars can pass ScrollConfigoration.of(context).copyWith(scrollbars: false) to the scrollable as behavior. Similar for widgets that don't need a glow.

As a bonus: This shouldn't be breaking.

Last, but not least, we need to change Scrollable to always have a ScrollController (if none was passed in, it creates one locally) to always pass it to ScrollDetails for the ScrollBehavior (otherwise we wouldn't be able to build the scrollbar).

Hm, let me experiment and see what this would look like. My first impression is that it's a bit complicated, and possibly hard to understand why buildViewportChrome is just for the glow while there are other methods for other decorations.
Also, how does this look if in the future we add more decorations?

I'll play with this a bit more..

Closing in favor of #78588