Slider: add themeable mouse cursor#88425
Conversation
HansMuller
left a comment
There was a problem hiding this comment.
Looks good, just needs a small tweak.
|
This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
HansMuller
left a comment
There was a problem hiding this comment.
This looks good; please consider adding support for MaterialState.dragged.
| /// | ||
| /// If this property is null, [MaterialStateMouseCursor.clickable] will be used. | ||
| /// If null, then the value of [SliderThemeData.mouseCursor] is used. If that | ||
| /// is also null, then [MaterialStateMouseCursor.clickable] is used. |
There was a problem hiding this comment.
Even though it's implicitly mentioned above, it would be helpful to include a "see also" section that points at MaterialStateMouseCursor
| }, | ||
| ); | ||
| final Set<MaterialState> states = <MaterialState>{ | ||
| if (!_enabled) MaterialState.disabled, |
There was a problem hiding this comment.
It would be useful to also include support for MaterialState.dragged, as Scrollbar does
|
I'm closing this PR in favor of #96623 which is essentially the same changes. |
Allow themes to override Slider's mouse cursor.
Partial fix to #88371
Pre-launch Checklist
///).