Fix RangeSlider thumb doesn't align with divisions, thumb padding, and rounded corners (#159792)

Fixes [`RangeSlider` thumb's center doesn't align with division's center, thumb padding, and rounded corners don't work as expected](https://github.com/flutter/flutter/issues/159586) This makes a similar fix as the one for `Slider` in https://github.com/flutter/flutter/pull/149594. This fix is essential to bring updated Material Design for `RangeSlider`. ### Code sample <details> <summary>expand to view the code sample</summary> ```dart import 'package:flutter/material.dart'; void main() => runApp(const MyApp()); class MyApp extends StatefulWidget { const MyApp({super.key}); @override State<MyApp> createState() => _MyAppState(); } class _MyAppState extends State<MyApp> { double _discreetSliderValue = 0.6; RangeValues _discreteRangeSliderValues = const RangeValues(0.2, 1.0); @override Widget build(BuildContext context) { return MaterialApp( debugShowCheckedModeBanner: false, theme: ThemeData( sliderTheme: const SliderThemeData( trackHeight: 32, thumbColor: Colors.green, activeTrackColor: Colors.deepPurple, inactiveTrackColor: Colors.amber, ), ), home: Scaffold( body: Center( child: Column( mainAxisAlignment: MainAxisAlignment.center, children: [ Text( 'Discrete Slider', style: Theme.of(context).textTheme.titleMedium, ), Slider( value: _discreetSliderValue, divisions: 5, onChanged: (double newValue) { setState(() { _discreetSliderValue = newValue; }); }), Text( 'Discrete Range Slider', style: Theme.of(context).textTheme.titleMedium, ), RangeSlider( values: _discreteRangeSliderValues, divisions: 5, onChanged: (RangeValues newValues) { setState(() { _discreteRangeSliderValues = newValues; }); }, ), ], ), ), ), ); } } ``` </details> ### Before <img width="701" alt="Screenshot 2024-12-02 at 18 57 03" src="https://github.com/user-attachments/assets/62d85476-87fd-48e9-aaa9-42d7629d4808"> ### After <img width="701" alt="Screenshot 2024-12-02 at 18 57 21" src="https://github.com/user-attachments/assets/36f136d1-a759-4b11-b0a9-8cb6b54b8573"> ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [ ] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord].  [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
2024-12-09 21:24:20 +02:00
parent 5f861d5bf7
commit f7871352fb
4 changed files with 103 additions and 42 deletions
--- a/packages/flutter/lib/src/material/range_slider.dart
+++ b/packages/flutter/lib/src/material/range_slider.dart
@@ -1402,8 +1402,32 @@ class _RenderRangeSlider extends RenderBox with RelayoutWhenSystemFontsChangeMix
        sliderTheme: _sliderTheme,
        isDiscrete: isDiscrete,
    );
-    _startThumbCenter = Offset(trackRect.left + startVisualPosition * trackRect.width, trackRect.center.dy);
-    _endThumbCenter = Offset(trackRect.left + endVisualPosition * trackRect.width, trackRect.center.dy);
+    final double padding = isDiscrete || _sliderTheme.rangeTrackShape!.isRounded ? trackRect.height : 0.0;
+    final double thumbYOffset = trackRect.center.dy;
+    final double startThumbPosition = isDiscrete
+      ? trackRect.left + startVisualPosition * (trackRect.width - padding) + padding / 2
+      : trackRect.left + startVisualPosition * trackRect.width;
+    final double endThumbPosition = isDiscrete
+      ? trackRect.left + endVisualPosition * (trackRect.width - padding) + padding / 2
+      : trackRect.left + endVisualPosition * trackRect.width;
+    final Size thumbPreferredSize = _sliderTheme.rangeThumbShape!.getPreferredSize(isEnabled, isDiscrete);
+    final double thumbPadding = (padding > thumbPreferredSize.width / 2 ? padding / 2 : 0);
+    _startThumbCenter = Offset(
+      clampDouble(
+        startThumbPosition,
+        trackRect.left + thumbPadding,
+        trackRect.right - thumbPadding,
+      ),
+      thumbYOffset,
+    );
+    _endThumbCenter = Offset(
+      clampDouble(
+        endThumbPosition,
+        trackRect.left + thumbPadding,
+        trackRect.right - thumbPadding,
+      ),
+      thumbYOffset,
+    );
    if (isEnabled) {
      final Size overlaySize = sliderTheme.overlayShape!.getPreferredSize(isEnabled, false);
      overlayStartRect = Rect.fromCircle(center: _startThumbCenter, radius: overlaySize.width / 2.0);
@@ -1766,7 +1790,6 @@ class _RenderRangeSlider extends RenderBox with RelayoutWhenSystemFontsChangeMix
  }
 }

