-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Update Material 3 LinearProgressIndicator for new visual style
#154817
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
Update Material 3 LinearProgressIndicator for new visual style
#154817
Conversation
5ace163 to
4b298ab
Compare
|
Thanks for the update😍! Sorry for the late response. I will review this one ASAP! |
QuncCccccc
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.
This looks great! Thanks a lot for the update!!
examples/api/lib/material/progress_indicator/linear_progress_indicator.0.dart
Outdated
Show resolved
Hide resolved
examples/api/lib/material/progress_indicator/linear_progress_indicator.0.dart
Outdated
Show resolved
Hide resolved
examples/api/lib/material/progress_indicator/linear_progress_indicator.1.dart
Outdated
Show resolved
Hide resolved
packages/flutter/lib/src/material/progress_indicator_theme.dart
Outdated
Show resolved
Hide resolved
39c5e88 to
a96393c
Compare
a96393c to
c1fd63e
Compare
|
Addressed feedback, I also fixed some properties which should've been nullable :) |
QuncCccccc
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.
LGTM:) Thanks for the fix! Somehow the Google testing is not triggered. I think there might be some breakages in g3.
c1fd63e to
2fc29b3
Compare
|
This also cause a lot of breakages in G3. Like the Slider PR, we probably should do the same thing (even though we haven't decided which approach we are going to take:)) |
|
@QuncCccccc tbh I'm not surprised. This has similar changes as Slider. and that's why I filed this PR to see what happens. And there will be more PRs for RangeSlider and CircularProgressIndicator. Maybe we need a global flag we decide to choose flag to opt-in for these updated widgets? |
I would prefer 4 flags for individual widgets(Slider, RangeSlider, Linear/Circular-progressIndicator). Because not too many widgets need this flag and I don't think there will be more token update for other Material 3 widgets, so one flag for each is probably okay. When we need to support Material 3+ in the future, a global flag is probably more desired? |
e6f8ba5 to
8012daf
Compare
|
Add a new flag to get the previous M3 design, defaults to false. Similar to the Slider PR |
QuncCccccc
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.
Sorry for the late response! I will catch up my review list this week!
| 'Use ProgressIndicatorTheme to customize the ProgressIndicator appearance. ' | ||
| 'This feature was deprecated after v3.26.0-0.1.pre.' | ||
| ) | ||
| this.year2023 = false, |
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 here default to true if the flag called year2023? I think from our discussion in the design doc, we want the flag defaults to a value which doesn't cause breaking changes. Like in the doc, the original flag is called gappedStyle and we want it default to false to hide the latest look:). Similar to the Slider PR.
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.
Ohhh i was thinking opposite so we users just can just turn off new design manually but setting to true by default makes sense if we don't wanna any breaking changes initially.
I updated the PR and looks like all existing tests are green 🎉
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'll write migration guide for the new flag as we're deprecating right away in this PR. WDYT?
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.
A migration guide would be super helpful! Thanks a lot! We can also create an example to toggle the two progress indicator themes so that when this flag is removed in the future, users can copy and paste the old theme from the example if they really want.
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.
As i started to write the migration guide i realized they've relatively small updates compare to Sliders. I think it'd be best to write single migration guide for both progress indicators (Linear and Circular). Now that we've a clear direction with the flag.
We can land this PR and I'm already working on circular progress indicator update PR. I'll include both widgets in a single progress indicators migration guide. It'd also be easier to track the changes for users.
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.
Sounds good! Since the flag will not be removed immediately, I think it's fine to have a single migration guide for both progress indicators! Thanks a lot!
8012daf to
db62af7
Compare
db62af7 to
44473fb
Compare
QuncCccccc
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.
LGTM! Thanks a lot for adding the flag!
|
Adding autosubmit but stop me if there was still something to address here! |
Manual roll requested by [email protected] flutter/flutter@fe71cad...0fe6153 2024-10-31 [email protected] Roll Flutter Engine from c40b0b602822 to f2154ef3e31c (8 revisions) (flutter/flutter#157926) 2024-10-31 [email protected] Hides added routes before top-most route finishes pushing (flutter/flutter#156104) 2024-10-31 [email protected] Fix cursor on hover expand/collapse icon (#155207) (flutter/flutter#155209) 2024-10-31 [email protected] Add test for `media_query_data.system_gesture_insets.0.dart` (flutter/flutter#157854) 2024-10-31 [email protected] Add and plumb `useImplicitPubspecResolution` across `flutter_tools`. (flutter/flutter#157879) 2024-10-31 [email protected] Roll Flutter Engine from 090c33aeae83 to c40b0b602822 (1 revision) (flutter/flutter#157896) 2024-10-31 [email protected] Roll Flutter Engine from 9295eeb6d3ce to 090c33aeae83 (4 revisions) (flutter/flutter#157893) 2024-10-30 [email protected] Roll Flutter Engine from 2bd854e23b61 to 9295eeb6d3ce (5 revisions) (flutter/flutter#157882) 2024-10-30 [email protected] Adds a new helpful tool exit message for SocketExceptions thrown during mdns discovery (flutter/flutter#157638) 2024-10-30 [email protected] Fix `GlowingOverscrollIndicator` examples (flutter/flutter#155203) 2024-10-30 [email protected] Roll Flutter Engine from 906a7ad88052 to 2bd854e23b61 (1 revision) (flutter/flutter#157878) 2024-10-30 [email protected] Upgrade templates to AGP 8.7/Gradle 8.10.2 (flutter/flutter#157872) 2024-10-30 [email protected] Roll Flutter Engine from 57ed5d338e7e to 906a7ad88052 (13 revisions) (flutter/flutter#157875) 2024-10-30 [email protected] Update Material 3 `LinearProgressIndicator` for new visual style (flutter/flutter#154817) 2024-10-30 [email protected] Roll Flutter Engine from 999797a2f690 to 57ed5d338e7e (5 revisions) (flutter/flutter#157833) 2024-10-30 [email protected] Add hidden `--no-implicit-pubspec-resolution` flag for one stable release. (flutter/flutter#157635) 2024-10-30 [email protected] Roll Packages from 028027e to 7cc1caa (5 revisions) (flutter/flutter#157864) 2024-10-30 [email protected] Mention partial PRs in the contributing docs (flutter/flutter#157863) 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 To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
LinearProgressIndicator for new visual styleLinearProgressIndicator for new visual style
Related to [Update Material 3 `CircularProgressIndicator` for new visual style](flutter/flutter#158104) and [Update Material 3 `LinearProgressIndicator` for new visual style](flutter/flutter#154817) ## Presubmit checklist - [x] This PR is marked as draft with an explanation if not meant to land until a future stable release. - [x] This PR doesn’t contain automatically generated corrections (Grammarly or similar). - [x] This PR follows the [Google Developer Documentation Style Guidelines](https://developers.google.com/style) — for example, it doesn’t use _i.e._ or _e.g._, and it avoids _I_ and _we_ (first person). - [x] This PR uses [semantic line breaks](https://github.com/dart-lang/site-shared/blob/main/doc/writing-for-dart-and-flutter-websites.md#semantic-line-breaks) of 80 characters or fewer.
Related to Update both
ProgressIndicatorfor Material 3 redesignCode sample
expand to view the code sample
Preview
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.