Unverified Commit bd7d34f0 authored by chunhtai's avatar chunhtai Committed by GitHub

Reland "Fix Backbutton is not displayed when there is a endDrawer (#1… (#104110)

* Reland "Fix Backbutton is not displayed when there is a endDrawer (#102093)"

This reverts commit a4a8e73b.

* add todos
parent c29a7a2d
......@@ -970,9 +970,11 @@ class _AppBarState extends State<AppBar> {
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) {
......
......@@ -401,7 +401,7 @@ class DrawerControllerState extends State<DrawerController> with SingleTickerPro
if (_historyEntry == null) {
final ModalRoute<dynamic>? 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);
}
......
......@@ -455,13 +455,21 @@ abstract class TransitionRoute<T> extends OverlayRoute<T> {
/// 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<dynamic>? _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<T> on Route<T> {
List<LocalHistoryEntry>? _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<T> on Route<T> {
_localHistory ??= <LocalHistoryEntry>[];
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<T> on Route<T> {
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<T> on Route<T> {
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<dynamic> 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<T> extends State<_ModalScope<T>> {
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<T> extends TransitionRoute<T> with LocalHistoryRoute<T
/// notified.
bool get canPop => 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<T>> _scopeKey = GlobalKey<_ModalScopeState<T>>();
......
......@@ -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<void> page1 = MaterialPage<void>(
key: const ValueKey<String>('1'),
child: Scaffold(
key: const ValueKey<String>('1'),
appBar: AppBar(),
endDrawer: const Drawer(),
)
);
final Page<void> page2 = MaterialPage<void>(
key: const ValueKey<String>('2'),
child: Scaffold(
key: const ValueKey<String>('2'),
appBar: AppBar(),
endDrawer: const Drawer(),
)
);
final List<Page<void>> pages = <Page<void>>[ page1, page2 ];
await tester.pumpWidget(
MaterialApp(
home: Navigator(
pages: pages,
onPopPage: (Route<Object?> route, Object? result) => false,
),
),
);
// The page2 should have a back button.
expect(
find.descendant(
of: find.byKey(const ValueKey<String>('2')),
matching: find.byType(BackButton),
),
findsOneWidget
);
});
testWidgets('AppBar.preferredHeightFor', (WidgetTester tester) async {
late double preferredHeight;
late Size preferredSize;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment