-
Notifications
You must be signed in to change notification settings - Fork 29.7k
adds trackRadius to ScrollbarPainter and RawScrollbar
#98018
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
adds trackRadius to ScrollbarPainter and RawScrollbar
#98018
Conversation
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe trackShape should also be added
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our engineering philosophy specifies
Avoid implementing features you don’t need.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, it is not necessary for my use case. @werainkhatri what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests are now failing @werainkhatri
c6119b4 to
36c7b68
Compare
Piinks
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for another contribution @werainkhatri!
Fixes #97876
Pre-launch Checklist
///).