-
 class _ValueIndicatorRenderObjectWidget extends LeafRenderObjectWidget {
  const _ValueIndicatorRenderObjectWidget({
    required this.state,
--- a/packages/flutter/lib/src/material/slider_theme.dart
+++ b/packages/flutter/lib/src/material/slider_theme.dart
@@ -1576,6 +1576,10 @@ abstract class RangeSliderTrackShape {
    bool isDiscrete = false,
    required TextDirection textDirection,
  });
+
+  /// Whether the track shape is rounded. This is used to determine the correct
+  /// position of the thumbs in relation to the track. Defaults to false.
+  bool get isRounded => false;
 }

 /// Base track shape that provides an implementation of [getPreferredRect] for
@@ -2106,15 +2110,6 @@ class RoundedRectRangeSliderTrackShape extends RangeSliderTrackShape with BaseRa
      ),
      inactivePaint,
    );
-    context.canvas.drawRect(
-      Rect.fromLTRB(
-        leftThumbOffset.dx,
-        trackRect.top - (additionalActiveTrackHeight / 2),
-        rightThumbOffset.dx,
-        trackRect.bottom + (additionalActiveTrackHeight / 2),
-      ),
-      activePaint,
-    );
    context.canvas.drawRRect(
      RRect.fromLTRBAndCorners(
        rightThumbOffset.dx,
@@ -2126,7 +2121,20 @@ class RoundedRectRangeSliderTrackShape extends RangeSliderTrackShape with BaseRa
      ),
      inactivePaint,
    );
+    context.canvas.drawRRect(
+      RRect.fromLTRBR(
+        leftThumbOffset.dx - (sliderTheme.trackHeight! / 2),
+        trackRect.top - (additionalActiveTrackHeight / 2),
+        rightThumbOffset.dx + (sliderTheme.trackHeight! / 2),
+        trackRect.bottom + (additionalActiveTrackHeight / 2),
+        trackRadius,
+      ),
+      activePaint,
+    );
  }
+
+  @override
+  bool get isRounded => true;
 }

 /// The default shape of each [Slider] tick mark.
