Skip to content
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

[Material] Unit test for skipping Slider tick mark due to overdensity #28013

Merged
merged 16 commits into from Feb 26, 2019
Merged
14 changes: 7 additions & 7 deletions packages/flutter/lib/src/material/slider.dart
Expand Up @@ -1005,16 +1005,16 @@ class _RenderSlider extends RenderBox {
isEnabled: isInteractive,
sliderTheme: _sliderTheme,
).width;
// If the ticks would be too dense, don't bother painting them.
if ((trackRect.width - tickMarkWidth) / divisions >= 3.0 * tickMarkWidth) {
final double adjustedTrackWidth = trackRect.width - tickMarkWidth;
// If the tick marks would be too dense, don't bother painting them.
if (adjustedTrackWidth / divisions >= 3.0 * tickMarkWidth) {
final double dy = trackRect.center.dy;
for (int i = 0; i <= divisions; i++) {
final double tickValue = i / divisions;
final double value = i / divisions;
// The ticks are mapped to be within the track, so the tick mark width
// must be subtracted from the track width.
final double tickX = trackRect.left +
tickValue * (trackRect.width - tickMarkWidth) + tickMarkWidth / 2;
final double tickY = trackRect.center.dy;
final Offset tickMarkOffset = Offset(tickX, tickY);
final double dx = trackRect.left + value * adjustedTrackWidth + tickMarkWidth / 2;
final Offset tickMarkOffset = Offset(dx, dy);
_sliderTheme.tickMarkShape.paint(
context,
tickMarkOffset,
Expand Down
57 changes: 57 additions & 0 deletions packages/flutter/test/material/slider_test.dart
Expand Up @@ -978,6 +978,63 @@ void main() {
await tester.pumpAndSettle();
});

testWidgets('Tick marks are skipped when they aro too dense', (WidgetTester tester) async {
clocksmith marked this conversation as resolved.
Show resolved Hide resolved
double value = 25.0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not needed for this test

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


Widget buildSlider({
int divisions,
TextDirection textDirection,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This parameter isn't needed

}) {
return Directionality(
textDirection: textDirection,
child: StatefulBuilder(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't need a StatefulBuilder, since we don't actually need to maintain the slider's value.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

builder: (BuildContext context, StateSetter setState) {
return MediaQuery(
data: MediaQueryData.fromWindow(window),
child: Material(
child: Center(
child: Slider(
min: 0.0,
max: 100.0,
divisions: divisions,
value: value,
onChanged: (double newValue) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simplify for this test, since we're not moving the slider:

onChanged: (double value) { },

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

setState(() {
value = newValue;
});
},
),
),
),
);
},
),
);
}

await tester.pumpWidget(
buildSlider(
textDirection: TextDirection.ltr,
divisions: 4,
),
);

final RenderBox sliderBox = tester.firstRenderObject<RenderBox>(find.byType(Slider));

// 5 tick marks and a thumb.
expect(sliderBox, paintsExactlyCountTimes(#drawCircle, 6));

await tester.pumpWidget(
buildSlider(
textDirection: TextDirection.ltr,
divisions: 500,
),
);

// No tick marks because they are too dense, just a thumb.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The failure mode we've seen has divisions = max - min (100). Would be best to duplicate that here to make sure we're not regressing a case that we've seen fail. We can assume that if 100 is too dense 500 will be too :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done and added comment

expect(sliderBox, paintsExactlyCountTimes(#drawCircle, 1));
});

testWidgets('Slider has correct animations when reparented', (WidgetTester tester) async {
final Key sliderKey = GlobalKey(debugLabel: 'A');
double value = 0.0;
Expand Down