Skip to content

Commit bd7d34f

Browse files
authored
Reland "Fix Backbutton is not displayed when there is a endDrawer (#1… (flutter#104110)
* Reland "Fix Backbutton is not displayed when there is a endDrawer (flutter#102093)" This reverts commit a4a8e73. * add todos
1 parent c29a7a2 commit bd7d34f

File tree

4 files changed

+86
-10
lines changed

4 files changed

+86
-10
lines changed

packages/flutter/lib/src/material/app_bar.dart

+5-3
Original file line numberDiff line numberDiff line change
@@ -970,9 +970,11 @@ class _AppBarState extends State<AppBar> {
970970
onPressed: _handleDrawerButton,
971971
tooltip: MaterialLocalizations.of(context).openAppDrawerTooltip,
972972
);
973-
} else {
974-
if (!hasEndDrawer && canPop)
975-
leading = useCloseButton ? const CloseButton() : const BackButton();
973+
// TODO(chunhtai): remove (!hasEndDrawer && canPop) once internal tests
974+
// are migrated.
975+
// https://github.com/flutter/flutter/issues/80256.
976+
} else if ((!hasEndDrawer && canPop) || (parentRoute?.impliesAppBarDismissal ?? false)) {
977+
leading = useCloseButton ? const CloseButton() : const BackButton();
976978
}
977979
}
978980
if (leading != null) {

packages/flutter/lib/src/material/drawer.dart

+1-1
Original file line numberDiff line numberDiff line change
@@ -401,7 +401,7 @@ class DrawerControllerState extends State<DrawerController> with SingleTickerPro
401401
if (_historyEntry == null) {
402402
final ModalRoute<dynamic>? route = ModalRoute.of(context);
403403
if (route != null) {
404-
_historyEntry = LocalHistoryEntry(onRemove: _handleHistoryEntryRemoved);
404+
_historyEntry = LocalHistoryEntry(onRemove: _handleHistoryEntryRemoved, impliesAppBarDismissal: false);
405405
route.addLocalHistoryEntry(_historyEntry!);
406406
FocusScope.of(context).setFirstFocus(_focusScopeNode);
407407
}

packages/flutter/lib/src/widgets/routes.dart

+42-6
Original file line numberDiff line numberDiff line change
@@ -455,13 +455,21 @@ abstract class TransitionRoute<T> extends OverlayRoute<T> {
455455
/// An entry in the history of a [LocalHistoryRoute].
456456
class LocalHistoryEntry {
457457
/// Creates an entry in the history of a [LocalHistoryRoute].
458-
LocalHistoryEntry({ this.onRemove });
458+
///
459+
/// The [impliesAppBarDismissal] defaults to true if not provided.
460+
LocalHistoryEntry({ this.onRemove, this.impliesAppBarDismissal = true });
459461

460462
/// Called when this entry is removed from the history of its associated [LocalHistoryRoute].
461463
final VoidCallback? onRemove;
462464

463465
LocalHistoryRoute<dynamic>? _owner;
464466

467+
/// Whether an [AppBar] in the route this entry belongs to should
468+
/// automatically add a back button or close button.
469+
///
470+
/// Defaults to true.
471+
final bool impliesAppBarDismissal;
472+
465473
/// Remove this entry from the history of its associated [LocalHistoryRoute].
466474
void remove() {
467475
_owner?.removeLocalHistoryEntry(this);
@@ -482,7 +490,7 @@ class LocalHistoryEntry {
482490
/// is removed from the list and its [LocalHistoryEntry.onRemove] is called.
483491
mixin LocalHistoryRoute<T> on Route<T> {
484492
List<LocalHistoryEntry>? _localHistory;
485-
493+
int _entriesImpliesAppBarDismissal = 0;
486494
/// Adds a local history entry to this route.
487495
///
488496
/// When asked to pop, if this route has any local history entries, this route
@@ -620,7 +628,12 @@ mixin LocalHistoryRoute<T> on Route<T> {
620628
_localHistory ??= <LocalHistoryEntry>[];
621629
final bool wasEmpty = _localHistory!.isEmpty;
622630
_localHistory!.add(entry);
623-
if (wasEmpty)
631+
bool internalStateChanged = false;
632+
if (entry.impliesAppBarDismissal) {
633+
internalStateChanged = _entriesImpliesAppBarDismissal == 0;
634+
_entriesImpliesAppBarDismissal += 1;
635+
}
636+
if (wasEmpty || internalStateChanged)
624637
changedInternalState();
625638
}
626639

@@ -632,10 +645,15 @@ mixin LocalHistoryRoute<T> on Route<T> {
632645
assert(entry != null);
633646
assert(entry._owner == this);
634647
assert(_localHistory!.contains(entry));
635-
_localHistory!.remove(entry);
648+
bool internalStateChanged = false;
649+
if (_localHistory!.remove(entry) && entry.impliesAppBarDismissal) {
650+
_entriesImpliesAppBarDismissal -= 1;
651+
internalStateChanged = _entriesImpliesAppBarDismissal == 0;
652+
}
636653
entry._owner = null;
637654
entry._notifyRemoved();
638-
if (_localHistory!.isEmpty) {
655+
if (_localHistory!.isEmpty || internalStateChanged) {
656+
assert(_entriesImpliesAppBarDismissal == 0);
639657
if (SchedulerBinding.instance.schedulerPhase == SchedulerPhase.persistentCallbacks) {
640658
// The local history might be removed as a result of disposing inactive
641659
// elements during finalizeTree. The state is locked at this moment, and
@@ -663,7 +681,12 @@ mixin LocalHistoryRoute<T> on Route<T> {
663681
assert(entry._owner == this);
664682
entry._owner = null;
665683
entry._notifyRemoved();
666-
if (_localHistory!.isEmpty)
684+
bool internalStateChanged = false;
685+
if (entry.impliesAppBarDismissal) {
686+
_entriesImpliesAppBarDismissal -= 1;
687+
internalStateChanged = _entriesImpliesAppBarDismissal == 0;
688+
}
689+
if (_localHistory!.isEmpty || internalStateChanged)
667690
changedInternalState();
668691
return false;
669692
}
@@ -697,6 +720,7 @@ class _ModalScopeStatus extends InheritedWidget {
697720
const _ModalScopeStatus({
698721
required this.isCurrent,
699722
required this.canPop,
723+
required this.impliesAppBarDismissal,
700724
required this.route,
701725
required super.child,
702726
}) : assert(isCurrent != null),
@@ -706,12 +730,14 @@ class _ModalScopeStatus extends InheritedWidget {
706730

707731
final bool isCurrent;
708732
final bool canPop;
733+
final bool impliesAppBarDismissal;
709734
final Route<dynamic> route;
710735

711736
@override
712737
bool updateShouldNotify(_ModalScopeStatus old) {
713738
return isCurrent != old.isCurrent ||
714739
canPop != old.canPop ||
740+
impliesAppBarDismissal != old.impliesAppBarDismissal ||
715741
route != old.route;
716742
}
717743

@@ -720,6 +746,7 @@ class _ModalScopeStatus extends InheritedWidget {
720746
super.debugFillProperties(description);
721747
description.add(FlagProperty('isCurrent', value: isCurrent, ifTrue: 'active', ifFalse: 'inactive'));
722748
description.add(FlagProperty('canPop', value: canPop, ifTrue: 'can pop'));
749+
description.add(FlagProperty('impliesAppBarDismissal', value: impliesAppBarDismissal, ifTrue: 'implies app bar dismissal'));
723750
}
724751
}
725752

@@ -822,6 +849,7 @@ class _ModalScopeState<T> extends State<_ModalScope<T>> {
822849
route: widget.route,
823850
isCurrent: widget.route.isCurrent, // _routeSetState is called if this updates
824851
canPop: widget.route.canPop, // _routeSetState is called if this updates
852+
impliesAppBarDismissal: widget.route.impliesAppBarDismissal,
825853
child: Offstage(
826854
offstage: widget.route.offstage, // _routeSetState is called if this updates
827855
child: PageStorage(
@@ -1561,6 +1589,14 @@ abstract class ModalRoute<T> extends TransitionRoute<T> with LocalHistoryRoute<T
15611589
/// notified.
15621590
bool get canPop => hasActiveRouteBelow || willHandlePopInternally;
15631591

1592+
/// Whether an [AppBar] in the route should automatically add a back button or
1593+
/// close button.
1594+
///
1595+
/// This getter returns true if there is at least one active route below it,
1596+
/// or there is at least one [LocalHistoryEntry] with `impliesAppBarDismissal`
1597+
/// set to true
1598+
bool get impliesAppBarDismissal => hasActiveRouteBelow || _entriesImpliesAppBarDismissal > 0;
1599+
15641600
// Internals
15651601

15661602
final GlobalKey<_ModalScopeState<T>> _scopeKey = GlobalKey<_ModalScopeState<T>>();

packages/flutter/test/material/app_bar_test.dart

+38
Original file line numberDiff line numberDiff line change
@@ -3267,6 +3267,44 @@ void main() {
32673267
});
32683268
});
32693269

3270+
// Regression test for https://github.com/flutter/flutter/issues/80256
3271+
testWidgets('The second page should have a back button even it has a end drawer', (WidgetTester tester) async {
3272+
final Page<void> page1 = MaterialPage<void>(
3273+
key: const ValueKey<String>('1'),
3274+
child: Scaffold(
3275+
key: const ValueKey<String>('1'),
3276+
appBar: AppBar(),
3277+
endDrawer: const Drawer(),
3278+
)
3279+
);
3280+
final Page<void> page2 = MaterialPage<void>(
3281+
key: const ValueKey<String>('2'),
3282+
child: Scaffold(
3283+
key: const ValueKey<String>('2'),
3284+
appBar: AppBar(),
3285+
endDrawer: const Drawer(),
3286+
)
3287+
);
3288+
final List<Page<void>> pages = <Page<void>>[ page1, page2 ];
3289+
await tester.pumpWidget(
3290+
MaterialApp(
3291+
home: Navigator(
3292+
pages: pages,
3293+
onPopPage: (Route<Object?> route, Object? result) => false,
3294+
),
3295+
),
3296+
);
3297+
3298+
// The page2 should have a back button.
3299+
expect(
3300+
find.descendant(
3301+
of: find.byKey(const ValueKey<String>('2')),
3302+
matching: find.byType(BackButton),
3303+
),
3304+
findsOneWidget
3305+
);
3306+
});
3307+
32703308
testWidgets('AppBar.preferredHeightFor', (WidgetTester tester) async {
32713309
late double preferredHeight;
32723310
late Size preferredSize;

0 commit comments

Comments
 (0)