这是indexloc提供的服务，不要输入任何密码
Skip to content

Added a ProgressIndicatorTheme. #81075

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Apr 26, 2021
Merged

Added a ProgressIndicatorTheme. #81075

merged 4 commits into from
Apr 26, 2021

Conversation

darrenaustin
Copy link
Contributor

@darrenaustin darrenaustin commented Apr 23, 2021 •
edited by HansMuller
Loading

Adds a new ProgressIndicatorTheme to allow applications to theme the color properties of progress indicators in their apps. Follows the pattern for themes described in Theme System Updates.

The following properties are supported by this theme:

Property Widget property ProgressIndicatorTheme property Default
Indicator color ProgressIndicator.color color ColorScheme.primary
Linear track color LinearProgressIndicator.backgroundColor linearTrackColor ColorScheme.background
Linear min height LinearProgressIndicator.minHeight linearMinHeight 4dp
Circular track color CircularProgressIndicator.backgroundColor circularTrackColor not painted
Refresh background RefreshIndicator.backgroundColor refreshBackgroundColor ThemeData.canvasColor

New tests were added to verify that the theme values are used if specified.

Fixes: #77647
This PR was added per #56918.

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 feature I am adding, or Hixie said the PR is test-exempt.
  • All existing and new tests are passing.

@flutter-dashboard flutter-dashboard bot added f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. labels Apr 23, 2021
@google-cla google-cla bot added the cla: yes label Apr 23, 2021
@darrenaustin darrenaustin requested a review from HansMuller April 23, 2021 20:33
@darrenaustin darrenaustin mentioned this pull request Apr 23, 2021
Merged
HansMuller
HansMuller approved these changes Apr 23, 2021
View reviewed changes
Copy link
Contributor

@HansMuller HansMuller left a comment

Choose a reason for hiding this comment

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

LGTM

@darrenaustin
Copy link
Contributor Author

LGTM

Thanks for the review. I have updated based on your comments.

I have also broken out the backgroundColor theme property to linearTrackColor, circularTrackColor, and refreshBackgroundColor as they each interpret the background color in different ways with different defaults. An app might want to specify the track color of the linear progress bars and not have it change the background color of the refresh indicators. If they were one property there would be no easy way to have different behavior for each of the indicator types.

Let me know what you think.

@darrenaustin darrenaustin requested a review from HansMuller April 26, 2021 07:32
HansMuller
HansMuller reviewed Apr 26, 2021
View reviewed changes
Copy link
Contributor

@HansMuller HansMuller left a comment

Choose a reason for hiding this comment

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

I have also broken out the backgroundColor theme property to linearTrackColor, circularTrackColor, and refreshBackgroundColor as they each interpret the background color in different ways with different defaults. An app might want to specify the track color of the linear progress bars and not have it change the background color of the refresh indicators. If they were one property there would be no easy way to have different behavior for each of the indicator types.

Let me know what you think.

Breaking out the properties that are roughly "background color" but actually mean something specific to the progress indicator subclass, makes sense to me. And I agree that theme users might want to just change the linearTrackColor. It might be clearer if the widget subclasses had the same constructor parameter name as the theme. So for LinearProgressIndicator: super(backgroundColor: linearTrackColor, ...) etc.

packages/flutter/lib/src/material/progress_indicator.dart
/// also null then with the current theme's [ColorScheme.primary].
/// If null, the progress indicator is rendered with [color]. If that is null,
/// then it will use the ambient [ProgressIndicatorThemeData.color]. If that
/// is also null then it defaults to the current theme's [ColorScheme.primary].
///
/// This property is ignored if used in an adaptive constructor inside an iOS
Copy link
Contributor

Choose a reason for hiding this comment

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

As far as I can tell, this warning, which is all over the place, just refers to CircularProgressIndicator.adaptive. Assuming that's true, we should just call out CircularProgressIndicator.adaptive specifically and eliminate this boilerplate comment where it's not really necessary. OK to do that in a separate PR BTW.

This was referenced Apr 26, 2021
Closed
Closed
@darrenaustin
Copy link
Contributor Author

It might be clearer if the widget subclasses had the same constructor parameter name as the theme. So for LinearProgressIndicator: super(backgroundColor: linearTrackColor, ...) etc.

I had that at one point, but we would still need to support backgroundColor as a constructor param for compatibility and then the documentation got even more confusing, so I just left it as is. We can revisit this later if it becomes an issue.

@darrenaustin darrenaustin merged commit e2ddeb0 into flutter:master Apr 26, 2021
@darrenaustin darrenaustin deleted the progress_indicator_theme branch April 26, 2021 20:53
@HansMuller HansMuller mentioned this pull request Apr 27, 2021
Closed
@darrenaustin darrenaustin mentioned this pull request Apr 28, 2021
Merged
8 tasks
davidmartos96 pushed a commit to davidmartos96/flutter that referenced this pull request May 19, 2021
@darrenaustin @davidmartos96
Added a ProgressIndicatorTheme. (flutter#81075)
fb92783
davidmartos96 pushed a commit to davidmartos96/flutter that referenced this pull request Jun 14, 2021
@darrenaustin @davidmartos96
Added a ProgressIndicatorTheme. (flutter#81075)
97cdd55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@HansMuller HansMuller HansMuller approved these changes

Assignees
No one assigned
Labels
f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

CircularProgressIndicator is not using ThemeData.backgroundColor by default
2 participants
@darrenaustin @HansMuller