Fix `Slider` thumb doesn't align with divisions, thumb padding, and rounded corners #149594

TahaTesser · 2024-06-03T14:14:35Z

fixes [Slider] Thumb's center doesn't align with division's center
fixes Slider thumb doesn't respect round slider track shape
fixes RoundedRectSliderTrackShape corners are not rendered correctly

(Verified these behaviors with Android components implementation)

Code sample

expand to view the code sample

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 _value = 5.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: Slider(
          value: _value,
          // divisions: 10,
          // ignore: avoid_redundant_argument_values
          min: 0,
          max: 10,
          onChanged: (double value) {
            setState(() {
              _value = value;
            });
          },
        ),
      ),
    );
  }
}

Description

This PR fixes several core Sliders issues which are apparent in #147783. As a result, fixing the these bugs will unblock it.

1. Fixes the thumb doesn't align with `Slider` divisions.

2. Fixes `RoundedRectSliderTrackShape` corners are not rendered correctly.

3. Fixes round track shape corners when the thumb is at the start or end of the round track shape.

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
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.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

flutter-dashboard · 2024-06-03T15:59:14Z

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #149594 at sha ffb98aae854a5674a394a9b0978c7ba8b5ea33cd

Piinks · 2024-06-03T18:51:13Z

Pinging to my inbox so it's in my queue. 👍

Piinks · 2024-06-05T20:26:00Z

packages/flutter/lib/src/material/slider.dart

Do we use is a lot in the slider code like this?

Maybe instead of is, RoundedRectSliderTrackShape and others can contains a getter for this?

Excellent idea! Added isRounded getter. Please let me know what do you think.

packages/flutter/lib/src/material/slider_theme.dart

flutter-dashboard · 2024-06-06T12:19:48Z

Golden file changes are available for triage from new commit, Click here to view.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #149594 at sha 8dfa9b4

packages/flutter/lib/src/material/slider_theme.dart

Piinks

LGTM! Thanks for your patience @TahaTesser 🙏

flutter-dashboard · 2024-06-14T00:58:34Z

Golden file changes are available for triage from new commit, Click here to view.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #149594 at sha fe6c6f3

flutter-dashboard · 2024-06-14T11:11:34Z

Golden file changes are available for triage from new commit, Click here to view.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #149594 at sha cff1bd9

TahaTesser · 2024-06-14T13:19:52Z

@Piinks
Rebased and even pushed empty commit, it seems Google testing is failing.

flutter-dashboard · 2024-06-17T13:19:26Z

Golden file changes are available for triage from new commit, Click here to view.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #149594 at sha 687c31d

justinmc

The goldens changes look like they're only due to slight changes in the rounded corners of the track (seems expected).

In the Google tests I see a bunch of visual diff tests related to the position of the thumb and sometimes its label (seems expected), but also I see breakages of classes that extends SliderTrackShape.

justinmc · 2024-06-17T18:02:21Z

packages/flutter/lib/src/material/slider_theme.dart

+  /// Whether the track shape is rounded.
+  ///
+  /// This is used to determine the correct position of the thumb in relation to the track.
+  bool get isRounded;


Is is possible to give a default value here? This will break anyone that extends SliderTrackShape.

flutter-dashboard · 2024-06-18T13:32:46Z

Golden file changes are available for triage from new commit, Click here to view.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #149594 at sha b8f446c

flutter-dashboard · 2024-06-20T15:05:01Z

Golden file changes are available for triage from new commit, Click here to view.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #149594 at sha 2095138

justinmc · 2024-06-20T16:15:57Z

It looks like there are still breakages of classes that use implements with SliderTrackShape because they don't have an implementation of isRounded. I always have to refresh myself on this in Dart, so to be specific:

abstract class Foo {
  bool get isRounded => false;
}

// Error, missing implementation of isRounded (even though Foo has an implementation).
class Bar implements Foo {
}

// No error.
class Baz extends Foo {
}

Some options to move forward here:

