diff --git a/packages/flutter/lib/src/material/constants.dart b/packages/flutter/lib/src/material/constants.dart index a156410114..2558975639 100644 --- a/packages/flutter/lib/src/material/constants.dart +++ b/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); diff --git a/packages/flutter/lib/src/material/tabs.dart b/packages/flutter/lib/src/material/tabs.dart index a2efdef60d..6e42c2c629 100644 --- a/packages/flutter/lib/src/material/tabs.dart +++ b/packages/flutter/lib/src/material/tabs.dart @@ -248,7 +248,6 @@ class _IndicatorPainter extends CustomPainter { TabController controller; List tabOffsets; Color color; - Animatable 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; } } diff --git a/packages/flutter/test/material/tabs_test.dart b/packages/flutter/test/material/tabs_test.dart index 25d19ffa8b..5f719b0e81 100644 --- a/packages/flutter/test/material/tabs_test.dart +++ b/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 { } } -Widget buildFrame({ List tabs, String value, bool isScrollable: false, Key tabBarKey }) { +Widget buildFrame({ + Key tabBarKey, + List 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 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 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 tabs = ['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 tabs = ['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); + }); } diff --git a/packages/flutter/test/rendering/mock_canvas.dart b/packages/flutter/test/rendering/mock_canvas.dart index 6e34f3187d..9b3b167717 100644 --- a/packages/flutter/test/rendering/mock_canvas.dart +++ b/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 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 _invocations = []; - - 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 get namedArguments => {}; - @override - List get positionalArguments => []; -} - -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 call); diff --git a/packages/flutter/test/rendering/recording_canvas.dart b/packages/flutter/test/rendering/recording_canvas.dart new file mode 100644 index 0000000000..e4366a351d --- /dev/null +++ b/packages/flutter/test/rendering/recording_canvas.dart @@ -0,0 +1,94 @@ +// 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 invocations = []; + + 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 get namedArguments => {}; + @override + List get positionalArguments => []; +}