diff --git a/packages/flutter/lib/src/animation/animation_controller.dart b/packages/flutter/lib/src/animation/animation_controller.dart index 26c629ea63..7f4b8c7eca 100644 --- a/packages/flutter/lib/src/animation/animation_controller.dart +++ b/packages/flutter/lib/src/animation/animation_controller.dart @@ -335,6 +335,9 @@ class AnimationController extends Animation final double range = upperBound - lowerBound; final double remainingFraction = range.isFinite ? (target - _value).abs() / range : 1.0; simulationDuration = this.duration * remainingFraction; + } else if (target == value) { + // Already at target, don't animate. + simulationDuration = Duration.ZERO; } stop(); if (simulationDuration == Duration.ZERO) { diff --git a/packages/flutter/lib/src/cupertino/button.dart b/packages/flutter/lib/src/cupertino/button.dart index b9f0e9f49b..7fbebb2f58 100644 --- a/packages/flutter/lib/src/cupertino/button.dart +++ b/packages/flutter/lib/src/cupertino/button.dart @@ -2,6 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'dart:async'; + import 'package:flutter/foundation.dart'; import 'package:flutter/widgets.dart'; @@ -110,7 +112,7 @@ class CupertinoButton extends StatefulWidget { class _CupertinoButtonState extends State with SingleTickerProviderStateMixin { // Eyeballed values. Feel free to tweak. static const Duration kFadeOutDuration = const Duration(milliseconds: 10); - static const Duration kFadeInDuration = const Duration(milliseconds: 350); + static const Duration kFadeInDuration = const Duration(milliseconds: 100); Tween _opacityTween; AnimationController _animationController; @@ -146,16 +148,40 @@ class _CupertinoButtonState extends State with SingleTickerProv _setTween(); } - void _handleTapDown(PointerDownEvent event) { - _animationController.animateTo(1.0, duration: kFadeOutDuration); + bool _buttonHeldDown = false; + + void _handleTapDown(TapDownDetails event) { + if (!_buttonHeldDown) { + _buttonHeldDown = true; + _animate(); + } } - void _handleTapUp(PointerUpEvent event) { - _animationController.animateTo(0.0, duration: kFadeInDuration); + void _handleTapUp(TapUpDetails event) { + if (_buttonHeldDown) { + _buttonHeldDown = false; + _animate(); + } } - void _handleTapCancel(PointerCancelEvent event) { - _animationController.animateTo(0.0, duration: kFadeInDuration); + void _handleTapCancel() { + if (_buttonHeldDown) { + _buttonHeldDown = false; + _animate(); + } + } + + void _animate() { + if (_animationController.isAnimating) + return; + final bool wasHeldDown = _buttonHeldDown; + final Future ticker = _buttonHeldDown + ? _animationController.animateTo(1.0, duration: kFadeOutDuration) + : _animationController.animateTo(0.0, duration: kFadeInDuration); + ticker.then((Null value) { + if (mounted && wasHeldDown != _buttonHeldDown) + _animate(); + }); } @override @@ -163,46 +189,44 @@ class _CupertinoButtonState extends State with SingleTickerProv final bool enabled = widget.enabled; final Color backgroundColor = widget.color; - return new Listener( - onPointerDown: enabled ? _handleTapDown : null, - onPointerUp: enabled ? _handleTapUp : null, - onPointerCancel: enabled ? _handleTapCancel : null, - child: new GestureDetector( - onTap: widget.onPressed, - child: new ConstrainedBox( - constraints: widget.minSize == null - ? const BoxConstraints() - : new BoxConstraints( - minWidth: widget.minSize, - minHeight: widget.minSize, - ), - child: new FadeTransition( - opacity: _opacityTween.animate(new CurvedAnimation( - parent: _animationController, - curve: Curves.decelerate, - )), - child: new DecoratedBox( - decoration: new BoxDecoration( - borderRadius: widget.borderRadius, - color: backgroundColor != null && !enabled - ? _kDisabledBackground - : backgroundColor, - ), - child: new Padding( - padding: widget.padding ?? (backgroundColor != null - ? _kBackgroundButtonPadding - : _kButtonPadding), - child: new Center( - widthFactor: 1.0, - heightFactor: 1.0, - child: new DefaultTextStyle( - style: backgroundColor != null - ? _kBackgroundButtonTextStyle - : enabled - ? _kButtonTextStyle - : _kDisabledButtonTextStyle, - child: widget.child, - ), + return new GestureDetector( + onTapDown: enabled ? _handleTapDown : null, + onTapUp: enabled ? _handleTapUp : null, + onTapCancel: enabled ? _handleTapCancel : null, + onTap: widget.onPressed, + child: new ConstrainedBox( + constraints: widget.minSize == null + ? const BoxConstraints() + : new BoxConstraints( + minWidth: widget.minSize, + minHeight: widget.minSize, + ), + child: new FadeTransition( + opacity: _opacityTween.animate(new CurvedAnimation( + parent: _animationController, + curve: Curves.decelerate, + )), + child: new DecoratedBox( + decoration: new BoxDecoration( + borderRadius: widget.borderRadius, + color: backgroundColor != null && !enabled + ? _kDisabledBackground + : backgroundColor, + ), + child: new Padding( + padding: widget.padding ?? (backgroundColor != null + ? _kBackgroundButtonPadding + : _kButtonPadding), + child: new Center( + widthFactor: 1.0, + heightFactor: 1.0, + child: new DefaultTextStyle( + style: backgroundColor != null + ? _kBackgroundButtonTextStyle + : enabled + ? _kButtonTextStyle + : _kDisabledButtonTextStyle, + child: widget.child, ), ), ), diff --git a/packages/flutter/lib/src/widgets/scroll_position_with_single_context.dart b/packages/flutter/lib/src/widgets/scroll_position_with_single_context.dart index 82b29a9907..84449c75c5 100644 --- a/packages/flutter/lib/src/widgets/scroll_position_with_single_context.dart +++ b/packages/flutter/lib/src/widgets/scroll_position_with_single_context.dart @@ -174,6 +174,12 @@ class ScrollPositionWithSingleContext extends ScrollPosition implements ScrollAc @required Duration duration, @required Curve curve, }) { + if (nearEqual(to, pixels, physics.tolerance.distance)) { + // Skip the animation, go straight to the position as we are already close. + jumpTo(to); + return new Future.value(); + } + final DrivenScrollActivity activity = new DrivenScrollActivity( this, from: pixels, diff --git a/packages/flutter/test/animation/animation_controller_test.dart b/packages/flutter/test/animation/animation_controller_test.dart index e16da0f164..8c52774505 100644 --- a/packages/flutter/test/animation/animation_controller_test.dart +++ b/packages/flutter/test/animation/animation_controller_test.dart @@ -292,4 +292,93 @@ void main() { expect((){ controller.repeat(period: null); }, throwsFlutterError); }); + test('Do not animate if already at target', () { + final List statusLog = []; + + final AnimationController controller = new AnimationController( + value: 0.5, + vsync: const TestVSync(), + )..addStatusListener(statusLog.add); + + expect(controller.value, equals(0.5)); + controller.animateTo(0.5, duration: const Duration(milliseconds: 100)); + expect(statusLog, equals([ AnimationStatus.completed ])); + expect(controller.value, equals(0.5)); + }); + + test('Do not animate to upperBound if already at upperBound', () { + final List statusLog = []; + + final AnimationController controller = new AnimationController( + value: 1.0, + upperBound: 1.0, + lowerBound: 0.0, + vsync: const TestVSync(), + )..addStatusListener(statusLog.add); + + expect(controller.value, equals(1.0)); + controller.animateTo(1.0, duration: const Duration(milliseconds: 100)); + expect(statusLog, equals([ AnimationStatus.completed ])); + expect(controller.value, equals(1.0)); + }); + + test('Do not animate to lowerBound if already at lowerBound', () { + final List statusLog = []; + + final AnimationController controller = new AnimationController( + value: 0.0, + upperBound: 1.0, + lowerBound: 0.0, + vsync: const TestVSync(), + )..addStatusListener(statusLog.add); + + expect(controller.value, equals(0.0)); + controller.animateTo(0.0, duration: const Duration(milliseconds: 100)); + expect(statusLog, equals([ AnimationStatus.completed ])); + expect(controller.value, equals(0.0)); + }); + + test('Do not animate if already at target mid-flight (forward)', () { + final List statusLog = []; + final AnimationController controller = new AnimationController( + value: 0.0, + duration: const Duration(milliseconds: 1000), + vsync: const TestVSync(), + )..addStatusListener(statusLog.add); + + expect(controller.value, equals(0.0)); + + controller.forward(); + tick(const Duration(milliseconds: 0)); + tick(const Duration(milliseconds: 500)); + expect(controller.value, inInclusiveRange(0.4, 0.6)); + expect(statusLog, equals([ AnimationStatus.forward ])); + + final double currentValue = controller.value; + controller.animateTo(currentValue, duration: const Duration(milliseconds: 100)); + expect(statusLog, equals([ AnimationStatus.forward, AnimationStatus.completed ])); + expect(controller.value, currentValue); + }); + + test('Do not animate if already at target mid-flight (reverse)', () { + final List statusLog = []; + final AnimationController controller = new AnimationController( + value: 1.0, + duration: const Duration(milliseconds: 1000), + vsync: const TestVSync(), + )..addStatusListener(statusLog.add); + + expect(controller.value, equals(1.0)); + + controller.reverse(); + tick(const Duration(milliseconds: 0)); + tick(const Duration(milliseconds: 500)); + expect(controller.value, inInclusiveRange(0.4, 0.6)); + expect(statusLog, equals([ AnimationStatus.reverse ])); + + final double currentValue = controller.value; + controller.animateTo(currentValue, duration: const Duration(milliseconds: 100)); + expect(statusLog, equals([ AnimationStatus.reverse, AnimationStatus.dismissed ])); + expect(controller.value, currentValue); + }); } diff --git a/packages/flutter/test/widgets/scrollable_animations_test.dart b/packages/flutter/test/widgets/scrollable_animations_test.dart new file mode 100644 index 0000000000..352212da1f --- /dev/null +++ b/packages/flutter/test/widgets/scrollable_animations_test.dart @@ -0,0 +1,72 @@ +// Copyright 2017 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/scheduler.dart'; +import 'package:flutter/widgets.dart'; + +void main() { + testWidgets('Does not animate if already at target position', (WidgetTester tester) async { + final List textWidgets = []; + for (int i = 0; i < 80; i++) + textWidgets.add(new Text('$i')); + final ScrollController controller = new ScrollController(); + await tester.pumpWidget(new ListView( + children: textWidgets, + controller: controller, + )); + + expectNoAnimation(); + final double currentPosition = controller.position.pixels; + controller.position.animateTo(currentPosition, duration: const Duration(seconds: 10), curve: Curves.linear); + + expectNoAnimation(); + expect(controller.position.pixels, currentPosition); + }); + + testWidgets('Does not animate if already at target position within tolerance', (WidgetTester tester) async { + final List textWidgets = []; + for (int i = 0; i < 80; i++) + textWidgets.add(new Text('$i')); + final ScrollController controller = new ScrollController(); + await tester.pumpWidget(new ListView( + children: textWidgets, + controller: controller, + )); + + expectNoAnimation(); + + final double halfTolerance = controller.position.physics.tolerance.distance / 2; + expect(halfTolerance, isNonZero); + final double targetPosition = controller.position.pixels + halfTolerance; + controller.position.animateTo(targetPosition, duration: const Duration(seconds: 10), curve: Curves.linear); + + expectNoAnimation(); + expect(controller.position.pixels, targetPosition); + }); + + testWidgets('Animates if going to a position outside of tolerance', (WidgetTester tester) async { + final List textWidgets = []; + for (int i = 0; i < 80; i++) + textWidgets.add(new Text('$i')); + final ScrollController controller = new ScrollController(); + await tester.pumpWidget(new ListView( + children: textWidgets, + controller: controller, + )); + + expectNoAnimation(); + + final double doubleTolerance = controller.position.physics.tolerance.distance * 2; + expect(doubleTolerance, isNonZero); + final double targetPosition = controller.position.pixels + doubleTolerance; + controller.position.animateTo(targetPosition, duration: const Duration(seconds: 10), curve: Curves.linear); + + expect(SchedulerBinding.instance.transientCallbackCount, equals(1), reason: 'Expected an animation.'); + }); +} + +void expectNoAnimation() { + expect(SchedulerBinding.instance.transientCallbackCount, equals(0), reason: 'Expected no animation.'); +}