From 3172e67085d3d02a269dafdd43d49be1de0edbe9 Mon Sep 17 00:00:00 2001 From: Anthony Robledo Date: Thu, 14 Feb 2019 18:39:59 -0500 Subject: [PATCH 01/12] Do not draw tick marks if they are too dense --- packages/flutter/lib/src/material/slider.dart | 37 ++++++++++--------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/packages/flutter/lib/src/material/slider.dart b/packages/flutter/lib/src/material/slider.dart index c3f59516e1f79e..c938999d5695bf 100644 --- a/packages/flutter/lib/src/material/slider.dart +++ b/packages/flutter/lib/src/material/slider.dart @@ -1005,23 +1005,26 @@ class _RenderSlider extends RenderBox { isEnabled: isInteractive, sliderTheme: _sliderTheme, ).width; - for (int i = 0; i <= divisions; i++) { - final double tickValue = 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); - _sliderTheme.tickMarkShape.paint( - context, - tickMarkOffset, - parentBox: this, - sliderTheme: _sliderTheme, - enableAnimation: _enableAnimation, - textDirection: _textDirection, - thumbCenter: thumbCenter, - isEnabled: isInteractive, - ); + if ((trackRect.width - tickMarkWidth) / divisions >= 3.0 * tickMarkWidth) { + for (int i = 0; i <= divisions; i++) { + final double tickValue = 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); + _sliderTheme.tickMarkShape.paint( + context, + tickMarkOffset, + parentBox: this, + sliderTheme: _sliderTheme, + enableAnimation: _enableAnimation, + textDirection: _textDirection, + thumbCenter: thumbCenter, + isEnabled: isInteractive, + ); + } } } From dd2aa5a06a19c03b3bfa01bde68cedac5832c371 Mon Sep 17 00:00:00 2001 From: Anthony Robledo Date: Thu, 14 Feb 2019 18:44:45 -0500 Subject: [PATCH 02/12] Do not draw tick marks if they are too dense --- packages/flutter/lib/src/material/slider.dart | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/flutter/lib/src/material/slider.dart b/packages/flutter/lib/src/material/slider.dart index c938999d5695bf..9408bbac6af2de 100644 --- a/packages/flutter/lib/src/material/slider.dart +++ b/packages/flutter/lib/src/material/slider.dart @@ -1005,6 +1005,7 @@ 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) { for (int i = 0; i <= divisions; i++) { final double tickValue = i / divisions; From 1a9cb3ebd2c6ef504830ecf34a4284543ccb4247 Mon Sep 17 00:00:00 2001 From: Anthony Robledo Date: Fri, 15 Feb 2019 14:05:23 -0500 Subject: [PATCH 03/12] formalize fix and add test --- packages/flutter/lib/src/material/slider.dart | 14 ++--- .../flutter/test/material/slider_test.dart | 57 +++++++++++++++++++ 2 files changed, 64 insertions(+), 7 deletions(-) diff --git a/packages/flutter/lib/src/material/slider.dart b/packages/flutter/lib/src/material/slider.dart index 9408bbac6af2de..50f12cc91b7d7b 100644 --- a/packages/flutter/lib/src/material/slider.dart +++ b/packages/flutter/lib/src/material/slider.dart @@ -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, diff --git a/packages/flutter/test/material/slider_test.dart b/packages/flutter/test/material/slider_test.dart index 49cd304e4b2b6f..c899d01e17cc33 100644 --- a/packages/flutter/test/material/slider_test.dart +++ b/packages/flutter/test/material/slider_test.dart @@ -978,6 +978,63 @@ void main() { await tester.pumpAndSettle(); }); + testWidgets('Tick marks are skipped when they aro too dense', (WidgetTester tester) async { + double value = 25.0; + + Widget buildSlider({ + int divisions, + TextDirection textDirection, + }) { + return Directionality( + textDirection: textDirection, + child: StatefulBuilder( + 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) { + setState(() { + value = newValue; + }); + }, + ), + ), + ), + ); + }, + ), + ); + } + + await tester.pumpWidget( + buildSlider( + textDirection: TextDirection.ltr, + divisions: 4, + ), + ); + + final RenderBox sliderBox = tester.firstRenderObject(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. + 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; From 91f9deec2444bd56b945177c131a748f28c9280f Mon Sep 17 00:00:00 2001 From: Anthony Robledo Date: Fri, 15 Feb 2019 14:28:55 -0500 Subject: [PATCH 04/12] typo --- packages/flutter/test/material/slider_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/flutter/test/material/slider_test.dart b/packages/flutter/test/material/slider_test.dart index c899d01e17cc33..5982a935e874a4 100644 --- a/packages/flutter/test/material/slider_test.dart +++ b/packages/flutter/test/material/slider_test.dart @@ -978,7 +978,7 @@ void main() { await tester.pumpAndSettle(); }); - testWidgets('Tick marks are skipped when they aro too dense', (WidgetTester tester) async { + testWidgets('Tick marks are skipped when they are too dense', (WidgetTester tester) async { double value = 25.0; Widget buildSlider({ From 0108da8b648e28038e569a73c38668be650ad565 Mon Sep 17 00:00:00 2001 From: Anthony Robledo Date: Fri, 22 Feb 2019 10:46:21 -0500 Subject: [PATCH 05/12] comments --- .../flutter/test/material/slider_test.dart | 105 ++++++++---------- 1 file changed, 49 insertions(+), 56 deletions(-) diff --git a/packages/flutter/test/material/slider_test.dart b/packages/flutter/test/material/slider_test.dart index e7a56829009abc..3076f537ab3792 100644 --- a/packages/flutter/test/material/slider_test.dart +++ b/packages/flutter/test/material/slider_test.dart @@ -28,17 +28,17 @@ class LoggingThumbShape extends SliderComponentShape { void paint( PaintingContext context, Offset thumbCenter, { - Animation activationAnimation, - Animation enableAnimation, - bool isEnabled, - bool isDiscrete, - bool onActiveTrack, - TextPainter labelPainter, - RenderBox parentBox, - SliderThemeData sliderTheme, - TextDirection textDirection, - double value, - }) { + Animation activationAnimation, + Animation enableAnimation, + bool isEnabled, + bool isDiscrete, + bool onActiveTrack, + TextPainter labelPainter, + RenderBox parentBox, + SliderThemeData sliderTheme, + TextDirection textDirection, + double value, + }) { log.add(thumbCenter); final Paint thumbPaint = Paint()..color = Colors.red; context.canvas.drawCircle(thumbCenter, 5.0, thumbPaint); @@ -558,8 +558,8 @@ void main() { final ValueChanged onChanged = !enabled ? null : (double d) { - value = d; - }; + value = d; + }; return Directionality( textDirection: TextDirection.ltr, child: MediaQuery( @@ -979,42 +979,32 @@ void main() { }); testWidgets('Tick marks are skipped when they are too dense', (WidgetTester tester) async { - double value = 25.0; - Widget buildSlider({ int divisions, - TextDirection textDirection, }) { return Directionality( - textDirection: textDirection, - child: StatefulBuilder( - 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) { - setState(() { - value = newValue; - }); - }, - ), - ), + textDirection: TextDirection.ltr, + child: MediaQuery( + data: MediaQueryData.fromWindow(window), + child: Material( + child: Center( + child: Slider( + min: 0.0, + max: 100.0, + divisions: divisions, + value: 0.25, + onChanged: (double newValue) { }, ), - ); - }, + ), + ), ), ); } + // Pump a slider with a reasonable amount of divisions to verify that the + // tick marks are drawn when the number of tick marks is not too dense. await tester.pumpWidget( buildSlider( - textDirection: TextDirection.ltr, divisions: 4, ), ); @@ -1024,14 +1014,17 @@ void main() { // 5 tick marks and a thumb. expect(sliderBox, paintsExactlyCountTimes(#drawCircle, 6)); + // 100 is the min value, 0, subtracted from the max value, 100. + // This translates to attempting to put a tick mark for every integer value, + // which would be too dense to draw. await tester.pumpWidget( buildSlider( - textDirection: TextDirection.ltr, - divisions: 500, + divisions: 100, ), ); - // No tick marks because they are too dense, just a thumb. + // No tick marks are drawn because they are too dense, but the thumb is + // still drawn. expect(sliderBox, paintsExactlyCountTimes(#drawCircle, 1)); }); @@ -1249,21 +1242,21 @@ void main() { ); expect( - semantics, - hasSemantics( - TestSemantics.root(children: [ - TestSemantics.rootChild( - id: 2, - value: '50%', - increasedValue: '60%', - decreasedValue: '40%', - textDirection: TextDirection.ltr, - actions: SemanticsAction.decrease.index | SemanticsAction.increase.index, - ), - ]), - ignoreRect: true, - ignoreTransform: true, - )); + semantics, + hasSemantics( + TestSemantics.root(children: [ + TestSemantics.rootChild( + id: 2, + value: '50%', + increasedValue: '60%', + decreasedValue: '40%', + textDirection: TextDirection.ltr, + actions: SemanticsAction.decrease.index | SemanticsAction.increase.index, + ), + ]), + ignoreRect: true, + ignoreTransform: true, + )); semantics.dispose(); }); From 49e08e7fe25724d8aa67a42e19901027f44cedcc Mon Sep 17 00:00:00 2001 From: Anthony Robledo Date: Fri, 22 Feb 2019 16:38:47 -0500 Subject: [PATCH 06/12] update From 490f509554c9769af07874696c20c7fbf1487a4f Mon Sep 17 00:00:00 2001 From: Anthony Robledo Date: Fri, 22 Feb 2019 16:41:40 -0500 Subject: [PATCH 07/12] formatting --- .../flutter/test/material/slider_test.dart | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/packages/flutter/test/material/slider_test.dart b/packages/flutter/test/material/slider_test.dart index 3076f537ab3792..c4a24b7d282dba 100644 --- a/packages/flutter/test/material/slider_test.dart +++ b/packages/flutter/test/material/slider_test.dart @@ -28,17 +28,17 @@ class LoggingThumbShape extends SliderComponentShape { void paint( PaintingContext context, Offset thumbCenter, { - Animation activationAnimation, - Animation enableAnimation, - bool isEnabled, - bool isDiscrete, - bool onActiveTrack, - TextPainter labelPainter, - RenderBox parentBox, - SliderThemeData sliderTheme, - TextDirection textDirection, - double value, - }) { + Animation activationAnimation, + Animation enableAnimation, + bool isEnabled, + bool isDiscrete, + bool onActiveTrack, + TextPainter labelPainter, + RenderBox parentBox, + SliderThemeData sliderTheme, + TextDirection textDirection, + double value, + }) { log.add(thumbCenter); final Paint thumbPaint = Paint()..color = Colors.red; context.canvas.drawCircle(thumbCenter, 5.0, thumbPaint); From 7e04ca073df86e8a9477186879fa95b3854e1dad Mon Sep 17 00:00:00 2001 From: Anthony Robledo Date: Fri, 22 Feb 2019 17:06:29 -0500 Subject: [PATCH 08/12] fix test from 100 divisions to 200 divisions --- packages/flutter/test/material/slider_test.dart | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/flutter/test/material/slider_test.dart b/packages/flutter/test/material/slider_test.dart index c4a24b7d282dba..3cc29abc255a0b 100644 --- a/packages/flutter/test/material/slider_test.dart +++ b/packages/flutter/test/material/slider_test.dart @@ -1014,12 +1014,11 @@ void main() { // 5 tick marks and a thumb. expect(sliderBox, paintsExactlyCountTimes(#drawCircle, 6)); - // 100 is the min value, 0, subtracted from the max value, 100. - // This translates to attempting to put a tick mark for every integer value, + // 200 divisions will produce a tick interval off less than 6, // which would be too dense to draw. await tester.pumpWidget( buildSlider( - divisions: 100, + divisions: 200, ), ); From 72eb39489f5fb57bea93fc4451c9141374d30e32 Mon Sep 17 00:00:00 2001 From: Anthony Robledo Date: Mon, 25 Feb 2019 13:40:16 -0500 Subject: [PATCH 09/12] run tests From f9512a9c45480ca4f43b2c59c6e3f632a2fc100f Mon Sep 17 00:00:00 2001 From: Anthony Robledo Date: Mon, 25 Feb 2019 13:44:59 -0500 Subject: [PATCH 10/12] formatting --- .../flutter/test/material/slider_test.dart | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/packages/flutter/test/material/slider_test.dart b/packages/flutter/test/material/slider_test.dart index 3cc29abc255a0b..3cf2a86ec3bbe0 100644 --- a/packages/flutter/test/material/slider_test.dart +++ b/packages/flutter/test/material/slider_test.dart @@ -1241,21 +1241,21 @@ void main() { ); expect( - semantics, - hasSemantics( - TestSemantics.root(children: [ - TestSemantics.rootChild( - id: 2, - value: '50%', - increasedValue: '60%', - decreasedValue: '40%', - textDirection: TextDirection.ltr, - actions: SemanticsAction.decrease.index | SemanticsAction.increase.index, - ), - ]), - ignoreRect: true, - ignoreTransform: true, - )); + semantics, + hasSemantics( + TestSemantics.root(children: [ + TestSemantics.rootChild( + id: 2, + value: '50%', + increasedValue: '60%', + decreasedValue: '40%', + textDirection: TextDirection.ltr, + actions: SemanticsAction.decrease.index | SemanticsAction.increase.index, + ), + ]), + ignoreRect: true, + ignoreTransform: true, + )); semantics.dispose(); }); From be87bbf40b32e293187009eb34f323a48e43179f Mon Sep 17 00:00:00 2001 From: Anthony Robledo Date: Mon, 25 Feb 2019 17:11:36 -0500 Subject: [PATCH 11/12] re run tests From 4c12c03ab60fdfa7f195093f7b605563641792c1 Mon Sep 17 00:00:00 2001 From: Anthony Robledo Date: Tue, 26 Feb 2019 11:26:06 -0500 Subject: [PATCH 12/12] re run build