Allow Passing an AnimationController to CircularProgressIndicator (Fix #165741) #170380

Sten435 · 2025-06-10T19:36:59Z

Pull request for issue: #165741
I also added support, to use an inherited controller, using the controller prop in progress_indicator_theme.dart.

fix.progress.indic.mp4

Pre-launch Checklist

I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
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
All existing and new tests are passing.

fix 165741

Sten435 · 2025-06-10T21:32:26Z

Shit i merged into main, i will change it to master.

👍 Fixed ✅

stan-at-work · 2025-06-11T07:40:33Z

@dkwingsmt The Linux analyze check fails, when i look into the docs this is what it say's:

You may find the errors by searching for "╡ERROR #" in the logs.
╔═╡ERROR #1╞════════════════════════════════════════════════════════════════════
║ Command: dart --enable-asserts /b/s/w/ir/x/w/flutter/dev/bots/analyze.dart
║ Command exited with exit code 1 but expected zero exit code.
║ Working directory: /b/s/w/ir/x/w/flutter
╚═══════════════════════════════════════════════════════════════════════════════
▌23:20:10▐ Test failed.
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
The error messages reported above are repeated here:
  -- This line intentionally left blank --  
Command: dart --enable-asserts /b/s/w/ir/x/w/flutter/dev/bots/analyze.dart
Command exited with exit code 1 but expected zero exit code.
Working directory: /b/s/w/ir/x/w/flutter

This is not something I changed with my pull request. Does that mean this is not an issue for me, or what should I do to address it?

dkwingsmt · 2025-06-11T07:44:58Z

If you click the detail link and see the full log (https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8712382049978374001/+/u/run_test.dart_for_analyze_shard_and_subshard_None/stdout?format=raw ) you'll find an error at ~45% of the file. I think it's just the eol space. Anyway, such errors can typically be solved by dart format packages/flutter.

stan-at-work · 2025-06-11T07:50:15Z

If you click the detail link and see the full log (https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8712382049978374001/+/u/run_test.dart_for_analyze_shard_and_subshard_None/stdout?format=raw ) you'll find an error at ~45% of the file. I think it's just the eol space. Anyway, such errors can typically be solved by dart format packages/flutter.

O thanks, don't know how you figured that out, from the logs. But thanks.

When i have access to my pc i will update the pull req ✅

dkwingsmt · 2025-06-11T07:52:16Z

The full log is here :) Yeah it can be a bit hidden

Sten435 · 2025-06-11T18:50:35Z

If you click the detail link and see the full log (https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8712382049978374001/+/u/run_test.dart_for_analyze_shard_and_subshard_None/stdout?format=raw ) you'll find an error at ~45% of the file. I think it's just the eol space. Anyway, such errors can typically be solved by dart format packages/flutter.

You were indeed right, I ran the command you provided and what happend.... 1 change... 🥳

stan-at-work · 2025-06-13T08:57:50Z

@dkwingsmt Does this pull request fall into the category of pull requests that the team will not merge until they have figured the Material and Cupertino styling out? I heard that something like that was going on at the moment.

Otherwise, I will close this pull request, and reopen it in a couple of months.

Thanks. 😃

dkwingsmt · 2025-06-14T03:20:02Z

Hi! I don't think so. Although I'm currently on vacation and will review your PR when I'm back next week.

Also if there were such cases we would definitely tell the author right away instead of having you suspecting. :)

stan-at-work · 2025-06-14T08:09:33Z

Hi! I don't think so. Although I'm currently on vacation and will review your PR when I'm back next week.

Also if there were such cases we would definitely tell the author right away instead of having you suspecting. :)

I'm sorry thanks,

Have a nice vacation ☀️🌊

packages/flutter/lib/src/material/progress_indicator.dart

packages/flutter/test/material/progress_indicator_test.dart

# Conflicts: # packages/flutter/lib/src/material/progress_indicator.dart

