AppBar shows CloseButton even on custom PageRoute (flutter#15643)

* AppBar accepts custom PageRoute * remove unused import * update AUTHORS
wangfangc · Mar 29, 2018 · 464109c · 464109c
1 parent f69d125
commit 464109c
Show file tree

Hide file tree

Showing 3 changed files with 72 additions and 12 deletions.
diff --git a/AUTHORS b/AUTHORS
@@ -20,3 +20,4 @@ Stefano Rodriguez <[email protected]>
 Yusuke Konishi <[email protected]>
 Fredrik Simón <[email protected]>
 Ali Bitek <[email protected]>
+Tetsuhiro Ueda <[email protected]>
diff --git a/packages/flutter/lib/src/material/app_bar.dart b/packages/flutter/lib/src/material/app_bar.dart
@@ -16,7 +16,6 @@ import 'icon_button.dart';
 import 'icons.dart';
 import 'material.dart';
 import 'material_localizations.dart';
-import 'page.dart';
 import 'scaffold.dart';
 import 'tabs.dart';
 import 'theme.dart';
@@ -339,7 +338,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 MaterialPageRoute<dynamic> && parentRoute.fullscreenDialog;
+    final bool useCloseButton = parentRoute is PageRoute<dynamic> && parentRoute.fullscreenDialog;
 
     IconThemeData appBarIconTheme = widget.iconTheme ?? themeData.primaryIconTheme;
     TextStyle centerStyle = widget.textTheme?.title ?? themeData.primaryTextTheme.title;

diff --git a/packages/flutter/test/material/scaffold_test.dart b/packages/flutter/test/material/scaffold_test.dart
@@ -411,38 +411,64 @@ void main() {
   });
 
   group('close button', () {
-    Future<Null> expectCloseIcon(WidgetTester tester, TargetPlatform platform, IconData expectedIcon) async {
+    Future<Null> expectCloseIcon(WidgetTester tester, TargetPlatform platform, IconData expectedIcon, PageRoute<void> routeBuilder()) async {
       await tester.pumpWidget(
         new MaterialApp(
           theme: new ThemeData(platform: platform),
           home: new Scaffold(appBar: new AppBar(), body: const Text('Page 1')),
         )
       );
 
-      tester.state<NavigatorState>(find.byType(Navigator)).push(new MaterialPageRoute<void>(
-        builder: (BuildContext context) {
-          return new Scaffold(appBar: new AppBar(), body: const Text('Page 2'));
-        },
-        fullscreenDialog: true,
-      ));
+      tester.state<NavigatorState>(find.byType(Navigator)).push(routeBuilder());
 
       await tester.pump();
       await tester.pump(const Duration(seconds: 1));
 
       final Icon icon = tester.widget(find.byType(Icon));
       expect(icon.icon, expectedIcon);
+      expect(find.byType(CloseButton), findsOneWidget);
+    }
+
+    PageRoute<void> materialRouteBuilder() {
+      return new MaterialPageRoute<void>(
+        builder: (BuildContext context) {
+          return new Scaffold(appBar: new AppBar(), body: const Text('Page 2'));
+        },
+        fullscreenDialog: true,
+      );
+    }
+
+    PageRoute<void> customPageRouteBuilder() {
+      return new _CustomPageRoute<void>(
+        builder: (BuildContext context) {
+          return new Scaffold(appBar: new AppBar(), body: const Text('Page 2'));
+        },
+        fullscreenDialog: true,
+      );
     }
 
     testWidgets('Close button shows correctly on Android', (WidgetTester tester) async {
-      await expectCloseIcon(tester, TargetPlatform.android, Icons.close);
+      await expectCloseIcon(tester, TargetPlatform.android, Icons.close, materialRouteBuilder);
     });
 
     testWidgets('Close button shows correctly on Fuchsia', (WidgetTester tester) async {
-      await expectCloseIcon(tester, TargetPlatform.fuchsia, Icons.close);
+      await expectCloseIcon(tester, TargetPlatform.fuchsia, Icons.close, materialRouteBuilder);
     });
 
     testWidgets('Close button shows correctly on iOS', (WidgetTester tester) async {
-      await expectCloseIcon(tester, TargetPlatform.iOS, Icons.close);
+      await expectCloseIcon(tester, TargetPlatform.iOS, Icons.close, materialRouteBuilder);
+    });
+
+    testWidgets('Close button shows correctly with custom page route on Android', (WidgetTester tester) async {
+      await expectCloseIcon(tester, TargetPlatform.android, Icons.close, customPageRouteBuilder);
+    });
+
+    testWidgets('Close button shows correctly with custom page route on Fuchsia', (WidgetTester tester) async {
+      await expectCloseIcon(tester, TargetPlatform.fuchsia, Icons.close, customPageRouteBuilder);
+    });
+
+    testWidgets('Close button shows correctly with custom page route on iOS', (WidgetTester tester) async {
+      await expectCloseIcon(tester, TargetPlatform.iOS, Icons.close, customPageRouteBuilder);
     });
   });
 
@@ -1119,3 +1145,37 @@ class _ComputeNotchSetterState extends State<_ComputeNotchSetter> {
     return new Container();
   }
 }
+
+class _CustomPageRoute<T> extends PageRoute<T> {
+  _CustomPageRoute({
+    @required this.builder,
+    RouteSettings settings: const RouteSettings(),
+    this.maintainState: true,
+    bool fullscreenDialog: false,
+  }) : assert(builder != null),
+       super(settings: settings, fullscreenDialog: fullscreenDialog);
+
+  final WidgetBuilder builder;
+
+  @override
+  Duration get transitionDuration => const Duration(milliseconds: 300);
+
+  @override
+  Color get barrierColor => null;
+
+  @override
+  String get barrierLabel => null;
+
+  @override
+  final bool maintainState;
+
+  @override
+  Widget buildPage(BuildContext context, Animation<double> animation, Animation<double> secondaryAnimation) {
+    return builder(context);
+  }
+
+  @override
+  Widget buildTransitions(BuildContext context, Animation<double> animation, Animation<double> secondaryAnimation, Widget child) {
+    return child;
+  }
+}