Skip to content

Conversation

@TahaTesser
Copy link
Member

@TahaTesser TahaTesser commented May 3, 2024

fixes Update Slider for Material 3 redesign

Description

This PR adds new Slider shapes BarSliderThumbShape, ExpressiveRoundedRectSliderTrackShape, and RoundedRectSliderValueIndicatorShape which are enabled when using the new SliderThemeData.use2024SliderShapes flag.

Preview

Thumb Pressed Thumb

New shapes when dragged

Light mode Dark mode
Record_2024-05-03-17-12-14.mp4
Record_2024-05-03-17-12-28.mp4

Stop indicator in the RoundedRectSliderValueIndicatorShape track shape

Preview 1 Preview 2
stop_indicator.mov
trackGapSize set to 0 Overridden trackGapSize & barThumbSize

Pre-launch Checklist

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

@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos labels May 3, 2024
@TahaTesser TahaTesser force-pushed the slider_new_shapes branch from 3fc68e1 to dc1178f Compare May 3, 2024 15:24
@TahaTesser TahaTesser marked this pull request as ready for review May 3, 2024 15:42
@TahaTesser TahaTesser force-pushed the slider_new_shapes branch from dc1178f to b1710e7 Compare May 3, 2024 16:36
@TahaTesser TahaTesser requested a review from Piinks May 3, 2024 17:13
Copy link
Member

@guidezpl guidezpl left a comment

Choose a reason for hiding this comment

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

Nice work, just a general comment, I think we should make it easier to use the new shapes, since they will become the new default. Rather than manually specifying them, developers could flip a flag on ThemeData.

@TahaTesser
Copy link
Member Author

Nice work, just a general comment, I think we should make it easier to use the new shapes, since they will become the new default. Rather than manually specifying them, developers could flip a flag on ThemeData.

Interesting idea, tho this is only specific to Slider (next RangeSlider). Maybe in the SliderThemeData? Do you've a name for this flag?

@guidezpl
Copy link
Member

guidezpl commented May 6, 2024

Perhaps useUpdatedSliderShapes or use2024SliderShapes? I think it would apply to all types of sliders

@TahaTesser
Copy link
Member Author

Perhaps useUpdatedSliderShapes or use2024SliderShapes? I think it would apply to all types of sliders

Added use2024SliderShapes flag, updated defaults and tests. I'll push the changes with updated docs tomorrow.

@TahaTesser TahaTesser force-pushed the slider_new_shapes branch from b1710e7 to bb9d19d Compare May 8, 2024 13:16
@TahaTesser TahaTesser requested a review from guidezpl May 8, 2024 15:03
@TahaTesser
Copy link
Member Author

@guidezpl
Appreciate the review! I'm about start traveling for Google I/O 2024. I'll address them after returning from states.

@TahaTesser TahaTesser force-pushed the slider_new_shapes branch from bb9d19d to 732076d Compare May 21, 2024 10:30
@TahaTesser TahaTesser mentioned this pull request May 22, 2024
9 tasks
@TahaTesser TahaTesser force-pushed the slider_new_shapes branch from 732076d to 428389a Compare May 22, 2024 08:38
@TahaTesser
Copy link
Member Author

TahaTesser commented May 22, 2024

This PR #148789 removes the tokens for current M3 defaults so I will update the new shapes to use new the tokens and restore deleted token values as we plan to keep the current M3 shapes under the new flag.

@TahaTesser TahaTesser force-pushed the slider_new_shapes branch 4 times, most recently from 260bd1a to 745acf8 Compare May 23, 2024 15:37
@TahaTesser
Copy link
Member Author

Updated the PR to use new tokens, restored the current M3 defaults, and made minor improvements.

Copy link
Member

@guidezpl guidezpl left a comment

Choose a reason for hiding this comment

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

nice! looking for a second review

@TahaTesser TahaTesser force-pushed the slider_new_shapes branch from dab29c7 to 025ac29 Compare May 27, 2024 10:36
@guidezpl
Copy link
Member

Added an internal fix, Google testing will test again now. Please ping me if it's still not passing and I forget to check.

@TahaTesser
Copy link
Member Author

Added an internal fix, Google testing will test again now. Please ping me if it's still not passing and I forget to check.

Thanks so much, looks like it passes now! Waiting for the second review.

Copy link
Contributor

@Piinks Piinks left a comment

Choose a reason for hiding this comment

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

This LGTM overall, it's just the use2024SliderShapes that gave me pause. Is this something we expect folks to migrate to? What is the difference between M2/M3/M3-2024?

Could we add some docs that explain this a bit more? And if we want folks to migrate, we should have a migration guide to let them know to start moving. :)

