-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Introduce new Material 3 Slider shapes
#152237
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
Conversation
|
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 Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding the feature! This looks so great! I haven't finish reviewing the whole change, will continue tomorrow😎!
I also noticed slider.0.dart and slider.1.dart are basically the same. I think slider.1.dart was added when useMaterial3 was still false? Maybe we can delete one of these and add a button to switch between M2 and M3 in just one example app. This can be done later in a separate PR:)
| /// If [SliderThemeData.thumbShape] is [BarSliderThumbShape], this property is used to | ||
| /// set the size of the thumb. Otherwise, the default thumb size is 4 pixels for the | ||
| /// width and 44 pixels for the height. | ||
| final MaterialStateProperty<Size?>? barThumbSize; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be better if we just call it thumbSize? Just feel it would be more consistent with other properties.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason i made it barThumbSize because it's a Size type and this is not compatible with existing Thumb as it is just a circle and it would be double aka radius value. If the user expect thumbSize to update existing thumb it might create confusion.
However, you raised a good point on consistency. I will take a look if thumbSize of Size type can be used for existing thumb radius by rounding to square value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree. I did not realize this theme data class was so large. There are a lot of parameters here, so having a clear name would be ideal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe (in a separate change) we can look at this, and maybe group the parameters together based on the type of slider they apply to.
| Color? get disabledThumbColor => _colors.onSurface.withOpacity(0.38).withOpacity(0.38); | ||
|
|
||
| @override | ||
| Color? get overlayColor => MaterialStateColor.resolveWith((Set<MaterialState> states) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From the specs, it seems we don't have overlayColor by default. Looks like these have been deprecated internally but haven't been updated on the website. Maybe we can change this to transparent color or change the default overlayShape to null?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, I also tested Slider on the native and it doesn't seem to have overlay color anymore but when using it there is no other way to indicate if the Slider is hovered or focused. I sent a Demo on Discord.
IMO, to avoid breaking existing tests and focus, hover behavior of the Flutter Slider widget, we can keep the overlay color as it is until we have other ways to indicate these states.
1e547a2 to
b4a762b
Compare
|
Golden file changes are available for triage from new commit, Click here to view. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
Piinks
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your patience here, I'll be much quicker on the follow up reviews. 🙏 💙
| labelPainter: _labelPainter, | ||
| parentBox: this, | ||
| sliderTheme: _sliderTheme, | ||
| sliderTheme: _sliderTheme.copyWith(barThumbSize: MaterialStatePropertyAll<Size>(Size(thumbWidth, thumbHeight))), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same explanation as #152237 (comment)
| /// If [SliderThemeData.thumbShape] is [BarSliderThumbShape], this property is used to | ||
| /// set the size of the thumb. Otherwise, the default thumb size is 4 pixels for the | ||
| /// width and 44 pixels for the height. | ||
| final MaterialStateProperty<Size?>? barThumbSize; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree. I did not realize this theme data class was so large. There are a lot of parameters here, so having a clear name would be ideal.
| /// If [SliderThemeData.thumbShape] is [BarSliderThumbShape], this property is used to | ||
| /// set the size of the thumb. Otherwise, the default thumb size is 4 pixels for the | ||
| /// width and 44 pixels for the height. | ||
| final MaterialStateProperty<Size?>? barThumbSize; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe (in a separate change) we can look at this, and maybe group the parameters together based on the type of slider they apply to.
| /// width and 44 pixels for the height. | ||
| final MaterialStateProperty<Size?>? barThumbSize; | ||
|
|
||
| /// The size of the gap between the active and inactive tracks of the [GappedSliderTrackShape]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this only relevant for the M3 slider? Or both? We should check that the docs clearly state when/how a parameter/behavior/etc applies for M2 versus M3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All the new shapes are compatible with M2/M3. Great suggestion
I added this documentation
/// The Slider defaults to [GappedSliderTrackShape] when the track shape is
/// not specified, and the [trackGapSize] can be used to adjust the gap size.
///
/// If the [ThemeData.useMaterial3] is false, then the Slider track shape
/// defaults to [RoundedRectSliderTrackShape] and the [trackGapSize] is ignored.
/// In this case, set the track shape to [GappedSliderTrackShape] to use the
/// [trackGapSize].
final double? trackGapSize;| } | ||
| } | ||
|
|
||
| /// The bar shape of a [Slider]'s thumb. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another thought for another change later - there's a ton of stuff in this massive file. Might be worth sending a change after this to move everything that does not relate to slider theme out of this file to a new one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's right, this file contains every Slider shape and it's messy.
|
Back from vacation, there are a lot of great feedback here and I'll address them. |
6c56faf to
e9e6fc7
Compare
|
Golden file changes are available for triage from new commit, Click here to view. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
There were some conflicts which i resolved today. I'm addressing a few more API challenges such as supporting the new |
d3efb93 to
2b5194b
Compare
|
I have tried several ways to support the new flutter/packages/flutter/lib/src/material/slider_theme.dart Lines 2282 to 2289 in 99f00a1
To move forward, we might need to state the new |
|
@QuncCccccc @Piinks |
2b5194b to
cf3c9c1
Compare
That SGTM. |
| } | ||
| } | ||
|
|
||
| Widget _buildMaterialSlider(BuildContext context) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the docs on Slider be updated to reflect all of this new new? :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bumping this, do we think there are any doc updates with this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So sorry, i wanted to comment. Yes, I want to write this Slider doc along side the migration guide so i can mention the new stuff things in the migration guide and sync them with Slider docs. Migration guide will have steps to restore the previous Slider components
Now that PR is approved. I will initiate migration guide draft.
60383ea to
f8c3c22
Compare
|
Golden file changes are available for triage from new commit, Click here to view. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
Hello, i am on the latest flutter, and i wanted to ask when these changes are online for everyone. flutter doktor: The Slider still look like the Material 2 one. but i want to use the one from Material 3 Kind regards |
Roll Flutter from 8536b96 to 93d772c (37 revisions) flutter/flutter@8536b96...93d772c 2024-11-21 [email protected] Added additional logging to `_listCoreDevices` (flutter/flutter#159275) 2024-11-21 [email protected] Roll Flutter Engine from 78b87f3fe023 to d1a08064e193 (1 revision) (flutter/flutter#159280) 2024-11-21 [email protected] Shut down DevTools and DDS processes if flutter_tools is killed by a signal (flutter/flutter#159238) 2024-11-21 [email protected] Remove `RepaintBoundary` that is no longer needed. (flutter/flutter#159232) 2024-11-21 [email protected] Try a speculative fix for Gradle OOMs. (flutter/flutter#159234) 2024-11-21 [email protected] On-device Widget Inspector button exits widget selection (flutter/flutter#158219) 2024-11-21 [email protected] Roll Flutter Engine from 523d381893c8 to 78b87f3fe023 (2 revisions) (flutter/flutter#159270) 2024-11-21 [email protected] Remove `firebase_abstract_method_smoke_test` (flutter/flutter#159145) 2024-11-21 [email protected] Roll Packages from e95f6d8 to 913b99e (7 revisions) (flutter/flutter#159268) 2024-11-21 [email protected] Roll Flutter Engine from 69c325513a65 to 523d381893c8 (3 revisions) (flutter/flutter#159263) 2024-11-21 [email protected] [flutter_tools] opt iOS/macOS apps out of Metal API validation via migrator, update templates in repo. (flutter/flutter#159228) 2024-11-21 [email protected] Scribe Android handwriting text input (flutter/flutter#148784) 2024-11-21 [email protected] Terminate non-detached test devices on `flutter run` completion (flutter/flutter#159170) 2024-11-21 [email protected] Un-skip tests that use `flutter build apk`. (flutter/flutter#159231) 2024-11-20 [email protected] Roll Flutter Engine from 2d32cf3a7971 to 69c325513a65 (1 revision) (flutter/flutter#159229) 2024-11-20 [email protected] Roll Flutter Engine from 3828681d1f86 to 2d32cf3a7971 (3 revisions) (flutter/flutter#159226) 2024-11-20 [email protected] Roll Flutter Engine from 3245c8976976 to 3828681d1f86 (1 revision) (flutter/flutter#159217) 2024-11-20 [email protected] Add `--dry-run` to `dev/bots/test.dart`. (flutter/flutter#158956) 2024-11-20 [email protected] Add docs for setting up Android Studio to auto format Kotlin code (flutter/flutter#159209) 2024-11-20 [email protected] Roll Flutter Engine from 80d77505fdde to 3245c8976976 (1 revision) (flutter/flutter#159214) 2024-11-20 [email protected] Fix: The enableFeedback property of InkWell cannot be set to a nullab� (flutter/flutter#158907) 2024-11-20 [email protected] Roll Flutter Engine from 3f19207e820e to 80d77505fdde (1 revision) (flutter/flutter#159210) 2024-11-20 [email protected] Roll Packages from fc4adc7 to e95f6d8 (6 revisions) (flutter/flutter#159201) 2024-11-20 [email protected] Remove dependency on [Target] and instead operate on [Architecture] (flutter/flutter#159196) 2024-11-20 [email protected] Fix git command in Quality-Assurance.md (flutter/flutter#155146) 2024-11-20 [email protected] Roll Flutter Engine from 7eb87547cbc6 to 3f19207e820e (4 revisions) (flutter/flutter#159176) 2024-11-20 [email protected] Make `runner` non-nullable as it always is. (flutter/flutter#159156) 2024-11-19 [email protected] Update Material 3 `CircularProgressIndicator` for new visual style (flutter/flutter#158104) 2024-11-19 [email protected] Roll Flutter Engine from 4ff696b555dc to 7eb87547cbc6 (3 revisions) (flutter/flutter#159168) 2024-11-19 [email protected] Add platform-android label for all flutter_tools *android* files (flutter/flutter#159166) 2024-11-19 [email protected] Roll Flutter Engine from d5820a638885 to 4ff696b555dc (1 revision) (flutter/flutter#159164) 2024-11-19 [email protected] Reland Add UI Benchmarks (flutter/flutter#153368) 2024-11-19 [email protected] fix lint usage of `task` inside `resolve_dependecies.gradle` file (flutter/flutter#158022) 2024-11-19 [email protected] Roll Flutter Engine from cff1e751f853 to d5820a638885 (5 revisions) (flutter/flutter#159155) 2024-11-19 [email protected] Removing redundant backticks in `flutter\packages\flutter_tools\gradle\gradle.kts` (flutter/flutter#159051) 2024-11-19 [email protected] Fixes initial validation with AutovalidateMode.always on first build (flutter/flutter#156708) 2024-11-19 [email protected] Introduce new Material 3 `Slider` shapes (flutter/flutter#152237) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose ...
In case you're still wondering: you need to set the |
|
Hi, I'm trying to use the latest Material 3 What is the correct way to use the latest Material 3 |
|
Hi! Setting |
|
@QuncCccccc but how do you use the latest M3 Slider? If I simply import the Slider component from flutter/lib/src/material it's still the old M2 design. So setting |
|
The design of M3 has changed a lot. Originally, the M3 Slider didn't change its style much except for the default colors, so I assume that's the "old M2 design" you're referring to. In late 2023, the M3 Slider design changed to its latest version. Because of this, we updated the Slider and introduced the year2023 flag to distinguish between M3 Sliders from before late 2023 and those from after. When we were migrating from M2 to M3, it caused many breaking changes, which was quite difficult for developers. For that reason, we decided to keep the "old" M3 Slider design as the default. We then introduced year2023 so developers could manually enable the flag to see the latest design while avoiding those breaking changes. So yes, setting year2023 to false is required if we wanted to see the latest Slider. |
|
Hi folks, old PRs are not the best venue for feedback and discussion. If there is an issue, please file one and we can follow up, including if we need to improve the documentation. |
fixes Update
Sliderfor Material 3 redesignprevious implementation #147783
Description
This PR introduces new Material 3 Slider design.
Slider Preview
Value indicator Preview
Screen.Recording.2024-07-24.at.16.39.16.mov
New stop indicator
Screen.Recording.2024-07-24.at.16.40.17.mov
Customized
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.