Repository navigation
[Material] Simple API for skipping over certain Slider shapes - #27613
Conversation
…d tick mark painting
zoechi
added
framework
…er-shape-empty
HansMuller
left a comment
HansMuller
left a comment
There was a problem hiding this comment.
This looks good. The gist of the feedback is to make the empty shape classes private and make the connection between the noFoo shapes and the SliderThemeData properties clear.
| /// [RoundSliderOverlayShape] for examples. | ||
| /// | ||
| /// The track painting can be skipped by specifying [trackHeight] as 0. | ||
| /// The thumb painting can be skipped by specifying |
There was a problem hiding this comment.
You need to identify which properties are to be set in each case.
| TextDirection textDirection, | ||
| }); | ||
|
|
||
| /// Special instance of [SliderTickMarkShape] to skip the thumb painting. |
There was a problem hiding this comment.
Here's where a "See also" section that points to the SliderThemeData tick mark property would be helpful
There was a problem hiding this comment.
Added here and in the other 2 special instance docs (thumb, overlay)
There was a problem hiding this comment.
Done for all 3 shapes
…utter into slider-shape-empty
|
Updated. Need a second look at new dartdoc added |
johnsonmh
left a comment
johnsonmh
left a comment
There was a problem hiding this comment.
LGTM with minor comments.
| await tester.pumpAndSettle(); | ||
|
|
||
| // Now that there is a tap, and the value indicator is specified to be | ||
| // , shown an path is expected to be painted. |
There was a problem hiding this comment.
Not totally sure what this supposed to say, but I think there is a typo on the second line.
There was a problem hiding this comment.
was a typo, updated
rami-a
left a comment
rami-a
left a comment
There was a problem hiding this comment.
LGTM, just some minor comments
| // | ||
| // The value indicator can be skipped by passing the appropriate | ||
| // [ShowValueIndicator]. | ||
| testWidgets('The slider shape paintings can be variously skipped or removed', (WidgetTester tester) async { |
There was a problem hiding this comment.
Nit/Optional: I would suggest breaking this into multiple smaller tests to make it easier to follow.
| await tester.pumpAndSettle(); | ||
|
|
||
| // 1 value indicator. | ||
| expect(sliderBox, paintsExactlyCountTimes(#drawPath, 1)); |
There was a problem hiding this comment.
I think we can strengthen the tests a bit by also checking that 0 circles/rects were painted. Like-wise for the other tests too
HansMuller
left a comment
HansMuller
left a comment
There was a problem hiding this comment.
LGTM with a few minor changes.
…r#27613) * Provide a simmplified API for skipping over slider thumb, overlay, and tick mark painting * doc fixes * comments * comments * comments * comments * comments * analyzer * comments
6 participants
Make it possible to selectively disable the visuals for the slider's track, thumb, overlay (expands around the thumb when it's pressed), tick marks, and value indicator.
SliderThemeData.trackHeightto 0.SliderThemeData.thumbShapetoSliderComponentShape.noThumb.SliderThemeData.overlayShapetoSliderComponentShape.noOverlay.SliderThemeData.tickMarkShapetoSliderTickMarkShape.noTickMark.SliderThemeData.showValueIndicatortoShowValueIndicator.never.