packages/flutter/test/material/progress_indicator_test.dart

packages/flutter/lib/src/material/progress_indicator.dart

dkwingsmt · 2025-07-15T03:19:53Z

Yep I've tested that adding disposal statements resolve the failure. 7a7da01

stan-at-work · 2025-07-15T07:51:19Z

Yep I've tested that adding disposal statements resolve the failure. 7a7da01

Thanks a lot for the feedback; I'll get those code comments adjusted. Thanks 👍

Fix 165741

Added docs

victorsanni · 2025-07-16T20:01:31Z

packages/flutter/test/material/progress_indicator_test.dart

@@ -567,6 +567,80 @@ void main() {
    expect(tester.binding.transientCallbackCount, 0);
  });

+  testWidgets('LinearProgressIndicator reflects controller value', (WidgetTester tester) async {


I don't see the point of both tests. The controller's value is passed into the progress indicator directly?

The PR is adding a new property that increases the API surface. Tests should check that that property is respected (for both progress indicators and ProgressIndicatorThemeData as well.).

In what way should I check that its respected ?
Like check if the hashcode is the same or ?

I don't see the point of both tests. The controller's value is passed into the progress indicator directly?

Any idea's on how you would tackle this, differently ?

@dkwingsmt What do you think about this ?

One example I can think of is passing the same animation controller into multiple progress indicators on the same page and verifying at certain points they all have the same value?

This can't be done because everything is private in the class.
And there is no way to access private proprties, not even in tests...

This can't be done because everything is private in the class.

Testing in the framework focuses on observable behavior, not implementation details.

In progress_indicator_test.dart, there are tests that check what is painted by the progress indicators. A good test could have multiple progress indicators aligned in a Column, inserted at different times. Then some of them can have the same controller, and the ones with the same controller can be observed to have the same progress (i.e by the same paint extent, indicating a synchronized animation) while the others have a different paint extent.

Hi @Sten435, did you run into problems adding a test that fit the description in https://github.com/flutter/flutter/pull/170380/files#r2214098382?

Fix 165741

packages/flutter/test/material/radio_list_tile_test.dart

packages/flutter/test/widgets/tap_region_test.dart

victorsanni · 2025-07-17T19:13:28Z

packages/flutter/test/material/progress_indicator_test.dart

@@ -567,6 +567,80 @@ void main() {
    expect(tester.binding.transientCallbackCount, 0);
  });

+  testWidgets('LinearProgressIndicator reflects controller value', (WidgetTester tester) async {


This can't be done because everything is private in the class.

Testing in the framework focuses on observable behavior, not implementation details.

In progress_indicator_test.dart, there are tests that check what is painted by the progress indicators. A good test could have multiple progress indicators aligned in a Column, inserted at different times. Then some of them can have the same controller, and the ones with the same controller can be observed to have the same progress (i.e by the same paint extent, indicating a synchronized animation) while the others have a different paint extent.

dkwingsmt · 2025-07-30T18:26:07Z

Hi! @Sten435 Are you able to address the comments above so we can work together to get this landed? This is a great addition and I'm looking forward to it!

stan-at-work · 2025-07-31T07:05:38Z

Hi! @Sten435 Are you able to address the comments above so we can work together to get this landed? This is a great addition and I'm looking forward to it!

👋 Yeah, I’ve tried a couple of things over the week, but none worked. I’m new to testing—definitely to animation testing—so I have to learn everything about widget testing. It’s taking some time, but I think I’ll get there.

stan-at-work · 2025-07-31T07:06:47Z

Hi! @Sten435 Are you able to address the comments above so we can work together to get this landed? This is a great addition and I'm looking forward to it!

I got the LinearProgressIndicator to work using a shared theming controller, but the CircularProgressIndicator is a whole other level to test.

victorsanni

I'm beginning to wonder if this change is even needed. Why can't multiple progress indicators be supplied the same animation controller value directly in order to synchronize them? Do they need an entire AnimationController?

dkwingsmt · 2025-08-08T20:52:00Z

I think it's because it's a very common demand for progress indicators to stay in sync when multiple are present, so much that the widget is pretty much unusable if there isn't such a feature (i.e. all developers will have to build it by themselves.) The progress indicators are typically scattered across the widget tree and supplying values directly will cause the entire tree to be rebuilt every frame.

stan-at-work · 2025-08-11T06:43:53Z

I think it's because it's a very common demand for progress indicators to stay in sync when multiple are present, so much that the widget is pretty much unusable if there isn't such a feature (i.e. all developers will have to build it by themselves.) The progress indicators are typically scattered across the widget tree and supplying values directly will cause the entire tree to be rebuilt every frame.

I'm sorry for the delay. This will be done. But i'm verrry busy atm.

dkwingsmt · 2025-08-11T06:52:37Z

No worries, take you time!

Edit: If it makes you feel better, feel free to turn it into a draft or close it for now, and we can always come back to this.

stan-at-work · 2025-08-11T07:20:38Z

No worries, take you time!

Edit: If it makes you feel better, feel free to turn it into a draft or close it for now, and we can always come back to this.

No, i rather not close it.

Fix 165741

Sten435 · 2025-08-17T20:32:22Z

@dkwingsmt I have added a test, but after several days of effort, I was unable to create one for the circular progress indicator. This task is beyond my current experience with Flutter widget testing.

I invested considerable effort into the linear progress indicator test and managed to complete it, showing my commitment to delivering quality work. I have poured my heart and soul into this pull request. It may seem unusual, but I have learned a lot—and also suffered quite a bit—in the process. I don’t want to give up on this change, but I will not attempt a circular progress indicator test because I don’t want to drain myself further with Flutter widget tests.

I genuinely believe this change brings value and hope it can be merged. If the lack of this specific test prevents that, I understand, but I want to assure you that I have given my best effort. Sometimes, progress means recognizing the limits of what can reasonably be achieved while still giving your full effort.

It has been 148 days since I proposed this issue, and I have made my peace with the possibility that it may not be merged any time soon.

If the test is really needed, I accept that this pull request will be closed and hopefully picked up by someone else in the future. Hopefully.

Sten435 and others added 2 commits June 10, 2025 21:32

fix 165741

8a1b072

Merge pull request #4 from Sten435/fix-165741

825efbc

fix 165741

github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels Jun 10, 2025

This was referenced Jun 10, 2025

[Proposal] Allow Passing an AnimationController to CircularProgressIndicator #165741

Open

Allow Passing an AnimationController to CircularProgressIndicator #166190

Closed

dkwingsmt self-requested a review June 10, 2025 20:19

Sten435 changed the base branch from main to master June 10, 2025 21:33

Sten435 added 2 commits June 10, 2025 23:34

Merge branch 'master' into main

bd6f142

Merge branch 'master' into main

e70097a

Sten435 and others added 3 commits June 11, 2025 20:40

Merge branch 'master' into fix-165741

8480570

Fix indentation issue that caused the Linux analyze to fail

4efde13

Merge branch 'main' into fix-165741

e8e58c1

Sten435 added 2 commits June 12, 2025 10:10

fix 165741

4e8686e

Fix indentation issue that caused the Linux analyze to fail

18349ab

Sten435 force-pushed the main branch from 8c4f7f7 to 18349ab Compare June 12, 2025 08:10

dkwingsmt reviewed Jun 23, 2025

View reviewed changes

packages/flutter/lib/src/material/progress_indicator.dart Show resolved Hide resolved

packages/flutter/lib/src/material/progress_indicator.dart Outdated Show resolved Hide resolved

packages/flutter/test/material/progress_indicator_test.dart Outdated Show resolved Hide resolved

Sten435 added 4 commits June 23, 2025 19:14

Merge branch 'main' of https://github.com/sten435/flutter

07ed6df

# Conflicts: # packages/flutter/lib/src/material/progress_indicator.dart

Merge branch 'master'

8a7091f

Merge branch 'main' into fix-165741

0391df9

Merge branch 'master' into fix-165741

633bba3

victorsanni reviewed Jul 14, 2025

View reviewed changes

packages/flutter/test/material/progress_indicator_test.dart Show resolved Hide resolved

packages/flutter/test/material/progress_indicator_test.dart Show resolved Hide resolved

victorsanni reviewed Jul 14, 2025

View reviewed changes

packages/flutter/lib/src/material/progress_indicator.dart Show resolved Hide resolved

packages/flutter/lib/src/material/progress_indicator.dart Show resolved Hide resolved

Sten435 and others added 7 commits July 16, 2025 21:04

Merge branch 'master' into fix-165741

24847c1

FIX-165741 Fix tests

73ed374

Merge branch 'main' into fix-165741

ff46c0b

Merge pull request #7 from Sten435/fix-165741

24d9add

Fix 165741

Added docs

c173a39

Merge pull request #8 from Sten435/fix-165741

cd37464

Added docs

Merge branch 'master' into main

7d1660c

victorsanni reviewed Jul 16, 2025

View reviewed changes

Sten435 and others added 3 commits July 17, 2025 08:03

FIX formatting

e153e71

Merge branch 'main' into fix-165741

81722ae

Merge pull request #9 from Sten435/fix-165741

e6b6346

Fix 165741

victorsanni reviewed Jul 17, 2025

View reviewed changes

Merge branch 'master' into fix-165741

688e2e0

victorsanni reviewed Aug 7, 2025

View reviewed changes

Sten435 and others added 4 commits August 17, 2025 12:21

Merge branch 'master' into fix-165741

7b9c42b

Add progress indicator test

a229099

Merge branch 'master' into fix-165741

33c9903

Merge pull request #10 from Sten435/fix-165741

df0dd2a

Fix 165741

Allow Passing an AnimationController to CircularProgressIndicator (Fix #165741) #170380

Are you sure you want to change the base?

Allow Passing an AnimationController to CircularProgressIndicator (Fix #165741) #170380

Conversation

Sten435 commented Jun 10, 2025

Pre-launch Checklist

Uh oh!

Sten435 commented Jun 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stan-at-work commented Jun 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

This is not something I changed with my pull request. Does that mean this is not an issue for me, or what should I do to address it?

Uh oh!

dkwingsmt commented Jun 11, 2025

Uh oh!

stan-at-work commented Jun 11, 2025

Uh oh!

dkwingsmt commented Jun 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Sten435 commented Jun 11, 2025

Uh oh!

stan-at-work commented Jun 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dkwingsmt commented Jun 14, 2025

Uh oh!

stan-at-work commented Jun 14, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dkwingsmt commented Jul 15, 2025

Uh oh!

stan-at-work commented Jul 15, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dkwingsmt commented Jul 30, 2025

Uh oh!

stan-at-work commented Jul 31, 2025

Uh oh!

stan-at-work commented Jul 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

victorsanni left a comment

Choose a reason for hiding this comment

Uh oh!

dkwingsmt commented Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stan-at-work commented Aug 11, 2025

Uh oh!

dkwingsmt commented Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Sten435 commented Jun 10, 2025 •

edited

Loading

stan-at-work commented Jun 11, 2025 •

edited

Loading

dkwingsmt commented Jun 11, 2025 •

edited

Loading

stan-at-work commented Jun 13, 2025 •

edited

Loading

stan-at-work commented Jul 31, 2025 •

edited

Loading

dkwingsmt commented Aug 8, 2025 •

edited

Loading

dkwingsmt commented Aug 11, 2025 •

edited

Loading

Sten435 commented Aug 17, 2025 •

edited

Loading