From f8a80b1da69214b1a6abfae9285efe13cb408995 Mon Sep 17 00:00:00 2001 From: Jason Simmons Date: Thu, 4 Aug 2016 10:26:35 -0700 Subject: [PATCH] Notify a Scrollable that the associated animation has stopped (#5209) This also required changing the AnimationController state transition logic to signal completion of the animation during the tick that finishes the simulation. Fixes https://github.com/flutter/flutter/issues/3675 --- .../src/animation/animation_controller.dart | 49 +++++++++++++------ .../flutter/lib/src/widgets/scrollable.dart | 8 ++- .../animation/animation_controller_test.dart | 20 ++++++++ .../test/widget/scrollable_dispose_test.dart | 27 ++++++++++ 4 files changed, 87 insertions(+), 17 deletions(-) create mode 100644 packages/flutter/test/widget/scrollable_dispose_test.dart diff --git a/packages/flutter/lib/src/animation/animation_controller.dart b/packages/flutter/lib/src/animation/animation_controller.dart index bb54bc72490..e20e4bd87ad 100644 --- a/packages/flutter/lib/src/animation/animation_controller.dart +++ b/packages/flutter/lib/src/animation/animation_controller.dart @@ -53,8 +53,9 @@ class AnimationController extends Animation this.upperBound: 1.0 }) { assert(upperBound >= lowerBound); - _value = (value ?? lowerBound).clamp(lowerBound, upperBound); + _direction = _AnimationDirection.forward; _ticker = new Ticker(_tick); + _internalSetValue(value ?? lowerBound); } /// Creates an animation controller with no upper or lower bound for its value. @@ -71,10 +72,11 @@ class AnimationController extends Animation this.duration, this.debugLabel }) : lowerBound = double.NEGATIVE_INFINITY, - upperBound = double.INFINITY, - _value = value { + upperBound = double.INFINITY { assert(value != null); + _direction = _AnimationDirection.forward; _ticker = new Ticker(_tick); + _internalSetValue(value); } /// The value at which this animation is deemed to be dismissed. @@ -119,11 +121,23 @@ class AnimationController extends Animation set value(double newValue) { assert(newValue != null); stop(); - _value = newValue.clamp(lowerBound, upperBound); + _internalSetValue(newValue); notifyListeners(); _checkStatusChanged(); } + void _internalSetValue(double newValue) { + _value = newValue.clamp(lowerBound, upperBound); + if (_value == lowerBound) { + _status = AnimationStatus.dismissed; + } else if (_value == upperBound) { + _status = AnimationStatus.completed; + } else + _status = (_direction == _AnimationDirection.forward) ? + AnimationStatus.forward : + AnimationStatus.reverse; + } + /// The amount of time that has passed between the time the animation started and the most recent tick of the animation. /// /// If the controller is not animating, the last elapsed duration is null. @@ -136,29 +150,22 @@ class AnimationController extends Animation _AnimationDirection _direction; @override - AnimationStatus get status { - if (!isAnimating && value == upperBound) - return AnimationStatus.completed; - if (!isAnimating && value == lowerBound) - return AnimationStatus.dismissed; - return _direction == _AnimationDirection.forward ? - AnimationStatus.forward : - AnimationStatus.reverse; - } + AnimationStatus get status => _status; + AnimationStatus _status; /// Starts running this animation forwards (towards the end). Future forward({ double from }) { + _direction = _AnimationDirection.forward; if (from != null) value = from; - _direction = _AnimationDirection.forward; return animateTo(upperBound); } /// Starts running this animation in reverse (towards the beginning). Future reverse({ double from }) { + _direction = _AnimationDirection.reverse; if (from != null) value = from; - _direction = _AnimationDirection.reverse; return animateTo(lowerBound); } @@ -174,6 +181,9 @@ class AnimationController extends Animation stop(); if (simulationDuration == Duration.ZERO) { assert(value == target); + _status = (_direction == _AnimationDirection.forward) ? + AnimationStatus.completed : + AnimationStatus.dismissed; _checkStatusChanged(); return new Future.value(); } @@ -215,6 +225,9 @@ class AnimationController extends Animation _lastElapsedDuration = Duration.ZERO; _value = simulation.x(0.0).clamp(lowerBound, upperBound); Future result = _ticker.start(); + _status = (_direction == _AnimationDirection.forward) ? + AnimationStatus.forward : + AnimationStatus.reverse; _checkStatusChanged(); return result; } @@ -245,8 +258,12 @@ class AnimationController extends Animation _lastElapsedDuration = elapsed; double elapsedInSeconds = elapsed.inMicroseconds.toDouble() / Duration.MICROSECONDS_PER_SECOND; _value = _simulation.x(elapsedInSeconds).clamp(lowerBound, upperBound); - if (_simulation.isDone(elapsedInSeconds)) + if (_simulation.isDone(elapsedInSeconds)) { + _status = (_direction == _AnimationDirection.forward) ? + AnimationStatus.completed : + AnimationStatus.dismissed; stop(); + } notifyListeners(); _checkStatusChanged(); } diff --git a/packages/flutter/lib/src/widgets/scrollable.dart b/packages/flutter/lib/src/widgets/scrollable.dart index d2476d6a2b8..b502a49d4e4 100644 --- a/packages/flutter/lib/src/widgets/scrollable.dart +++ b/packages/flutter/lib/src/widgets/scrollable.dart @@ -249,7 +249,8 @@ class ScrollableState extends State { void initState() { super.initState(); _controller = new AnimationController.unbounded() - ..addListener(_handleAnimationChanged); + ..addListener(_handleAnimationChanged) + ..addStatusListener(_handleAnimationStatusChanged); _scrollOffset = PageStorage.of(context)?.readState(context) ?? config.initialScrollOffset ?? 0.0; } @@ -365,6 +366,11 @@ class ScrollableState extends State { _setScrollOffset(_controller.value); } + void _handleAnimationStatusChanged(AnimationStatus status) { + if (!_controller.isAnimating) + _simulation = null; + } + void _setScrollOffset(double newScrollOffset, { DragUpdateDetails details }) { if (_scrollOffset == newScrollOffset) return; diff --git a/packages/flutter/test/animation/animation_controller_test.dart b/packages/flutter/test/animation/animation_controller_test.dart index 7cb9788ba23..d2f76135c66 100644 --- a/packages/flutter/test/animation/animation_controller_test.dart +++ b/packages/flutter/test/animation/animation_controller_test.dart @@ -132,6 +132,26 @@ void main() { expect(controller.value, equals(0.0)); }); + test('Forward only from value', () { + AnimationController controller = new AnimationController( + duration: const Duration(milliseconds: 100) + ); + List valueLog = []; + List statusLog = []; + controller + ..addStatusListener((AnimationStatus status) { + statusLog.add(status); + }) + ..addListener(() { + valueLog.add(controller.value); + }); + + controller.forward(from: 0.2); + expect(statusLog, equals([ AnimationStatus.forward ])); + expect(valueLog, equals([ 0.2 ])); + expect(controller.value, equals(0.2)); + }); + test('Can fling to upper and lower bounds', () { AnimationController controller = new AnimationController( duration: const Duration(milliseconds: 100) diff --git a/packages/flutter/test/widget/scrollable_dispose_test.dart b/packages/flutter/test/widget/scrollable_dispose_test.dart new file mode 100644 index 00000000000..4148ea2f0a0 --- /dev/null +++ b/packages/flutter/test/widget/scrollable_dispose_test.dart @@ -0,0 +1,27 @@ +// Copyright 2016 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'package:flutter_test/flutter_test.dart'; +import 'package:flutter/widgets.dart'; + +import 'test_widgets.dart'; + +void main() { + testWidgets('simultaneously dispose a widget and end the scroll animation', (WidgetTester tester) async { + List textWidgets = []; + for (int i = 0; i < 250; i++) + textWidgets.add(new Text('$i')); + await tester.pumpWidget(new FlipWidget( + left: new Block(children: textWidgets), + right: new Container() + )); + + await tester.fling(find.byType(Scrollable), new Offset(0.0, -200.0), 1000.0); + await tester.pump(); + + FlipWidgetState flipWidget = tester.state(find.byType(FlipWidget)); + flipWidget.flip(); + await tester.pump(new Duration(hours: 5)); + }); +}