@TahaTesser
Copy link
Member Author

TahaTesser commented May 30, 2024

Pushed some doc updates.

What is the difference between M2/M3/M3-2024?

Great questions! It's all in the visuals. Instead of listing all the shapes, I'll add some Slider diagrams in assets-for-api-docs and mention them here.

we should have a migration guide to let them know to start moving. :)

Will write a migration for flutter/website.

@Piinks
Copy link
Contributor

Piinks commented May 30, 2024

Are we wanting folks to migrate so we can have an updated default shape? Will all of the shapes be available (backwards compatible) for folks to use after the migration (and removal of the use2024Shapes flag)?
If both answers are yes, and it is a simple migration of setting the shape on the slider, it may be worth considering just updating the default and writing a migration guide rather than doing a multi-step flag migration.

@TahaTesser
Copy link
Member Author

Yes, all shapes are backwards compatible. I love this idea, however changing the defaults will give different slider appearance and it would be overridden with regular SliderTheme.

Shall I remove the use2024SliderShapes flag? If we update defaults and they can customise the slider then we don't need this flag.

@TahaTesser
Copy link
Member Author

Discussed this with Kate offline, moving forward with default update and migration guide.

@TahaTesser
Copy link
Member Author

TahaTesser commented May 31, 2024

While updating the defaults, I ran into an existing bug with divisions that could revert this new shapes update, it's a blocker.
This division bug can reproduced in the current Slider in both M2 and M3 configurations, I should fix this bug first in separate PR.

Update: #62567

Update: Visual progress #62567 (comment)

@TahaTesser
Copy link
Member Author

#149594 fixes the issues to unblock this PR.

auto-submit bot pushed a commit that referenced this pull request Jul 15, 2024
…ounded corners (#149594)

fixes [[Slider] Thumb's center doesn't align with division's center](#62567)
fixes [Slider thumb doesn't respect round slider track shape](#149591)
fixes [`RoundedRectSliderTrackShape` corners are not rendered correctly](#149589)

(Verified these behaviors with Android components implementation)

### 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 _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;
            });
          },
        ),
      ),
    );
  }
}
```

</details>

### 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.

![Group 8](https://github.com/flutter/flutter/assets/48603081/9aa138ae-9525-4af4-8fc7-3cea0692a6d7)

![Group 9](https://github.com/flutter/flutter/assets/48603081/e97940ae-a1c8-4b8b-9971-1cf417d32e40)

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

![Group 10](https://github.com/flutter/flutter/assets/48603081/ed20a6bb-d5c9-486b-a020-2c9ca7de55da)

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

![Group 4](https://github.com/flutter/flutter/assets/48603081/37a2e820-402d-4964-a206-717ccf1c5c02)

![Group 3](https://github.com/flutter/flutter/assets/48603081/5d36d523-5fb7-466f-9d53-b6928963fcab)

![Group 7](https://github.com/flutter/flutter/assets/48603081/8f3b4c48-f04d-4681-a62f-a7ea5a3e19fa)
@TahaTesser
Copy link
Member Author

#149594 just landed which unblocks this PR.

I'll update this PR and writing migration guide

@TahaTesser TahaTesser closed this Jul 24, 2024
@TahaTesser TahaTesser deleted the slider_new_shapes branch July 24, 2024 11:37
@TahaTesser
Copy link
Member Author

Closing this PR for a new clean PR which will also be included in the migration guide.

github-merge-queue bot pushed a commit that referenced this pull request Nov 19, 2024
fixes [Update `Slider` for Material 3
redesign](#141842)
previous implementation #147783

### Description

This PR introduces new Material 3 Slider design. 

### Slider Preview

<img width="912" alt="Screenshot 2024-07-24 at 16 38 11"
src="https://github.com/user-attachments/assets/9645ff6c-b72a-40aa-ae95-4f76994f8302">

<img width="912" alt="Screenshot 2024-07-24 at 16 38 24"
src="https://github.com/user-attachments/assets/fbaed8bb-2717-43a9-9415-ea1365165d9a">

### Value indicator Preview



https://github.com/user-attachments/assets/45fa001c-de81-433a-a8e9-6c0d6a2335c0

### New stop indicator



https://github.com/user-attachments/assets/ad05621d-042d-4b17-9dbb-7f7b802a2593


### Customized 

<img width="912" alt="Screenshot 2024-07-24 at 16 41 49"
src="https://github.com/user-attachments/assets/2f279240-5af8-4bc8-9c65-a4b4ac718101">



## 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].

<!-- Links -->
[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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update Slider for Material 3 redesign

4 participants