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

Enable scrollbars #735

Merged
merged 3 commits into from
Jul 13, 2022
Merged

Enable scrollbars #735

merged 3 commits into from
Jul 13, 2022

Conversation

guidezpl
Copy link
Member

Fixes #523

I can't repro the original issue, but out of caution, I've set primary: false on the secondary scrollable on the home screen.

@guidezpl guidezpl requested a review from Piinks July 11, 2022 18:04
Copy link
Contributor

@Piinks Piinks left a comment

Choose a reason for hiding this comment

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

Does the gallery write tests?

// the gallery need to be audited before enabling this feature,
// see https:/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.

I think the error presented itself in other demo apps in the gallery as well. We recently changed the default PrimaryScrollController behavior (flutter/flutter#102099) to better avoid that error in general, so it may be ok.

I think the Shrine app was one that also had this issue.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah I see. Can't repro on Shrine anymore.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fab!

@guidezpl
Copy link
Member Author

There are integration tests that open all the demos.

Copy link
Contributor

@Piinks Piinks left a comment

Choose a reason for hiding this comment

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

LGTM!

@guidezpl guidezpl merged commit 89c9492 into main Jul 13, 2022
@guidezpl guidezpl deleted the enable-scrollbars branch August 1, 2022 21:32
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.

Auto-Scrollbars on Desktop are disabled
2 participants