Modify this PR so that it does not cause a breakage. It looks doable but I don't know if it would make the code ugly, what do you think?
Keep the PR as-is and migrate all of the broken users, and follow the breaking changes policy. There are 200+ breakages in the Google tests due to this, though I'm guessing the number of places I'd have to add isRounded is not that many. The code change should be straightforward. I think this is doable is we decide to do it.
Keep isRounded, but also make changes to mitigate this kind of breakage in the future. Maybe SliderTrackShape should have been an abstract base class to prevent users from using implements with it (docs on Dart's newish class modifiers)? Maybe we could deprecate SliderTrackShape and move to a new base class without causing an immediate hard breaking change? We would still have to do all of the work from option 2, but this might be better for broken users and better for the long term maintainability of this code.

Piinks · 2024-06-20T16:58:29Z

Hmm, maybe we can add a g3 fix in frob to add the getter for folks that have implements? I've also in the past changed the customer's code toe extends temporarily, or permanently if they agree, to unblock changes like these.

TahaTesser · 2024-06-25T07:21:27Z

Thanks @justinmc!

Modify this PR so that it does not cause a breakage. It looks doable but I don't know if it would make the code ugly, what do you think?

I noticed RoundedRectSliderTrackShape and RectangularSliderTrackShape doesn't override the getPreferredRect from the SliderTrackShape, yet they don't throw an error. This is due to all track shapes using BaseSliderTrackShape mixin with returns a default getPreferredRect.

| updated the PR to return default isRounded in the BaseSliderTrackShape, since the current track shapes are using this mixin, it could be expected the customer's code should be using it too? This solution makes sense since getPreferredRect is already handled this way, and isRounded is just another addition.

Please let me know what do you think.

flutter-dashboard · 2024-06-25T08:10:43Z

Golden file changes are available for triage from new commit, Click here to view.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #149594 at sha 05ee675

…g, and rounded corners (flutter/flutter#149594)

hauvudinh24 · 2024-09-10T09:38:49Z

Hi,
I'm sorry, sir. Will we fix it in the next version of the release?
Thank you.

TahaTesser · 2024-09-10T09:51:54Z

@hauvudinh24!
This seems to be still on the master channel. It should be eventually make it to the next MAJOR stable release.

@OverRide

…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](#159586) This makes a similar fix as the one for `Slider` in #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

github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels Jun 3, 2024

TahaTesser force-pushed the fix_slider_issues branch from b53e452 to ffb98aa Compare June 3, 2024 14:54

TahaTesser mentioned this pull request Jun 3, 2024

Introduce new Slider shapes #147783

Closed

9 tasks

TahaTesser marked this pull request as ready for review June 3, 2024 15:22

flutter-dashboard bot added the will affect goldens Changes to golden files label Jun 3, 2024

Piinks reviewed Jun 5, 2024

View reviewed changes

TahaTesser force-pushed the fix_slider_issues branch from ffb98aa to 8dfa9b4 Compare June 6, 2024 11:49

TahaTesser requested a review from Piinks June 11, 2024 13:43

Piinks reviewed Jun 14, 2024

View reviewed changes

packages/flutter/lib/src/material/slider_theme.dart Outdated Show resolved Hide resolved

Piinks approved these changes Jun 14, 2024

View reviewed changes

TahaTesser force-pushed the fix_slider_issues branch from fe6c6f3 to 98a3c58 Compare June 14, 2024 06:52

TahaTesser force-pushed the fix_slider_issues branch from cff1bd9 to 687c31d Compare June 17, 2024 12:46

justinmc reviewed Jun 17, 2024

View reviewed changes

TahaTesser force-pushed the fix_slider_issues branch from 687c31d to b8f446c Compare June 18, 2024 11:18

TahaTesser requested a review from justinmc June 20, 2024 12:15

TahaTesser force-pushed the fix_slider_issues branch from 2095138 to 05ee675 Compare June 25, 2024 07:21

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 17, 2024

9e88446 Fix Slider thumb doesn't align with divisions, thumb paddin…

2c3dd23

…g, and rounded corners (flutter/flutter#149594)

engine-flutter-autoroll mentioned this pull request Jul 17, 2024

Roll Flutter from 58068d8ea4ec to 49d6d520cb45 (55 revisions) flutter/packages#7150

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 17, 2024

9e88446 Fix Slider thumb doesn't align with divisions, thumb paddin…

9535776

…g, and rounded corners (flutter/flutter#149594)

engine-flutter-autoroll mentioned this pull request Jul 17, 2024

Roll Flutter from 58068d8ea4ec to aeaef9be3505 (56 revisions) flutter/packages#7151

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 17, 2024

9e88446 Fix Slider thumb doesn't align with divisions, thumb paddin…

fdd48c0

…g, and rounded corners (flutter/flutter#149594)

engine-flutter-autoroll mentioned this pull request Jul 17, 2024

Roll Flutter from 58068d8ea4ec to e5d8466006cd (57 revisions) flutter/packages#7152

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 17, 2024

9e88446 Fix Slider thumb doesn't align with divisions, thumb paddin…

2aa1bbe

…g, and rounded corners (flutter/flutter#149594)

engine-flutter-autoroll mentioned this pull request Jul 17, 2024

Roll Flutter from 58068d8ea4ec to 46cb56d313c0 (58 revisions) flutter/packages#7153

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 17, 2024

9e88446 Fix Slider thumb doesn't align with divisions, thumb paddin…

8ba983f

…g, and rounded corners (flutter/flutter#149594)

engine-flutter-autoroll mentioned this pull request Jul 17, 2024

Roll Flutter from 58068d8ea4ec to 1f489de409bd (59 revisions) flutter/packages#7154

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 17, 2024

9e88446 Fix Slider thumb doesn't align with divisions, thumb paddin…

9f6193a

…g, and rounded corners (flutter/flutter#149594)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 17, 2024

9e88446 Fix Slider thumb doesn't align with divisions, thumb paddin…

d2bc410

…g, and rounded corners (flutter/flutter#149594)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 17, 2024

9e88446 Fix Slider thumb doesn't align with divisions, thumb paddin…

c613ff2

…g, and rounded corners (flutter/flutter#149594)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 18, 2024

9e88446 Fix Slider thumb doesn't align with divisions, thumb paddin…

6f03981

…g, and rounded corners (flutter/flutter#149594)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 18, 2024

9e88446 Fix Slider thumb doesn't align with divisions, thumb paddin…

e30050e

…g, and rounded corners (flutter/flutter#149594)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 18, 2024

9e88446 Fix Slider thumb doesn't align with divisions, thumb paddin…

79e74ae

…g, and rounded corners (flutter/flutter#149594)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 18, 2024

9e88446 Fix Slider thumb doesn't align with divisions, thumb paddin…

741176c

…g, and rounded corners (flutter/flutter#149594)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 18, 2024

9e88446 Fix Slider thumb doesn't align with divisions, thumb paddin…

b429f86

…g, and rounded corners (flutter/flutter#149594)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 18, 2024

9e88446 Fix Slider thumb doesn't align with divisions, thumb paddin…

f9ad2ed

…g, and rounded corners (flutter/flutter#149594)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 18, 2024

9e88446 Fix Slider thumb doesn't align with divisions, thumb paddin…

4d236b6

…g, and rounded corners (flutter/flutter#149594)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 19, 2024

9e88446 Fix Slider thumb doesn't align with divisions, thumb paddin…

9306eac

…g, and rounded corners (flutter/flutter#149594)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 19, 2024

9e88446 Fix Slider thumb doesn't align with divisions, thumb paddin…

d073061

…g, and rounded corners (flutter/flutter#149594)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 19, 2024

9e88446 Fix Slider thumb doesn't align with divisions, thumb paddin…

35e5ecb

…g, and rounded corners (flutter/flutter#149594)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 19, 2024

9e88446 Fix Slider thumb doesn't align with divisions, thumb paddin…

b6e8183

…g, and rounded corners (flutter/flutter#149594)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 19, 2024

9e88446 Fix Slider thumb doesn't align with divisions, thumb paddin…

6ea961b

…g, and rounded corners (flutter/flutter#149594)

Piinks mentioned this pull request Jul 26, 2024

Flutter Framework Team: Notable Commits #121415

Closed

TahaTesser mentioned this pull request Dec 4, 2024

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

Merged

9 tasks

Fix Slider thumb doesn't align with divisions, thumb padding, and rounded corners #149594

Fix Slider thumb doesn't align with divisions, thumb padding, and rounded corners #149594

Uh oh!

Conversation

TahaTesser commented Jun 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code sample

Description

1. Fixes the thumb doesn't align with Slider divisions.

2. Fixes RoundedRectSliderTrackShape corners are not rendered correctly.

3. Fixes round track shape corners when the thumb is at the start or end of the round track shape.

Pre-launch Checklist

Uh oh!

flutter-dashboard bot commented Jun 3, 2024

Uh oh!

Piinks commented Jun 3, 2024

Uh oh!

Piinks Jun 5, 2024

Choose a reason for hiding this comment

Uh oh!

Piinks Jun 5, 2024

Choose a reason for hiding this comment

Uh oh!

TahaTesser Jun 6, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

flutter-dashboard bot commented Jun 6, 2024

Uh oh!

Uh oh!

Piinks left a comment

Choose a reason for hiding this comment

Uh oh!

flutter-dashboard bot commented Jun 14, 2024

Uh oh!

flutter-dashboard bot commented Jun 14, 2024

Uh oh!

TahaTesser commented Jun 14, 2024

Uh oh!

flutter-dashboard bot commented Jun 17, 2024

Uh oh!

justinmc left a comment

Choose a reason for hiding this comment

Uh oh!

justinmc Jun 17, 2024

Choose a reason for hiding this comment

Uh oh!

TahaTesser Jun 18, 2024

Choose a reason for hiding this comment

Uh oh!

flutter-dashboard bot commented Jun 18, 2024

Uh oh!

flutter-dashboard bot commented Jun 20, 2024

Uh oh!

justinmc commented Jun 20, 2024

Uh oh!

Piinks commented Jun 20, 2024

Uh oh!

TahaTesser commented Jun 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

flutter-dashboard bot commented Jun 25, 2024

Uh oh!

hauvudinh24 commented Sep 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TahaTesser commented Sep 10, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Fix `Slider` thumb doesn't align with divisions, thumb padding, and rounded corners #149594

Fix `Slider` thumb doesn't align with divisions, thumb padding, and rounded corners #149594

TahaTesser commented Jun 3, 2024 •

edited

Loading

1. Fixes the thumb doesn't align with `Slider` divisions.

2. Fixes `RoundedRectSliderTrackShape` corners are not rendered correctly.

TahaTesser commented Jun 25, 2024 •

edited

Loading

hauvudinh24 commented Sep 10, 2024 •

edited

Loading