adds trackRadius to ScrollbarPainter and RawScrollbar#98018
adds trackRadius to ScrollbarPainter and RawScrollbar#98018fluttergithubbot merged 3 commits intoflutter:masterfrom
trackRadius to ScrollbarPainter and RawScrollbar#98018Conversation
5090253 to
01be575
Compare
|
Should this PR add |
| double crossAxisMargin = 0.0, | ||
| Radius? radius, | ||
| Radius? trackRadius, | ||
| OutlinedBorder? shape, |
There was a problem hiding this comment.
I believe trackShape should also be added
There was a problem hiding this comment.
Just in case someone wants a very custom scrollbar. For my use case, only trackRadius should all, but, since in Flutter we should have control over every pixel on the screen, I believe trackShape is a good parameter for this widget.
We already have shape for the thumb, why not trackShape?
There was a problem hiding this comment.
Our engineering philosophy specifies
Avoid implementing features you don’t need.
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#lazy-programming
Asking why not ...? is a slippery slope to a large and complicated API. Imagine a Scrollbar with 100 different parameters... 😵💫
You are right, in Flutter you can control every pixel on the screen, that does not mean we need to do so in the framework. You can certainly extend the scrollbar class in your own code, or even publish a package. There are a number of scrollbar packages on pub.dev with a variety of special features.
The framework focuses on providing fundamental core UI features, but anyone can change and modify as they like in their own project. You are very right that you can control every pixel as you desire. :)
There was a problem hiding this comment.
Indeed, it is not necessary for my use case. @werainkhatri what do you think?
There was a problem hiding this comment.
i agree @Piinks' point. we may add it in the future if a user has a specific use case. i'll go ahead and remove this change.
c6119b4 to
36c7b68
Compare
Piinks
left a comment
There was a problem hiding this comment.
LGTM, thanks for another contribution @werainkhatri!
Fixes #97876
Pre-launch Checklist
///).