From d50bfd5f66df1f66abc5d21a402ba9ec5a355dbc Mon Sep 17 00:00:00 2001 From: Shi-Hao Hong Date: Tue, 27 Oct 2020 21:52:04 +0800 Subject: [PATCH] Fix null issue with dynamically updating from zero tabs for TabBar (#69005) --- packages/flutter/lib/src/material/tabs.dart | 43 +++++------ packages/flutter/test/material/tabs_test.dart | 75 +++++++++++++++++++ 2 files changed, 93 insertions(+), 25 deletions(-) diff --git a/packages/flutter/lib/src/material/tabs.dart b/packages/flutter/lib/src/material/tabs.dart index daabd26fb7..9e05045c4d 100644 --- a/packages/flutter/lib/src/material/tabs.dart +++ b/packages/flutter/lib/src/material/tabs.dart @@ -4,6 +4,7 @@ import 'dart:ui' show lerpDouble; +import 'package:flutter/foundation.dart'; import 'package:flutter/rendering.dart'; import 'package:flutter/widgets.dart'; import 'package:flutter/gestures.dart' show DragStartBehavior; @@ -331,8 +332,12 @@ class _IndicatorPainter extends CustomPainter { final TabBarIndicatorSize? indicatorSize; final List tabKeys; - late List _currentTabOffsets; - late TextDirection _currentTextDirection; + // _currentTabOffsets and _currentTextDirection are set each time TabBar + // layout is completed. These values can be null when TabBar contains no + // tabs, since there are nothing to lay out. + List? _currentTabOffsets; + TextDirection? _currentTextDirection; + Rect? _currentRect; BoxPainter? _painter; bool _needsPaint = false; @@ -344,38 +349,38 @@ class _IndicatorPainter extends CustomPainter { _painter?.dispose(); } - void saveTabOffsets(List tabOffsets, TextDirection textDirection) { + void saveTabOffsets(List? tabOffsets, TextDirection? textDirection) { _currentTabOffsets = tabOffsets; _currentTextDirection = textDirection; } // _currentTabOffsets[index] is the offset of the start edge of the tab at index, and // _currentTabOffsets[_currentTabOffsets.length] is the end edge of the last tab. - int get maxTabIndex => _currentTabOffsets.length - 2; + int get maxTabIndex => _currentTabOffsets!.length - 2; double centerOf(int tabIndex) { assert(_currentTabOffsets != null); - assert(_currentTabOffsets.isNotEmpty); + assert(_currentTabOffsets!.isNotEmpty); assert(tabIndex >= 0); assert(tabIndex <= maxTabIndex); - return (_currentTabOffsets[tabIndex] + _currentTabOffsets[tabIndex + 1]) / 2.0; + return (_currentTabOffsets![tabIndex] + _currentTabOffsets![tabIndex + 1]) / 2.0; } Rect indicatorRect(Size tabBarSize, int tabIndex) { assert(_currentTabOffsets != null); assert(_currentTextDirection != null); - assert(_currentTabOffsets.isNotEmpty); + assert(_currentTabOffsets!.isNotEmpty); assert(tabIndex >= 0); assert(tabIndex <= maxTabIndex); double tabLeft, tabRight; - switch (_currentTextDirection) { + switch (_currentTextDirection!) { case TextDirection.rtl: - tabLeft = _currentTabOffsets[tabIndex + 1]; - tabRight = _currentTabOffsets[tabIndex]; + tabLeft = _currentTabOffsets![tabIndex + 1]; + tabRight = _currentTabOffsets![tabIndex]; break; case TextDirection.ltr: - tabLeft = _currentTabOffsets[tabIndex]; - tabRight = _currentTabOffsets[tabIndex + 1]; + tabLeft = _currentTabOffsets![tabIndex]; + tabRight = _currentTabOffsets![tabIndex + 1]; break; } @@ -426,25 +431,13 @@ class _IndicatorPainter extends CustomPainter { _painter!.paint(canvas, _currentRect!.topLeft, configuration); } - static bool _tabOffsetsEqual(List a, List b) { - // TODO(shihaohong): The following null check should be replaced when a fix - // for https://github.com/flutter/flutter/issues/40014 is available. - if (a == null || b == null || a.length != b.length) - return false; - for (int i = 0; i < a.length; i += 1) { - if (a[i] != b[i]) - return false; - } - return true; - } - @override bool shouldRepaint(_IndicatorPainter old) { return _needsPaint || controller != old.controller || indicator != old.indicator || tabKeys.length != old.tabKeys.length - || (!_tabOffsetsEqual(_currentTabOffsets, old._currentTabOffsets)) + || (!listEquals(_currentTabOffsets, old._currentTabOffsets)) || _currentTextDirection != old._currentTextDirection; } } diff --git a/packages/flutter/test/material/tabs_test.dart b/packages/flutter/test/material/tabs_test.dart index a3429202ca..bec04f8b69 100644 --- a/packages/flutter/test/material/tabs_test.dart +++ b/packages/flutter/test/material/tabs_test.dart @@ -2571,6 +2571,81 @@ void main() { expect(find.text('No tabs'), findsOneWidget); }); + testWidgets('TabBar - updating to and from zero tabs', (WidgetTester tester) async { + // Regression test for https://github.com/flutter/flutter/issues/68962. + final List tabTitles = []; + final List tabContents = []; + TabController _tabController = TabController(length: tabContents.length, vsync: const TestVSync()); + + void _onTabAdd(StateSetter setState) { + setState(() { + tabTitles.add('Tab ${tabTitles.length + 1}'); + tabContents.add( + Container( + color: Colors.red, + height: 200, + width: 200, + ), + ); + _tabController = TabController(length: tabContents.length, vsync: const TestVSync()); + }); + } + + void _onTabRemove(StateSetter setState) { + setState(() { + tabTitles.removeLast(); + tabContents.removeLast(); + _tabController = TabController(length: tabContents.length, vsync: const TestVSync()); + }); + } + + await tester.pumpWidget( + MaterialApp( + home: StatefulBuilder( + builder: (BuildContext context, StateSetter setState) { + return Scaffold( + appBar: AppBar( + actions: [ + FlatButton( + key: const Key('Add tab'), + child: const Text('Add tab'), + onPressed: () => _onTabAdd(setState), + ), + FlatButton( + key: const Key('Remove tab'), + child: const Text('Remove tab'), + onPressed: () => _onTabRemove(setState), + ), + ], + bottom: PreferredSize( + preferredSize: const Size.fromHeight(40.0), + child: Expanded( + child: TabBar( + controller: _tabController, + tabs: tabTitles + .map((String title) => Tab(text: title)) + .toList(), + ), + ), + ), + ), + ); + }, + ), + ), + ); + + expect(find.text('Tab 1'), findsNothing); + expect(find.text('Add tab'), findsOneWidget); + await tester.tap(find.byKey(const Key('Add tab'))); + await tester.pumpAndSettle(); + expect(find.text('Tab 1'), findsOneWidget); + + await tester.tap(find.byKey(const Key('Remove tab'))); + await tester.pumpAndSettle(); + expect(find.text('Tab 1'), findsNothing); + }); + testWidgets('TabBar expands vertically to accommodate the Icon and child Text() pair the same amount it would expand for Icon and text pair.', (WidgetTester tester) async { const double indicatorWeight = 2.0;