diff --git a/packages/flutter/lib/src/material/app_bar.dart b/packages/flutter/lib/src/material/app_bar.dart index 636a118799822..569acfe8830e0 100644 --- a/packages/flutter/lib/src/material/app_bar.dart +++ b/packages/flutter/lib/src/material/app_bar.dart @@ -970,9 +970,11 @@ class _AppBarState extends State { onPressed: _handleDrawerButton, tooltip: MaterialLocalizations.of(context).openAppDrawerTooltip, ); - } else { - if (!hasEndDrawer && canPop) - leading = useCloseButton ? const CloseButton() : const BackButton(); + // TODO(chunhtai): remove (!hasEndDrawer && canPop) once internal tests + // are migrated. + // https://github.com/flutter/flutter/issues/80256. + } else if ((!hasEndDrawer && canPop) || (parentRoute?.impliesAppBarDismissal ?? false)) { + leading = useCloseButton ? const CloseButton() : const BackButton(); } } if (leading != null) { diff --git a/packages/flutter/lib/src/material/drawer.dart b/packages/flutter/lib/src/material/drawer.dart index 6542de82dc7e6..3297793262b90 100644 --- a/packages/flutter/lib/src/material/drawer.dart +++ b/packages/flutter/lib/src/material/drawer.dart @@ -401,7 +401,7 @@ class DrawerControllerState extends State with SingleTickerPro if (_historyEntry == null) { final ModalRoute? route = ModalRoute.of(context); if (route != null) { - _historyEntry = LocalHistoryEntry(onRemove: _handleHistoryEntryRemoved); + _historyEntry = LocalHistoryEntry(onRemove: _handleHistoryEntryRemoved, impliesAppBarDismissal: false); route.addLocalHistoryEntry(_historyEntry!); FocusScope.of(context).setFirstFocus(_focusScopeNode); } diff --git a/packages/flutter/lib/src/widgets/routes.dart b/packages/flutter/lib/src/widgets/routes.dart index 83bcdcee9ad97..069548b7ed0d8 100644 --- a/packages/flutter/lib/src/widgets/routes.dart +++ b/packages/flutter/lib/src/widgets/routes.dart @@ -455,13 +455,21 @@ abstract class TransitionRoute extends OverlayRoute { /// An entry in the history of a [LocalHistoryRoute]. class LocalHistoryEntry { /// Creates an entry in the history of a [LocalHistoryRoute]. - LocalHistoryEntry({ this.onRemove }); + /// + /// The [impliesAppBarDismissal] defaults to true if not provided. + LocalHistoryEntry({ this.onRemove, this.impliesAppBarDismissal = true }); /// Called when this entry is removed from the history of its associated [LocalHistoryRoute]. final VoidCallback? onRemove; LocalHistoryRoute? _owner; + /// Whether an [AppBar] in the route this entry belongs to should + /// automatically add a back button or close button. + /// + /// Defaults to true. + final bool impliesAppBarDismissal; + /// Remove this entry from the history of its associated [LocalHistoryRoute]. void remove() { _owner?.removeLocalHistoryEntry(this); @@ -482,7 +490,7 @@ class LocalHistoryEntry { /// is removed from the list and its [LocalHistoryEntry.onRemove] is called. mixin LocalHistoryRoute on Route { List? _localHistory; - + int _entriesImpliesAppBarDismissal = 0; /// Adds a local history entry to this route. /// /// When asked to pop, if this route has any local history entries, this route @@ -620,7 +628,12 @@ mixin LocalHistoryRoute on Route { _localHistory ??= []; final bool wasEmpty = _localHistory!.isEmpty; _localHistory!.add(entry); - if (wasEmpty) + bool internalStateChanged = false; + if (entry.impliesAppBarDismissal) { + internalStateChanged = _entriesImpliesAppBarDismissal == 0; + _entriesImpliesAppBarDismissal += 1; + } + if (wasEmpty || internalStateChanged) changedInternalState(); } @@ -632,10 +645,15 @@ mixin LocalHistoryRoute on Route { assert(entry != null); assert(entry._owner == this); assert(_localHistory!.contains(entry)); - _localHistory!.remove(entry); + bool internalStateChanged = false; + if (_localHistory!.remove(entry) && entry.impliesAppBarDismissal) { + _entriesImpliesAppBarDismissal -= 1; + internalStateChanged = _entriesImpliesAppBarDismissal == 0; + } entry._owner = null; entry._notifyRemoved(); - if (_localHistory!.isEmpty) { + if (_localHistory!.isEmpty || internalStateChanged) { + assert(_entriesImpliesAppBarDismissal == 0); if (SchedulerBinding.instance.schedulerPhase == SchedulerPhase.persistentCallbacks) { // The local history might be removed as a result of disposing inactive // elements during finalizeTree. The state is locked at this moment, and @@ -663,7 +681,12 @@ mixin LocalHistoryRoute on Route { assert(entry._owner == this); entry._owner = null; entry._notifyRemoved(); - if (_localHistory!.isEmpty) + bool internalStateChanged = false; + if (entry.impliesAppBarDismissal) { + _entriesImpliesAppBarDismissal -= 1; + internalStateChanged = _entriesImpliesAppBarDismissal == 0; + } + if (_localHistory!.isEmpty || internalStateChanged) changedInternalState(); return false; } @@ -697,6 +720,7 @@ class _ModalScopeStatus extends InheritedWidget { const _ModalScopeStatus({ required this.isCurrent, required this.canPop, + required this.impliesAppBarDismissal, required this.route, required super.child, }) : assert(isCurrent != null), @@ -706,12 +730,14 @@ class _ModalScopeStatus extends InheritedWidget { final bool isCurrent; final bool canPop; + final bool impliesAppBarDismissal; final Route route; @override bool updateShouldNotify(_ModalScopeStatus old) { return isCurrent != old.isCurrent || canPop != old.canPop || + impliesAppBarDismissal != old.impliesAppBarDismissal || route != old.route; } @@ -720,6 +746,7 @@ class _ModalScopeStatus extends InheritedWidget { super.debugFillProperties(description); description.add(FlagProperty('isCurrent', value: isCurrent, ifTrue: 'active', ifFalse: 'inactive')); description.add(FlagProperty('canPop', value: canPop, ifTrue: 'can pop')); + description.add(FlagProperty('impliesAppBarDismissal', value: impliesAppBarDismissal, ifTrue: 'implies app bar dismissal')); } } @@ -822,6 +849,7 @@ class _ModalScopeState extends State<_ModalScope> { route: widget.route, isCurrent: widget.route.isCurrent, // _routeSetState is called if this updates canPop: widget.route.canPop, // _routeSetState is called if this updates + impliesAppBarDismissal: widget.route.impliesAppBarDismissal, child: Offstage( offstage: widget.route.offstage, // _routeSetState is called if this updates child: PageStorage( @@ -1561,6 +1589,14 @@ abstract class ModalRoute extends TransitionRoute with LocalHistoryRoute hasActiveRouteBelow || willHandlePopInternally; + /// Whether an [AppBar] in the route should automatically add a back button or + /// close button. + /// + /// This getter returns true if there is at least one active route below it, + /// or there is at least one [LocalHistoryEntry] with `impliesAppBarDismissal` + /// set to true + bool get impliesAppBarDismissal => hasActiveRouteBelow || _entriesImpliesAppBarDismissal > 0; + // Internals final GlobalKey<_ModalScopeState> _scopeKey = GlobalKey<_ModalScopeState>(); diff --git a/packages/flutter/test/material/app_bar_test.dart b/packages/flutter/test/material/app_bar_test.dart index 824a711c415b2..2767cc15c7bab 100644 --- a/packages/flutter/test/material/app_bar_test.dart +++ b/packages/flutter/test/material/app_bar_test.dart @@ -3267,6 +3267,44 @@ void main() { }); }); + // Regression test for https://github.com/flutter/flutter/issues/80256 + testWidgets('The second page should have a back button even it has a end drawer', (WidgetTester tester) async { + final Page page1 = MaterialPage( + key: const ValueKey('1'), + child: Scaffold( + key: const ValueKey('1'), + appBar: AppBar(), + endDrawer: const Drawer(), + ) + ); + final Page page2 = MaterialPage( + key: const ValueKey('2'), + child: Scaffold( + key: const ValueKey('2'), + appBar: AppBar(), + endDrawer: const Drawer(), + ) + ); + final List> pages = >[ page1, page2 ]; + await tester.pumpWidget( + MaterialApp( + home: Navigator( + pages: pages, + onPopPage: (Route route, Object? result) => false, + ), + ), + ); + + // The page2 should have a back button. + expect( + find.descendant( + of: find.byKey(const ValueKey('2')), + matching: find.byType(BackButton), + ), + findsOneWidget + ); + }); + testWidgets('AppBar.preferredHeightFor', (WidgetTester tester) async { late double preferredHeight; late Size preferredSize;