Updated Interactive Scrollbars #71664
Conversation
flutter-dashboard
bot
added
a: internationalization
Piinks
added
f: scrolling
There was a problem hiding this comment.
This looks good; most of the feedback is just nitpickery to prove that I've read it.
The main non-trivial feedback is about making RawScrollbar's protected gesture API less touch-specific and using the Theme's color scheme to compute defaults in the Material scrollbar.
| } | ||
|
|
||
| /// Same as hitTest, but includes some padding to make sure that the region | ||
| /// isn't too small to be interacted with by the user. | ||
| bool hitTestInteractive(Offset position) { | ||
| if (_thumbRect == null) { |
There was a problem hiding this comment.
I assume that eventually this will obviate the _thumbRect! expressions.
There was a problem hiding this comment.
I have updated to remove all of the ! by making the _thumbRect late. :) Good call!
There was a problem hiding this comment.
That actually broke, switching back to nullable. 😅
| _currentController = null; | ||
| } | ||
|
|
||
| void _handleTrackTapDown(TapDownDetails details) { |
There was a problem hiding this comment.
Eventually we'll want subclasses to be able to handle track gestures via more protected methods.
| if (_hoverIsActive) MaterialState.hovered, | ||
| }; | ||
|
|
||
| MaterialStateProperty<Color> get _thumbColor { |
There was a problem hiding this comment.
The default values of the _thumbColor, _trackColor, etc properties should be based on the theme's color scheme because dark mode and consistency with other Material components.
| /// or use a PrimaryScrollController to share it. | ||
| /// | ||
| /// Here is an example of using the `controller` parameter to enable | ||
| /// scrollbar dragging for multiple independent ListViews: |
There was a problem hiding this comment.
If it's just an example that we've moved, it's OK to leave it as is. This example and the one for the controller property seem unnecessarily complicated. And they should be written in terms of RawScrollbar. We can improve them in a separate PR.
Description
Originally from #71181
Started to split up into multiple changes starting with #71242
Splitting this up did not make review easier, so I have applied the feedback that has been received so far into this new PR.
Design doc: https://flutter.dev/go/update-scrollbars
This is the first of several changes planned for #70866
This change will refactor the Scrollbar and CupertinoScrollbar widgets to use RawScrollbar as a base class.
I wanted to complete this refactoring first since these widgets have so many common lines of code.
The RawScrollbar widget will provide the basic fade in-out scrollbar thumb the framework currently provides, with the option to customize and add additional behavior by extending the class.
RawScrollbar will enable dragging on the thumb for RawScrollbar, Scrollbar, and CupertinoScrollbar since everyone is sharing. (Previously only CupertinoScrollbar supported dragging the thumb). It will also enable tapping on the scrollbar track to page the scroll view.
This will also remove the adaptive nature of the Scrollbar widget. I want to separate the CupertinoScrollbar from Scrollbar since as we expand on these widgets and look forward to additional platforms, having special handling in Scrollbar for CupertinoScrollbar was going to get even more complicated. Adaptive widgets like these do not scale well, and are easier to maintain and debug when separated. The new Material Design Scrollbar is very similar to the CupertinoScrollbar, so I am hoping the change will have a minimal effect.
Planned Changes for Scrollbars
(Order may change as these are reviewed and land)
1. Refactor to RawScrollbar
2. Update Scrollbar to match Material Design spec
3. Add track gestures
4. Add code samples, etc.
5. Add ScrollbarTheme
6. Create Scrollbars by default for relevant scrollables on desktop and web
7. ++ There will be more iterations and other bugs/behaviors people have been sending my way. We'll get to these, the above listed items are the primary focus for now. :)
Related Issues
Part of #70866
Fixes #70105
Tests
Existing tests pass after refactoring. :)
Added tests for
Checklist
Before you create this PR, confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change