AppBar automatically show a close button for dialogs (#9268)

* Wiring fullscreen dialog

* Make fullscreen dialog use the new API. Add tests

* Review notes

* Move field back up

* final

examples/flutter_gallery/lib/demo/material/full_screen_dialog_demo.dart

@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+import 'dart:async';
+
 import 'package:flutter/foundation.dart';
 import 'package:flutter/material.dart';
 import 'package:intl/intl.dart';
@@ -104,16 +106,14 @@ class FullScreenDialogDemoState extends State<FullScreenDialogDemo> {
   bool _allDayValue = false;
   bool _saveNeeded = false;
 
-  void handleDismissButton(BuildContext context) {
-    if (!_saveNeeded) {
-      Navigator.pop(context, null);
-      return;
-    }
+  Future<bool> _onWillPop() async {
+    if (!_saveNeeded)
+      return true;
 
     final ThemeData theme = Theme.of(context);
     final TextStyle dialogTextStyle = theme.textTheme.subhead.copyWith(color: theme.textTheme.caption.color);
 
-    showDialog<DismissDialogAction>(
+    return await showDialog<bool>(
       context: context,
       child: new AlertDialog(
         content: new Text(
@@ -123,19 +123,19 @@ class FullScreenDialogDemoState extends State<FullScreenDialogDemo> {
         actions: <Widget>[
           new FlatButton(
             child: const Text('CANCEL'),
-            onPressed: () { Navigator.pop(context, DismissDialogAction.cancel); }
+            onPressed: () {
+              Navigator.of(context).pop(false); // Pops the confirmation dialog but not the page.
+            }
           ),
           new FlatButton(
             child: const Text('DISCARD'),
             onPressed: () {
-              Navigator.of(context)
-                ..pop(DismissDialogAction.discard) // pop the cancel/discard dialog
-                ..pop(); // pop this route
+              Navigator.of(context).pop(true); // Returning true to _onWillPop will pop again.
             }
           )
         ]
       )
-    );
+    ) ?? false;
   }
 
   @override
@@ -144,10 +144,6 @@ class FullScreenDialogDemoState extends State<FullScreenDialogDemo> {
 
     return new Scaffold(
       appBar: new AppBar(
-        leading: new IconButton(
-          icon: const Icon(Icons.clear),
-          onPressed: () { handleDismissButton(context); }
-        ),
         title: const Text('New event'),
         actions: <Widget> [
           new FlatButton(
@@ -158,84 +154,88 @@ class FullScreenDialogDemoState extends State<FullScreenDialogDemo> {
           )
         ]
       ),
-      body: new ListView(
-        padding: const EdgeInsets.all(16.0),
-        children: <Widget>[
-          new Container(
-            padding: const EdgeInsets.symmetric(vertical: 8.0),
-            decoration: new BoxDecoration(
-              border: new Border(bottom: new BorderSide(color: theme.dividerColor))
+      body: new Form(
+        onWillPop: _onWillPop,
+        child: new ListView(
+          padding: const EdgeInsets.all(16.0),
+          children: <Widget>[
+            new Container(
+              padding: const EdgeInsets.symmetric(vertical: 8.0),
+              decoration: new BoxDecoration(
+                border: new Border(bottom: new BorderSide(color: theme.dividerColor))
+              ),
+              alignment: FractionalOffset.bottomLeft,
+              child: new Text('Event name', style: theme.textTheme.display2)
             ),
-            alignment: FractionalOffset.bottomLeft,
-            child: new Text('Event name', style: theme.textTheme.display2)
-          ),
-          new Container(
-            padding: const EdgeInsets.symmetric(vertical: 8.0),
-            decoration: new BoxDecoration(
-              border: new Border(bottom: new BorderSide(color: theme.dividerColor))
+            new Container(
+              padding: const EdgeInsets.symmetric(vertical: 8.0),
+              decoration: new BoxDecoration(
+                border: new Border(bottom: new BorderSide(color: theme.dividerColor))
+              ),
+              alignment: FractionalOffset.bottomLeft,
+              child: new Text('Location', style: theme.textTheme.title.copyWith(color: Colors.black54))
             ),
-            alignment: FractionalOffset.bottomLeft,
-            child: new Text('Location', style: theme.textTheme.title.copyWith(color: Colors.black54))
-          ),
-          new Column(
-            crossAxisAlignment: CrossAxisAlignment.start,
-            children: <Widget>[
-              new Text('From', style: theme.textTheme.caption),
-              new DateTimeItem(
-                dateTime: _fromDateTime,
-                onChanged: (DateTime value) {
-                  setState(() {
-                    _fromDateTime = value;
-                    _saveNeeded = true;
-                  });
-                }
-              )
-            ]
-          ),
-          new Column(
-            crossAxisAlignment: CrossAxisAlignment.start,
-            children: <Widget>[
-              new Text('To', style: theme.textTheme.caption),
-              new DateTimeItem(
-                dateTime: _toDateTime,
-                onChanged: (DateTime value) {
-                  setState(() {
-                    _toDateTime = value;
-                    _saveNeeded = true;
-                  });
-                }
-              )
-            ]
-          ),
-          new Container(
-            decoration: new BoxDecoration(
-              border: new Border(bottom: new BorderSide(color: theme.dividerColor))
+            new Column(
+              crossAxisAlignment: CrossAxisAlignment.start,
+              children: <Widget>[
+                new Text('From', style: theme.textTheme.caption),
+                new DateTimeItem(
+                  dateTime: _fromDateTime,
+                  onChanged: (DateTime value) {
+                    setState(() {
+                      _fromDateTime = value;
+                      _saveNeeded = true;
+                    });
+                  }
+                )
+              ]
             ),
-            child: new Row(
-              children: <Widget> [
-                new Checkbox(
-                  value: _allDayValue,
-                  onChanged: (bool value) {
+            new Column(
+              crossAxisAlignment: CrossAxisAlignment.start,
+              children: <Widget>[
+                new Text('To', style: theme.textTheme.caption),
+                new DateTimeItem(
+                  dateTime: _toDateTime,
+                  onChanged: (DateTime value) {
                     setState(() {
-                      _allDayValue = value;
+                      _toDateTime = value;
                       _saveNeeded = true;
                     });
                   }
                 ),
                 const Text('All-day')
               ]
+            ),
+            new Container(
+              decoration: new BoxDecoration(
+                border: new Border(bottom: new BorderSide(color: theme.dividerColor))
+              ),
+              child: new Row(
+                children: <Widget> [
+                  new Checkbox(
+                    value: _allDayValue,
+                    onChanged: (bool value) {
+                      setState(() {
+                        _allDayValue = value;
+                        _saveNeeded = true;
+                      });
+                    }
+                  ),
+                  new Text('All-day')
+                ]
+              )
             )
-          )
-        ]
-        .map((Widget child) {
-          return new Container(
-            padding: const EdgeInsets.symmetric(vertical: 8.0),
-            height: 96.0,
-            child: child
-          );
-        })
-        .toList()
-      )
+          ]
+          .map((Widget child) {
+            return new Container(
+              padding: const EdgeInsets.symmetric(vertical: 8.0),
+              height: 96.0,
+              child: child
+            );
+          })
+          .toList()
+        )
+      ),
     );
   }
 }

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

@@ -17,6 +17,7 @@ import 'icon_theme.dart';
 import 'icon_theme_data.dart';
 import 'icons.dart';
 import 'material.dart';
+import 'page.dart';
 import 'scaffold.dart';
 import 'tabs.dart';
 import 'theme.dart';
@@ -332,13 +333,16 @@ class AppBar extends StatefulWidget {
 class _AppBarState extends State<AppBar> {
   bool _hasDrawer = false;
   bool _canPop = false;
+  bool _useCloseButton = false;
 
   @override
   void didChangeDependencies() {
     super.didChangeDependencies();
     final ScaffoldState scaffold = Scaffold.of(context, nullOk: true);
     _hasDrawer = scaffold?.hasDrawer ?? false;
-    _canPop = ModalRoute.of(context)?.canPop ?? false;
+    final ModalRoute<dynamic> parentRoute = ModalRoute.of(context);
+    _canPop = parentRoute?.canPop ?? false;
+    _useCloseButton = parentRoute is MaterialPageRoute<dynamic> && parentRoute.fullscreenDialog;
   }
 
   void _handleDrawerButton() {
@@ -380,7 +384,7 @@ class _AppBarState extends State<AppBar> {
         );
       } else {
         if (_canPop)
-          leading = const BackButton();
+          leading = _useCloseButton ? const CloseButton() : const BackButton();
       }
     }
     if (leading != null) {

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

@@ -29,6 +29,8 @@ import 'theme.dart';
 ///    [AppBar.leading] slot when appropriate.
 ///  * [IconButton], which is a more general widget for creating buttons with
 ///    icons.
+///  * [CloseButton], an alternative which may be more appropriate for leaf
+///    node pages in the navigation tree.
 class BackButton extends StatelessWidget {
   /// Creates an [IconButton] with the appropriate "back" icon for the current
   /// target platform.
@@ -58,3 +60,33 @@ class BackButton extends StatelessWidget {
     );
   }
 }
+
+/// A material design close button.
+///
+/// A [CloseButton] is an [IconButton] with a "close" icon. When pressed, the
+/// close button calls [Navigator.maybePop] to return to the previous route.
+///
+/// Use a [CloseButton] instead of a [BackButton] on fullscreen dialogs or
+/// pages that may solicit additional actions to close.
+///
+/// See also:
+///
+///  * [AppBar], which automatically uses a [CloseButton] in its
+///    [AppBar.leading] slot when appropriate.
+///  * [BackButton], which is more appropriate for middle nodes in the 
+///    navigation tree or where pages can be popped instantaneously with
+///    no user data consequence.
+class CloseButton extends StatelessWidget {
+  const CloseButton({ Key key }) : super(key: key);
+
+  @override
+  Widget build(BuildContext context) {
+    return new IconButton(
+      icon: const Icon(Icons.close),
+      tooltip: 'Close',
+      onPressed: () {
+        Navigator.of(context).maybePop();
+      },
+    );
+  }
+}

packages/flutter/test/material/scaffold_test.dart

@@ -329,6 +329,42 @@ void main() {
     });
   });
 
+  group('close button', () {
+    Future<Null> expectCloseIcon(WidgetTester tester, TargetPlatform platform, IconData expectedIcon) async {
+      await tester.pumpWidget(
+        new MaterialApp(
+          theme: new ThemeData(platform: platform),
+          home: new Scaffold(appBar: new AppBar(), body: new Text('Page 1')),
+        )
+      );
+
+      tester.state<NavigatorState>(find.byType(Navigator)).push(new MaterialPageRoute<Null>(
+        builder: (BuildContext context) {
+          return new Scaffold(appBar: new AppBar(), body: new Text('Page 2'));
+        },
+        fullscreenDialog: true,
+      ));
+
+      await tester.pump();
+      await tester.pump(const Duration(seconds: 1));
+
+      final Icon icon = tester.widget(find.byType(Icon));
+      expect(icon.icon, expectedIcon);
+    }
+
+    testWidgets('Close button shows correctly on Android', (WidgetTester tester) async {
+      await expectCloseIcon(tester, TargetPlatform.android, Icons.close);
+    });
+
+    testWidgets('Close button shows correctly on Fuchsia', (WidgetTester tester) async {
+      await expectCloseIcon(tester, TargetPlatform.fuchsia, Icons.close);
+    });
+
+    testWidgets('Close button shows correctly on iOS', (WidgetTester tester) async {
+      await expectCloseIcon(tester, TargetPlatform.iOS, Icons.close);
+    });
+  });
+
   group('body size', () {
     testWidgets('body size with container', (WidgetTester tester) async {
       final Key testKey = new UniqueKey();