Fix selected tab indicator tap animation (#8939)

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

@@ -25,7 +25,8 @@ const Duration kRadialReactionDuration = const Duration(milliseconds: 200);
 /// The value of the alpha channel to use when drawing a circular material ink response.
 const int kRadialReactionAlpha = 0x33;
 
-/// The duration
-const Duration kTabScrollDuration = const Duration(milliseconds: 200);
+/// The duration of the horizontal scroll animation that occurs when a tab is tapped.
+const Duration kTabScrollDuration = const Duration(milliseconds: 300);
 
+/// The padding added around material list items.
 const EdgeInsets kMaterialListPadding = const EdgeInsets.symmetric(vertical: 8.0);

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

@@ -248,7 +248,6 @@ class _IndicatorPainter extends CustomPainter {
   TabController controller;
   List<double> tabOffsets;
   Color color;
-  Animatable<Rect> indicatorTween;
   Rect currentRect;
 
   // tabOffsets[index] is the offset of the left edge of the tab at index, and
@@ -267,7 +266,7 @@ class _IndicatorPainter extends CustomPainter {
   void paint(Canvas canvas, Size size) {
     if (controller.indexIsChanging) {
       final Rect targetRect = indicatorRect(size, controller.index);
-      currentRect = Rect.lerp(currentRect ?? targetRect, targetRect, _indexChangeProgress(controller));
+      currentRect = Rect.lerp(targetRect, currentRect ?? targetRect, _indexChangeProgress(controller));
     } else {
       final int currentIndex = controller.index;
       final Rect left = currentIndex > 0 ? indicatorRect(size, currentIndex - 1) : null;
@@ -304,7 +303,8 @@ class _IndicatorPainter extends CustomPainter {
   bool shouldRepaint(_IndicatorPainter old) {
     return controller != old.controller ||
       tabOffsets?.length != old.tabOffsets?.length ||
-      tabOffsetsNotEqual(tabOffsets, old.tabOffsets);
+      tabOffsetsNotEqual(tabOffsets, old.tabOffsets) ||
+      currentRect != old.currentRect;
   }
 }
 

packages/flutter/test/material/tabs_test.dart

@@ -6,6 +6,8 @@ import 'package:flutter_test/flutter_test.dart';
 import 'package:flutter/material.dart';
 import 'package:flutter/widgets.dart';
 
+import '../rendering/recording_canvas.dart';
+
 class StateMarker extends StatefulWidget {
   StateMarker({ Key key, this.child }) : super(key: key);
 
@@ -26,7 +28,13 @@ class StateMarkerState extends State<StateMarker> {
   }
 }
 
-Widget buildFrame({ List<String> tabs, String value, bool isScrollable: false, Key tabBarKey }) {
+Widget buildFrame({
+    Key tabBarKey,
+    List<String> tabs,
+    String value,
+    bool isScrollable: false,
+    Color indicatorColor,
+  }) {
   return new Material(
     child: new DefaultTabController(
       initialIndex: tabs.indexOf(value),
@@ -35,6 +43,7 @@ Widget buildFrame({ List<String> tabs, String value, bool isScrollable: false, K
         key: tabBarKey,
         tabs: tabs.map((String tab) => new Tab(text: tab)).toList(),
         isScrollable: isScrollable,
+        indicatorColor: indicatorColor,
       ),
     ),
   );
@@ -102,6 +111,19 @@ Widget buildLeftRightApp({ List<String> tabs, String value }) {
   );
 }
 
+class TabIndicatorRecordingCanvas extends TestRecordingCanvas {
+  TabIndicatorRecordingCanvas(this.indicatorColor);
+
+  final Color indicatorColor;
+  Rect indicatorRect;
+
+  @override
+  void drawRect(Rect rect, Paint paint) {
+    if (paint.color == indicatorColor)
+      indicatorRect = rect;
+  }
+}
+
 void main() {
   testWidgets('TabBar tap selects tab', (WidgetTester tester) async {
     final List<String> tabs = <String>['A', 'B', 'C'];
@@ -673,4 +695,39 @@ void main() {
     expect(find.text('First'), findsOneWidget);
     expect(find.text('Second'), findsNothing);
   });
+
+  testWidgets('TabBar tap animates the selection indicator', (WidgetTester tester) async {
+    // This is a regression test for https://github.com/flutter/flutter/issues/7479
+
+    final List<String> tabs = <String>['A', 'B'];
+
+    const Color indicatorColor = const Color(0xFFFF0000);
+    await tester.pumpWidget(buildFrame(tabs: tabs, value: 'A', indicatorColor: indicatorColor));
+
+    final RenderBox box = tester.renderObject(find.byType(TabBar));
+    final TabIndicatorRecordingCanvas canvas = new TabIndicatorRecordingCanvas(indicatorColor);
+    final TestRecordingPaintingContext context = new TestRecordingPaintingContext(canvas);
+
+    box.paint(context, Offset.zero);
+    final Rect indicatorRect0 = canvas.indicatorRect;
+    expect(indicatorRect0.left, 0.0);
+    expect(indicatorRect0.width, 400.0);
+    expect(indicatorRect0.height, 2.0);
+
+    await tester.tap(find.text('B'));
+    await tester.pump();
+    await tester.pump(const Duration(milliseconds: 50));
+    box.paint(context, Offset.zero);
+    final Rect indicatorRect1 = canvas.indicatorRect;
+    expect(indicatorRect1.left, greaterThan(indicatorRect0.left));
+    expect(indicatorRect1.right, lessThan(800.0));
+    expect(indicatorRect1.height, 2.0);
+
+    await tester.pump(const Duration(milliseconds: 300));
+    box.paint(context, Offset.zero);
+    final Rect indicatorRect2 = canvas.indicatorRect;
+    expect(indicatorRect2.left, 400.0);
+    expect(indicatorRect2.width, 400.0);
+    expect(indicatorRect2.height, 2.0);
+  });
 }

packages/flutter/test/rendering/mock_canvas.dart

@@ -8,6 +8,8 @@ import 'package:flutter/foundation.dart';
 import 'package:flutter/rendering.dart';
 import 'package:flutter_test/flutter_test.dart';
 
+import 'recording_canvas.dart';
+
 /// Matches objects or functions that paint a display list that matches the
 /// canvas calls described by the pattern.
 ///
@@ -292,8 +294,8 @@ class _TestRecordingCanvasPatternMatcher extends Matcher implements PaintPattern
 
   @override
   bool matches(Object object, Map<dynamic, dynamic> matchState) {
-    final _TestRecordingCanvas canvas = new _TestRecordingCanvas();
-    final _TestRecordingPaintingContext context = new _TestRecordingPaintingContext(canvas);
+    final TestRecordingCanvas canvas = new TestRecordingCanvas();
+    final TestRecordingPaintingContext context = new TestRecordingPaintingContext(canvas);
     if (object is _ContextPainterFunction) {
       final _ContextPainterFunction function = object;
       function(context, Offset.zero);
@@ -315,12 +317,12 @@ class _TestRecordingCanvasPatternMatcher extends Matcher implements PaintPattern
       }
     }
     final StringBuffer description = new StringBuffer();
-    final bool result = _evaluatePredicates(canvas._invocations, description);
+    final bool result = _evaluatePredicates(canvas.invocations, description);
     if (!result) {
       const String indent = '\n            '; // the length of '   Which: ' in spaces, plus two more
-      if (canvas._invocations.isNotEmpty)
+      if (canvas.invocations.isNotEmpty)
         description.write(' The complete display list was:');
-        for (Invocation call in canvas._invocations)
+        for (Invocation call in canvas.invocations)
           description.write('$indent${_describeInvocation(call)}');
     }
     matchState[this] = description.toString();
@@ -375,76 +377,6 @@ class _TestRecordingCanvasPatternMatcher extends Matcher implements PaintPattern
   }
 }
 
-class _TestRecordingCanvas implements Canvas {
-  final List<Invocation> _invocations = <Invocation>[];
-
-  int _saveCount = 0;
-
-  @override
-  int getSaveCount() => _saveCount;
-
-  @override
-  void save() {
-    _saveCount += 1;
-    _invocations.add(new _MethodCall(#save));
-  }
-
-  @override
-  void restore() {
-    _saveCount -= 1;
-    assert(_saveCount >= 0);
-    _invocations.add(new _MethodCall(#restore));
-  }
-
-  @override
-  void noSuchMethod(Invocation invocation) {
-    _invocations.add(invocation);
-  }
-}
-
-class _MethodCall implements Invocation {
-  _MethodCall(this._name);
-  final Symbol _name;
-  @override
-  bool get isAccessor => false;
-  @override
-  bool get isGetter => false;
-  @override
-  bool get isMethod => true;
-  @override
-  bool get isSetter => false;
-  @override
-  Symbol get memberName => _name;
-  @override
-  Map<Symbol, dynamic> get namedArguments => <Symbol, dynamic>{};
-  @override
-  List<dynamic> get positionalArguments => <dynamic>[];
-}
-
-class _TestRecordingPaintingContext implements PaintingContext {
-  _TestRecordingPaintingContext(this.canvas);
-
-  @override
-  final Canvas canvas;
-
-  @override
-  void paintChild(RenderObject child, Offset offset) {
-    child.paint(this, offset);
-  }
-
-  @override
-  void pushClipRect(bool needsCompositing, Offset offset, Rect clipRect, PaintingContextCallback painter) {
-    canvas.save();
-    canvas.clipRect(clipRect.shift(offset));
-    painter(this, offset);
-    canvas.restore();
-  }
-
-  @override
-  void noSuchMethod(Invocation invocation) {
-  }
-}
-
 abstract class _PaintPredicate {
   void match(Iterator<Invocation> call);
 

packages/flutter/test/rendering/recording_canvas.dart

+// 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/rendering.dart';
+
+/// A [Canvas] for tests that records its method calls.
+///
+/// This class can be used in conjuction with [TestRecordingPaintingContext]
+/// to record the [Canvas] method calls made by a renderer. For example:
+///
+/// ```dart
+/// RenderBox box = tester.renderObject(find.text('ABC'));
+/// TestRecordingCanvas canvas = new TestRecordingCanvas();
+/// TestRecordingPaintingContext context = new TestRecordingPaintingContext(canvas);
+/// box.paint(context, Offset.zero);
+/// // Now test the expected canvas.invocations.
+/// ```
+///
+/// In some cases it may be useful to define a subclass that overrides the
+/// Canvas methods the test is checking and squirrels away the parameters
+/// that the test requires.
+class TestRecordingCanvas implements Canvas {
+  /// All of the method calls on this canvas.
+  final List<Invocation> invocations = <Invocation>[];
+
+  int _saveCount = 0;
+
+  @override
+  int getSaveCount() => _saveCount;
+
+  @override
+  void save() {
+    _saveCount += 1;
+    invocations.add(new _MethodCall(#save));
+  }
+
+  @override
+  void restore() {
+    _saveCount -= 1;
+    assert(_saveCount >= 0);
+    invocations.add(new _MethodCall(#restore));
+  }
+
+  @override
+  void noSuchMethod(Invocation invocation) {
+    invocations.add(invocation);
+  }
+}
+
+/// A [PaintingContext] for tests that use [TestRecordingCanvas].
+class TestRecordingPaintingContext implements PaintingContext {
+  /// Creates a [PaintingContext] for tests that use [TestRecordingCanvas].
+  TestRecordingPaintingContext(this.canvas);
+
+  @override
+  final Canvas canvas;
+
+  @override
+  void paintChild(RenderObject child, Offset offset) {
+    child.paint(this, offset);
+  }
+
+  @override
+  void pushClipRect(bool needsCompositing, Offset offset, Rect clipRect, PaintingContextCallback painter) {
+    canvas.save();
+    canvas.clipRect(clipRect.shift(offset));
+    painter(this, offset);
+    canvas.restore();
+  }
+
+  @override
+  void noSuchMethod(Invocation invocation) {
+  }
+}
+
+class _MethodCall implements Invocation {
+  _MethodCall(this._name);
+  final Symbol _name;
+  @override
+  bool get isAccessor => false;
+  @override
+  bool get isGetter => false;
+  @override
+  bool get isMethod => true;
+  @override
+  bool get isSetter => false;
+  @override
+  Symbol get memberName => _name;
+  @override
+  Map<Symbol, dynamic> get namedArguments => <Symbol, dynamic>{};
+  @override
+  List<dynamic> get positionalArguments => <dynamic>[];
+}