Skip to content

Commit

Permalink
Revert "Fix Backbutton is not displayed when there is a endDrawer (#1…
Browse files Browse the repository at this point in the history
…02093)" (#104039)

This reverts commit f8f4387.
  • Loading branch information
Casey Hillers committed May 18, 2022
1 parent 6eb65b3 commit a4a8e73
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 83 deletions.
6 changes: 4 additions & 2 deletions packages/flutter/lib/src/material/app_bar.dart
Expand Up @@ -862,6 +862,7 @@ class _AppBarState extends State<AppBar> {

final bool hasDrawer = scaffold?.hasDrawer ?? false;
final bool hasEndDrawer = scaffold?.hasEndDrawer ?? false;
final bool canPop = parentRoute?.canPop ?? false;
final bool useCloseButton = parentRoute is PageRoute<dynamic> && parentRoute.fullscreenDialog;

final double toolbarHeight = widget.toolbarHeight ?? appBarTheme.toolbarHeight ?? kToolbarHeight;
Expand Down Expand Up @@ -947,8 +948,9 @@ class _AppBarState extends State<AppBar> {
onPressed: _handleDrawerButton,
tooltip: MaterialLocalizations.of(context).openAppDrawerTooltip,
);
} else if (parentRoute?.impliesAppBarDismissal ?? false) {
leading = useCloseButton ? const CloseButton() : const BackButton();
} else {
if (!hasEndDrawer && canPop)
leading = useCloseButton ? const CloseButton() : const BackButton();
}
}
if (leading != null) {
Expand Down
2 changes: 1 addition & 1 deletion packages/flutter/lib/src/material/drawer.dart
Expand Up @@ -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, impliesAppBarDismissal: false);
_historyEntry = LocalHistoryEntry(onRemove: _handleHistoryEntryRemoved);
route.addLocalHistoryEntry(_historyEntry!);
FocusScope.of(context).setFirstFocus(_focusScopeNode);
}
Expand Down
48 changes: 6 additions & 42 deletions packages/flutter/lib/src/widgets/routes.dart
Expand Up @@ -455,21 +455,13 @@ 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].
///
/// The [impliesAppBarDismissal] defaults to true if not provided.
LocalHistoryEntry({ this.onRemove, this.impliesAppBarDismissal = true });
LocalHistoryEntry({ this.onRemove });

/// 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);
Expand All @@ -490,7 +482,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
Expand Down Expand Up @@ -628,12 +620,7 @@ mixin LocalHistoryRoute<T> on Route<T> {
_localHistory ??= <LocalHistoryEntry>[];
final bool wasEmpty = _localHistory!.isEmpty;
_localHistory!.add(entry);
bool internalStateChanged = false;
if (entry.impliesAppBarDismissal) {
internalStateChanged = _entriesImpliesAppBarDismissal == 0;
_entriesImpliesAppBarDismissal += 1;
}
if (wasEmpty || internalStateChanged)
if (wasEmpty)
changedInternalState();
}

Expand All @@ -645,15 +632,10 @@ mixin LocalHistoryRoute<T> on Route<T> {
assert(entry != null);
assert(entry._owner == this);
assert(_localHistory!.contains(entry));
bool internalStateChanged = false;
if (_localHistory!.remove(entry) && entry.impliesAppBarDismissal) {
_entriesImpliesAppBarDismissal -= 1;
internalStateChanged = _entriesImpliesAppBarDismissal == 0;
}
_localHistory!.remove(entry);
entry._owner = null;
entry._notifyRemoved();
if (_localHistory!.isEmpty || internalStateChanged) {
assert(_entriesImpliesAppBarDismissal == 0);
if (_localHistory!.isEmpty) {
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
Expand Down Expand Up @@ -681,12 +663,7 @@ mixin LocalHistoryRoute<T> on Route<T> {
assert(entry._owner == this);
entry._owner = null;
entry._notifyRemoved();
bool internalStateChanged = false;
if (entry.impliesAppBarDismissal) {
_entriesImpliesAppBarDismissal -= 1;
internalStateChanged = _entriesImpliesAppBarDismissal == 0;
}
if (_localHistory!.isEmpty || internalStateChanged)
if (_localHistory!.isEmpty)
changedInternalState();
return false;
}
Expand Down Expand Up @@ -720,7 +697,6 @@ class _ModalScopeStatus extends InheritedWidget {
const _ModalScopeStatus({
required this.isCurrent,
required this.canPop,
required this.impliesAppBarDismissal,
required this.route,
required super.child,
}) : assert(isCurrent != null),
Expand All @@ -730,14 +706,12 @@ 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;
}

Expand All @@ -746,7 +720,6 @@ 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'));
}
}

Expand Down Expand Up @@ -849,7 +822,6 @@ 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(
Expand Down Expand Up @@ -1589,14 +1561,6 @@ 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>>();
Expand Down
38 changes: 0 additions & 38 deletions packages/flutter/test/material/app_bar_test.dart
Expand Up @@ -3074,44 +3074,6 @@ 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;
Expand Down

0 comments on commit a4a8e73

Please sign in to comment.