From 9e4ab00e34f8a18e22b48da9efda1f17f6260eee Mon Sep 17 00:00:00 2001 From: xster Date: Thu, 7 Feb 2019 17:25:15 -0800 Subject: [PATCH] Handle CupertinoTabScaffold rebuilds with deleted tabs (#27576) --- .../lib/src/cupertino/bottom_tab_bar.dart | 8 +- .../lib/src/cupertino/tab_scaffold.dart | 10 +++ .../test/cupertino/tab_scaffold_test.dart | 76 +++++++++++++++++++ 3 files changed, 92 insertions(+), 2 deletions(-) diff --git a/packages/flutter/lib/src/cupertino/bottom_tab_bar.dart b/packages/flutter/lib/src/cupertino/bottom_tab_bar.dart index 0416d936d6..34ae20feed 100644 --- a/packages/flutter/lib/src/cupertino/bottom_tab_bar.dart +++ b/packages/flutter/lib/src/cupertino/bottom_tab_bar.dart @@ -54,7 +54,10 @@ class CupertinoTabBar extends StatelessWidget implements PreferredSizeWidget { ), ), }) : assert(items != null), - assert(items.length >= 2), + assert( + items.length >= 2, + "Tabs need at least 2 items to conform to Apple's HIG", + ), assert(currentIndex != null), assert(0 <= currentIndex && currentIndex < items.length), assert(iconSize != null), @@ -75,7 +78,8 @@ class CupertinoTabBar extends StatelessWidget implements PreferredSizeWidget { /// The index into [items] of the current active item. /// - /// Must not be null. + /// Must not be null and must inclusively be between 0 and the number of tabs + /// minus 1. final int currentIndex; /// The background color of the tab bar. If it contains transparency, the diff --git a/packages/flutter/lib/src/cupertino/tab_scaffold.dart b/packages/flutter/lib/src/cupertino/tab_scaffold.dart index fa3fa79574..4e56f5a871 100644 --- a/packages/flutter/lib/src/cupertino/tab_scaffold.dart +++ b/packages/flutter/lib/src/cupertino/tab_scaffold.dart @@ -167,6 +167,16 @@ class _CupertinoTabScaffoldState extends State { @override void didUpdateWidget(CupertinoTabScaffold oldWidget) { super.didUpdateWidget(oldWidget); + if (_currentPage >= widget.tabBar.items.length) { + // Clip down to an acceptable range. + _currentPage = widget.tabBar.items.length - 1; + // Sanity check, since CupertinoTabBar.items's minimum length is 2. + assert( + _currentPage >= 0, + 'CupertinoTabBar is expected to keep at least 2 tabs after updating', + ); + } + // The user can still specify an exact desired index. if (widget.tabBar.currentIndex != oldWidget.tabBar.currentIndex) { _currentPage = widget.tabBar.currentIndex; } diff --git a/packages/flutter/test/cupertino/tab_scaffold_test.dart b/packages/flutter/test/cupertino/tab_scaffold_test.dart index 206e6a8d60..5e57787379 100644 --- a/packages/flutter/test/cupertino/tab_scaffold_test.dart +++ b/packages/flutter/test/cupertino/tab_scaffold_test.dart @@ -395,6 +395,82 @@ void main() { expect(tester.getRect(find.byType(Placeholder)), Rect.fromLTWH(0, 0, 800, 400)); expect(MediaQuery.of(innerContext).padding.bottom, 0); }); + + testWidgets('Deleting tabs after selecting them works', (WidgetTester tester) async { + final List tabsBuilt = []; + + BottomNavigationBarItem tabGenerator(int index) { + return BottomNavigationBarItem( + icon: const ImageIcon(TestImageProvider(24, 24)), + title: Text('Tab ${index + 1}'), + ); + } + + await tester.pumpWidget( + CupertinoApp( + home: CupertinoTabScaffold( + tabBar: CupertinoTabBar( + items: List.generate(4, tabGenerator), + onTap: (int newTab) => selectedTabs.add(newTab), + ), + tabBuilder: (BuildContext context, int index) { + tabsBuilt.add(index); + return Text('Page ${index + 1}'); + }, + ), + ), + ); + + expect(tabsBuilt, [0]); + // selectedTabs list is appended to on onTap callbacks. We didn't tap + // any tabs yet. + expect(selectedTabs, []); + tabsBuilt.clear(); + + await tester.tap(find.text('Tab 4')); + await tester.pump(); + + // Tabs 1 and 4 are built but only one is onstage. + expect(tabsBuilt, [0, 3]); + expect(selectedTabs, [3]); + expect(find.text('Page 1', skipOffstage: false), isOffstage); + expect(find.text('Page 4'), findsOneWidget); + tabsBuilt.clear(); + + // Delete 2 tabs. + await tester.pumpWidget( + CupertinoApp( + home: CupertinoTabScaffold( + tabBar: CupertinoTabBar( + items: List.generate(2, tabGenerator), + onTap: (int newTab) => selectedTabs.add(newTab), + ), + tabBuilder: (BuildContext context, int index) { + tabsBuilt.add(index); + // Change the builder too. + return Text('Different page ${index + 1}'); + }, + ), + ), + ); + + expect(tabsBuilt, [0, 1]); + // We didn't tap on any additional tabs to invoke the onTap callback. We + // just deleted a tab. + expect(selectedTabs, [3]); + // Tab 1 was previously built so it's rebuilt again, albeit offstage. + expect(find.text('Different page 1', skipOffstage: false), isOffstage); + // Since all the tabs after tab 2 are deleted, tab 2 is now the last tab and + // the actively shown tab. + expect(find.text('Different page 2'), findsOneWidget); + // No more tab 4 since it's deleted. + expect(find.text('Different page 4', skipOffstage: false), findsNothing); + // We also changed the builder so no tabs should be built with the old + // builder. + expect(find.text('Page 1', skipOffstage: false), findsNothing); + expect(find.text('Page 2', skipOffstage: false), findsNothing); + expect(find.text('Page 4', skipOffstage: false), findsNothing); + }); } CupertinoTabBar _buildTabBar({ int selectedTab = 0 }) {