diff --git a/packages/flutter/lib/src/cupertino/tab_scaffold.dart b/packages/flutter/lib/src/cupertino/tab_scaffold.dart index 7673806d5d..345788d461 100644 --- a/packages/flutter/lib/src/cupertino/tab_scaffold.dart +++ b/packages/flutter/lib/src/cupertino/tab_scaffold.dart @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'package:flutter/foundation.dart'; import 'package:flutter/widgets.dart'; import 'bottom_tab_bar.dart'; import 'theme.dart'; @@ -258,12 +259,12 @@ class CupertinoTabScaffold extends StatefulWidget { /// An [IndexedWidgetBuilder] that's called when tabs become active. /// - /// The widgets built by [IndexedWidgetBuilder] is typically a [CupertinoTabView] - /// in order to achieve the parallel hierarchies information architecture seen - /// on iOS apps with tab bars. + /// The widgets built by [IndexedWidgetBuilder] are typically a + /// [CupertinoTabView] in order to achieve the parallel hierarchical + /// information architecture seen on iOS apps with tab bars. /// - /// When the tab becomes inactive, its content is still cached in the widget - /// tree [Offstage] and its animations disabled. + /// When the tab becomes inactive, its content is cached in the widget tree + /// [Offstage] and its animations disabled. /// /// Content can slide under the [tabBar] when they're translucent. /// In that case, the child's [BuildContext]'s [MediaQuery] will have a @@ -351,7 +352,7 @@ class _CupertinoTabScaffoldState extends State { Widget content = _TabSwitchingView( currentTabIndex: _controller.index, - tabNumber: widget.tabBar.items.length, + tabCount: widget.tabBar.items.length, tabBuilder: widget.tabBuilder, ); EdgeInsets contentPadding = EdgeInsets.zero; @@ -444,14 +445,14 @@ class _CupertinoTabScaffoldState extends State { class _TabSwitchingView extends StatefulWidget { const _TabSwitchingView({ @required this.currentTabIndex, - @required this.tabNumber, + @required this.tabCount, @required this.tabBuilder, }) : assert(currentTabIndex != null), - assert(tabNumber != null && tabNumber > 0), + assert(tabCount != null && tabCount > 0), assert(tabBuilder != null); final int currentTabIndex; - final int tabNumber; + final int tabCount; final IndexedWidgetBuilder tabBuilder; @override @@ -459,13 +460,19 @@ class _TabSwitchingView extends StatefulWidget { } class _TabSwitchingViewState extends State<_TabSwitchingView> { - List shouldBuildTab; - List tabFocusNodes; + final List shouldBuildTab = []; + final List tabFocusNodes = []; + + // When focus nodes are no longer needed, we need to dispose of them, but we + // can't be sure that nothing else is listening to them until this widget is + // disposed of, so when they are no longer needed, we move them to this list, + // and dispose of them when we dispose of this widget. + final List discardedNodes = []; @override void initState() { super.initState(); - shouldBuildTab = List.filled(widget.tabNumber, false, growable: true); + shouldBuildTab.addAll(List.filled(widget.tabCount, false)); } @override @@ -483,21 +490,30 @@ class _TabSwitchingViewState extends State<_TabSwitchingView> { // - new tabs are appended to the tab list, or // - some trailing tabs are removed. // If the above assumption is not true, some tabs may lose their state. - final int lengthDiff = widget.tabNumber - shouldBuildTab.length; + final int lengthDiff = widget.tabCount - shouldBuildTab.length; if (lengthDiff > 0) { shouldBuildTab.addAll(List.filled(lengthDiff, false)); } else if (lengthDiff < 0) { - shouldBuildTab.removeRange(widget.tabNumber, shouldBuildTab.length); + shouldBuildTab.removeRange(widget.tabCount, shouldBuildTab.length); } _focusActiveTab(); } + // Will focus the active tab if the FocusScope above it has focus already. If + // not, then it will just mark it as the preferred focus for that scope. void _focusActiveTab() { - if (tabFocusNodes?.length != widget.tabNumber) { - tabFocusNodes = List.generate( - widget.tabNumber, - (int index) => FocusScopeNode(debugLabel: 'Tab Focus Scope $index'), - ); + if (tabFocusNodes.length != widget.tabCount) { + if (tabFocusNodes.length > widget.tabCount) { + discardedNodes.addAll(tabFocusNodes.sublist(widget.tabCount)); + tabFocusNodes.removeRange(widget.tabCount, tabFocusNodes.length); + } else { + tabFocusNodes.addAll( + List.generate( + widget.tabCount - tabFocusNodes.length, + (int index) => FocusScopeNode(debugLabel: '${describeIdentity(widget)} Tab ${index + tabFocusNodes.length}'), + ), + ); + } } FocusScope.of(context).setFirstFocus(tabFocusNodes[widget.currentTabIndex]); } @@ -507,6 +523,9 @@ class _TabSwitchingViewState extends State<_TabSwitchingView> { for (FocusScopeNode focusScopeNode in tabFocusNodes) { focusScopeNode.dispose(); } + for (FocusScopeNode focusScopeNode in discardedNodes) { + focusScopeNode.dispose(); + } super.dispose(); } @@ -514,7 +533,7 @@ class _TabSwitchingViewState extends State<_TabSwitchingView> { Widget build(BuildContext context) { return Stack( fit: StackFit.expand, - children: List.generate(widget.tabNumber, (int index) { + children: List.generate(widget.tabCount, (int index) { final bool active = index == widget.currentTabIndex; shouldBuildTab[index] = active || shouldBuildTab[index]; @@ -524,9 +543,9 @@ class _TabSwitchingViewState extends State<_TabSwitchingView> { enabled: active, child: FocusScope( node: tabFocusNodes[index], - child: shouldBuildTab[index] - ? widget.tabBuilder(context, index) - : Container(), + child: Builder(builder: (BuildContext context) { + return shouldBuildTab[index] ? widget.tabBuilder(context, index) : Container(); + }), ), ), ); diff --git a/packages/flutter/test/cupertino/tab_scaffold_test.dart b/packages/flutter/test/cupertino/tab_scaffold_test.dart index a54696e5ee..5285777203 100644 --- a/packages/flutter/test/cupertino/tab_scaffold_test.dart +++ b/packages/flutter/test/cupertino/tab_scaffold_test.dart @@ -920,6 +920,55 @@ void main() { expect(message, contains('with 3 tabs')); }); + testWidgets("Don't replace focus nodes for existing tabs when changing tab count", (WidgetTester tester) async { + final CupertinoTabController controller = CupertinoTabController(initialIndex: 2); + + final List scopes = List(5); + await tester.pumpWidget( + CupertinoApp( + home: CupertinoTabScaffold( + tabBar: CupertinoTabBar( + items: List.generate(3, tabGenerator), + ), + controller: controller, + tabBuilder: (BuildContext context, int index) { + scopes[index] = FocusScope.of(context); + return Container(); + }, + ), + ) + ); + + for (int i = 0; i < 3; i++) { + controller.index = i; + await tester.pump(); + } + await tester.pump(); + + final List newScopes = List(5); + await tester.pumpWidget( + CupertinoApp( + home: CupertinoTabScaffold( + tabBar: CupertinoTabBar( + items: List.generate(5, tabGenerator), + ), + controller: controller, + tabBuilder: (BuildContext context, int index) { + newScopes[index] = FocusScope.of(context); + return Container(); + }, + ), + ) + ); + for (int i = 0; i < 5; i++) { + controller.index = i; + await tester.pump(); + } + await tester.pump(); + + expect(scopes.sublist(0, 3), equals(newScopes.sublist(0, 3))); + }); + testWidgets('Current tab index cannot go below zero or be null', (WidgetTester tester) async { void expectAssertionError(VoidCallback callback, String errorMessage) { try {