Skip to content
This repository has been archived by the owner on Jun 13, 2024. It is now read-only.

Disable desktop scrollbars #524

Merged
merged 2 commits into from
Jun 17, 2021
Merged

Disable desktop scrollbars #524

merged 2 commits into from
Jun 17, 2021

Conversation

Piinks
Copy link
Contributor

@Piinks Piinks commented Jun 16, 2021

See #523

On desktop, scrollbars are applied automatically by the scroll behavior. Currently, most vertical scroll views in the gallery app are all using the same primary scroll controller. This causes a crash when users try to interact with the scrollbars. This change will disable the automatic scrollbars to eliminate the crash while we audit and fix up all of the scrollables in the gallery that are using the same scroll controller.

@Piinks Piinks requested review from dnfield and rami-a June 16, 2021 23:30
lib/main.dart Outdated
// ScrollBehavior. This overrides that. All vertical scrollables in
// the gallery need to be audited before enabling this feature,
// see https://github.com/flutter/gallery/issues/523
scrollBehavior: const MaterialScrollBehavior().copyWith(scrollbars: false),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, this problem also exists in the legacy Flutter gallery.

I'm missing some context here to really understand why this is the right fix. In particular, why do certain scrollables need to set primary or not, when it used to "just work" that they didn't.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, this problem also exists in the legacy Flutter gallery.

Yeah, I need to track that down too.

why do certain scrollables need to set primary or not, when it used to "just work" that they didn't.

Vertical scrollables automatically attach to the primary scroll controller if they aren't given a scroll controller. The auto-scrollbar stuff is new, and the gallery will need to be updated for it. It 'just worked' before because there wasn't a widget in the tree enforcing that a certain scroll controller only has one position attached. Since the ScrollBehavior on desktop is adding scrollbars now, the gallery crashes because the when the user tries to interact with the scrollbar, the scrollbar tries to use the scroll controller to change the position. Since more than one position is attached, it breaks.

There is a migration guide here: https://flutter.dev/docs/release/breaking-changes/default-desktop-scrollbars

I am also updating the assertion in the framework because I do think it is a little too aggressive right now.

Copy link
Contributor

@rami-a rami-a left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Piinks Piinks merged commit c858388 into flutter:master Jun 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants