-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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 progress indicators and slider guides for theme migration #11733
Conversation
… steps using themes
0ac8589
to
d0cdf7f
Compare
Visit the preview URL for this PR (updated for commit 4dea60c): https://flutter-docs-prod--pr11733-update-m3-guides-fkcx5me7.web.app |
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 these clarifications for these design updates!
I have a question and a few small suggestions. I only made the suggestions for the first one, but they apply to each instance.
|
||
```dart highlightLines=2 | ||
return MaterialApp( | ||
theme: ThemeData(progressIndicatorTheme: const ProgressIndicatorThemeData(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.
I tried this out and it's saying the year2023
parameter and corresponding property are deprecated?
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.
Yes, we added this flag with deprecation a few months ago to encourage users to read the migration steps and opt in the design.
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.
Eventually, we'll toggle the default value so users won't need to use the flag for the new design.
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 see, thanks! It's a bit confusing though, as it's a deprecation warning, but in this case, you don't want to remove it.
Should we add an ignore comment to these snippets with an explanation of why the diagnostic is ignored?
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 is the deprecation message:
@Deprecated(
'Set this flag to false to opt into the 2024 progress indicator appearance. Defaults to true. '
'In the future, this flag will default to false. Use ProgressIndicatorThemeData to customize individual properties. '
'This feature was deprecated after v3.26.0-0.1.pre.',
)
this.year2023,
This message explains what will happen to the default value in the future and this flag isn't permanent hence the deprecation. Adding message to ignore will defeat the purpose of deprecation message.
src/content/release/breaking-changes/updated-material-3-progress-indicators.md
Outdated
Show resolved
Hide resolved
src/content/release/breaking-changes/updated-material-3-progress-indicators.md
Outdated
Show resolved
Hide resolved
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 the PR and the adjustments @TahaTesser! Looks good to me.
Optionally do consider a comment or note like I mentioned in https://github.com/flutter/website/pull/11733/files#r1970471078.
Fixes Update slider and progress indicator migration guides with
year2023
flag from component themesPresubmit checklist