Fix the progress indicator demo #3367
Conversation
|
|
| AnimationController controller; | ||
| @override | ||
| void dispose() { | ||
| _controller.stop(); |
There was a problem hiding this comment.
Woh. So any time we use an AnimationController in a Widget and don't implement dispose it's a bug?
There was a problem hiding this comment.
for symmetry when disposing from a dispose() function, use dispose() rather than stop().
There was a problem hiding this comment.
Animations that repeat need to be stopped. In this case the animation's status listener makes it cycle, so it definitely needed to be stopped.
The test framework verifies that tests don't leave behind running animations (thanks Ian) so a series of simple tests that run the gallery demos should catch this kind of thing.
Will file an issue and then do that.
There was a problem hiding this comment.
(credit for catching the leaking frame callbacks goes to adam, i just padded out the error message you get)
| List<Layer> layers2 = tester.layers; | ||
| expect(layers1, isNot(equals(layers2))); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
did this test not fail before?
|
LGTM but add a test for the gallery app :-) Actually might be worth making a test for the gallery app that just visits each page in turn and then quits, just to see if anything is left pending at the end. |
4 participants
When the demo is disposed, stop its animation. Also made some minor code-cosmetic changes.
Added some simple progress indicator tests to ensure that the animations similarly stop.