--- a/packages/flutter/test/material/range_slider_test.dart
+++ b/packages/flutter/test/material/range_slider_test.dart
@@ -1113,8 +1113,8 @@ void main() {
      sliderBox,
      paints
        ..rrect(color: sliderTheme.inactiveTrackColor)
-        ..rect(color: sliderTheme.activeTrackColor)
-        ..rrect(color: sliderTheme.inactiveTrackColor),
+        ..rrect(color: sliderTheme.inactiveTrackColor)
+        ..rrect(color: sliderTheme.activeTrackColor),
    );
    expect(
      sliderBox,
@@ -1142,8 +1142,8 @@ void main() {
      sliderBox,
      paints
        ..rrect(color: sliderTheme.inactiveTrackColor)
-        ..rect(color: activeColor)
-        ..rrect(color: sliderTheme.inactiveTrackColor),
+        ..rrect(color: sliderTheme.inactiveTrackColor)
+        ..rrect(color: activeColor),
    );
    expect(
      sliderBox,
@@ -1170,8 +1170,8 @@ void main() {
      sliderBox,
      paints
        ..rrect(color: inactiveColor)
-        ..rect(color: sliderTheme.activeTrackColor)
-        ..rrect(color: inactiveColor),
+        ..rrect(color: inactiveColor)
+        ..rrect(color: sliderTheme.activeTrackColor),
    );
    expect(
      sliderBox,
@@ -1202,8 +1202,8 @@ void main() {
      sliderBox,
      paints
        ..rrect(color: inactiveColor)
-        ..rect(color: activeColor)
-        ..rrect(color: inactiveColor),
+        ..rrect(color: inactiveColor)
+        ..rrect(color: activeColor),
    );
    expect(
      sliderBox,
@@ -1229,8 +1229,8 @@ void main() {
      sliderBox,
      paints
        ..rrect(color: sliderTheme.inactiveTrackColor)
-        ..rect(color: sliderTheme.activeTrackColor)
-        ..rrect(color: sliderTheme.inactiveTrackColor),
+        ..rrect(color: sliderTheme.inactiveTrackColor)
+        ..rrect(color: sliderTheme.activeTrackColor),
    );
    expect(
      sliderBox,
@@ -1267,8 +1267,8 @@ void main() {
      sliderBox,
      paints
        ..rrect(color: inactiveColor)
-        ..rect(color: activeColor)
-        ..rrect(color: inactiveColor),
+        ..rrect(color: inactiveColor)
+        ..rrect(color: activeColor),
    );
    expect(
      sliderBox,
@@ -1300,8 +1300,8 @@ void main() {
      sliderBox,
      paints
        ..rrect(color: sliderTheme.disabledInactiveTrackColor)
-        ..rect(color: sliderTheme.disabledActiveTrackColor)
-        ..rrect(color: sliderTheme.disabledInactiveTrackColor),
+        ..rrect(color: sliderTheme.disabledInactiveTrackColor)
+        ..rrect(color: sliderTheme.disabledActiveTrackColor),
    );
    expect(sliderBox, isNot(paints..circle(color: sliderTheme.thumbColor)));
    expect(sliderBox, isNot(paints..rect(color: sliderTheme.activeTrackColor)));
@@ -1327,8 +1327,8 @@ void main() {
      sliderBox,
      paints
        ..rrect(color: sliderTheme.disabledInactiveTrackColor)
-        ..rect(color: sliderTheme.disabledActiveTrackColor)
-        ..rrect(color: sliderTheme.disabledInactiveTrackColor),
+        ..rrect(color: sliderTheme.disabledInactiveTrackColor)
+        ..rrect(color: sliderTheme.disabledActiveTrackColor),
    );
    expect(sliderBox, isNot(paints..circle(color: sliderTheme.thumbColor)));
    expect(sliderBox, isNot(paints..rect(color: sliderTheme.activeTrackColor)));
@@ -2080,10 +2080,10 @@ void main() {
      paints
        // left inactive track RRect
        ..rrect(rrect: RRect.fromLTRBAndCorners(-24.0, 3.0, -12.0, 7.0, topLeft: const Radius.circular(2.0), bottomLeft: const Radius.circular(2.0)))
-        // active track RRect
-        ..rect(rect: const Rect.fromLTRB(-12.0, 2.0, 0.0, 8.0))
        // right inactive track RRect
        ..rrect(rrect: RRect.fromLTRBAndCorners(0.0, 3.0, 24.0, 7.0, topRight: const Radius.circular(2.0), bottomRight: const Radius.circular(2.0)))
+        // active track RRect
+        ..rrect(rrect: RRect.fromLTRBR(-14.0, 2.0, 2.0, 8.0, const Radius.circular(2.0)))
        // thumbs
        ..circle(x: -12.0, y: 5.0, radius: 10.0)
        ..circle(x: 0.0, y: 5.0, radius: 10.0),
@@ -2161,22 +2161,41 @@ void main() {
    await tester.pumpWidget(buildFrame(15));
    await tester.pumpAndSettle(); // Finish the animation.

-    late Rect activeTrackRect;
-    expect(renderObject, paints..something((Symbol method, List<dynamic> arguments) {
-      if (method != #drawRect) {
+    late RRect activeTrackRRect;
+    expect(renderObject, paints
+      ..rrect()
+      ..rrect()
+      ..something((Symbol method, List<dynamic> arguments) {
+      if (method != #drawRRect) {
        return false;
      }
-      activeTrackRect = arguments[0] as Rect;
+      activeTrackRRect = arguments[0] as RRect;
      return true;
    }));

+    const double padding = 4.0;
    // The 1st thumb should at one-third(5 / 15) of the Slider.
    // The 2nd thumb should at (8 / 15) of the Slider.
    // The left of the active track shape is the position of the 1st thumb.
    // The right of the active track shape is the position of the 2nd thumb.
-    // 24.0 is the default margin, (800.0 - 24.0 - 24.0) is the slider's width.
-    expect(nearEqual(activeTrackRect.left, (800.0 - 24.0 - 24.0) * (5 / 15) + 24.0, 0.01), true);
-    expect(nearEqual(activeTrackRect.right, (800.0 - 24.0 - 24.0) * (8 / 15) + 24.0, 0.01), true);
+    // 24.0 is the default margin, (800.0 - 24.0 - 24.0 - padding) is the slider's width.
+    // Where the padding value equals to the track height.
+    expect(
+      nearEqual(
+        activeTrackRRect.left,
+        (800.0 - 24.0 - 24.0 - padding) * (5 / 15) + 24.0,
+        0.01,
+      ),
+      true,
+    );
+    expect(
+      nearEqual(
+        activeTrackRRect.right,
+        (800.0 - 24.0 - 24.0 - padding) * (8 / 15) + 24.0 + padding,
+        0.01,
+      ),
+      true,
+    );
  });

  testWidgets('RangeSlider changes mouse cursor when hovered', (WidgetTester tester) async {
--- a/packages/flutter/test/material/slider_theme_test.dart
+++ b/packages/flutter/test/material/slider_theme_test.dart
@@ -1533,8 +1533,6 @@ void main() {
          topLeft: const Radius.circular(2.0),
          bottomLeft: const Radius.circular(2.0),
        ))
-      // active track RRect Start 10 pixels from left screen.
-        ..rect(rect:const Rect.fromLTRB(10.0, 297.0, 790.0, 303.0),)
        // inactive track RRect. Ends 10 pixels from right of screen.
        ..rrect(rrect: RRect.fromLTRBAndCorners(
          790.0,
@@ -1544,6 +1542,11 @@ void main() {
          topRight: const Radius.circular(2.0),
          bottomRight: const Radius.circular(2.0),
        ))
+        // active track RRect Start 10 pixels from left screen.
+        ..rrect(rrect: RRect.fromLTRBR(
+          8.0, 297.0, 792.0, 303.0,
+          const Radius.circular(2.0))
+        )
        // The thumb Left.
        ..circle(x: 10.0, y: 300.0, radius: 10.0)
        // The thumb Right.
@@ -1806,12 +1809,15 @@ void main() {
            topLeft: const Radius.circular(2.0),
            bottomLeft: const Radius.circular(2.0),
          ))
-          ..rect(rect: const Rect.fromLTRB(24.0, 297.0, 24.0, 303.0))
          ..rrect(rrect: RRect.fromLTRBAndCorners(
            24.0, 298.0, 776.0, 302.0,
            topRight: const Radius.circular(2.0),
            bottomRight: const Radius.circular(2.0),
          ))
+          ..rrect(rrect: RRect.fromLTRBR(
+            22.0, 297.0, 26.0, 303.0,
+            const Radius.circular(2.0)),
+          )
          ..circle(x: 24.0, y: 300.0)
          ..shadow(elevation: 1.0)
          ..circle(x: 24.0, y: 300.0)
@@ -1855,12 +1861,15 @@ void main() {
          topLeft: const Radius.circular(2.0),
          bottomLeft: const Radius.circular(2.0),
        ))
-        ..rect(rect: const Rect.fromLTRB(24.0, 297.0, 24.0, 303.0))
        ..rrect(rrect: RRect.fromLTRBAndCorners(
          24.0, 298.0, 776.0, 302.0,
          topRight: const Radius.circular(2.0),
          bottomRight: const Radius.circular(2.0),
        ))
+        ..rrect(rrect: RRect.fromLTRBR(
+          22.0, 297.0, 26.0, 303.0,
+          const Radius.circular(2)),
+        )
        ..circle(x: 24.0, y: 300.0)
        ..path(strokeWidth: 1.0 * 2.0, color: Colors.black)
        ..circle(x: 24.0, y: 300.0)
@@ -2554,9 +2563,11 @@ void main() {
    });
  });

-  testWidgets('SliderTrackShape isRounded defaults', (WidgetTester tester) async {
+  testWidgets('Track shape isRounded defaults', (WidgetTester tester) async {
    expect(const RectangularSliderTrackShape().isRounded, isFalse);
    expect(const RoundedRectSliderTrackShape().isRounded, isTrue);
+    expect(const RectangularRangeSliderTrackShape().isRounded, isFalse);
+    expect(const RoundedRectRangeSliderTrackShape().isRounded, isTrue);
  });

  testWidgets('SliderThemeData.padding can override the default Slider padding', (WidgetTester tester) async {