Conform appbar buttons to Material (#8263)

Extend app bar left to edge, right to 4dp
Make leading button square and 56dp
Keep title at 72dp on Android according to Material
Renamed IconButton.size to .iconSize
IconButton minimum size expands to 48dp (#8264)
IconButton default constraints to 48. Can still stretch to infinity but can't be smaller than 48.
Ink splash for IconButton 40% bigger than the touch target to match Material
Tests

examples/flutter_gallery/lib/demo/icons_demo.dart

@@ -45,10 +45,10 @@ class IconsDemoState extends State<IconsDemo> {
     });
   }
 
-  Widget buildIconButton(double size, IconData icon, bool enabled) {
+  Widget buildIconButton(double iconSize, IconData icon, bool enabled) {
     return new IconButton(
       icon: new Icon(icon),
-      size: size,
+      iconSize: iconSize,
       color: iconColor,
       tooltip: "${enabled ? 'Enabled' : 'Disabled'} icon button",
       onPressed: enabled ? handleIconButtonPress : null

examples/flutter_gallery/lib/demo/tooltip_demo.dart

@@ -41,7 +41,7 @@ class TooltipDemo extends StatelessWidget {
               ),
               new Center(
                 child: new IconButton(
-                  size: 48.0,
+                  iconSize: 48.0,
                   icon: new Icon(Icons.call),
                   color: theme.iconTheme.color,
                   tooltip: 'Place a phone call',

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

@@ -46,8 +46,8 @@ class _ToolbarLayout extends MultiChildLayoutDelegate {
   // space bewteen the leading and actions widgets).
   final bool centerTitle;
 
-  static const double kLeadingWidth = 40.0; // Same size as an IconButton
-  static const double kTitleLeft = 64.0; // The AppBar pads left and right an additional 8.0.
+  static const double kLeadingWidth = 56.0; // So it's square with kToolbarHeight.
+  static const double kTitleLeft = 72.0; // As per https://material.io/guidelines/layout/metrics-keylines.html#metrics-keylines-keylines-spacing.
 
   @override
   void performLayout(Size size) {
@@ -353,7 +353,6 @@ class _AppBarState extends State<AppBar> {
       if (_hasDrawer) {
         leading = new IconButton(
           icon: new Icon(Icons.menu),
-          alignment: FractionalOffset.centerLeft,
           onPressed: _handleDrawerButton,
           tooltip: 'Open navigation menu' // TODO(ianh): Figure out how to localize this string
         );
@@ -372,7 +371,6 @@ class _AppBarState extends State<AppBar> {
           assert(backIcon != null);
           leading = new IconButton(
             icon: new Icon(backIcon),
-            alignment: FractionalOffset.centerLeft,
             onPressed: _handleBackButton,
             tooltip: 'Back' // TODO(ianh): Figure out how to localize this string
           );
@@ -407,6 +405,7 @@ class _AppBarState extends State<AppBar> {
           id: _ToolbarSlot.actions,
           child: new Row(
             mainAxisSize: MainAxisSize.min,
+            crossAxisAlignment: CrossAxisAlignment.stretch,
             children: config.actions,
           ),
         ),
@@ -414,7 +413,7 @@ class _AppBarState extends State<AppBar> {
     }
 
     Widget toolbar = new Padding(
-      padding: const EdgeInsets.symmetric(horizontal: 8.0),
+      padding: const EdgeInsets.only(right: 4.0),
       child: new CustomMultiChildLayout(
         delegate: new _ToolbarLayout(
           centerTitle: config._getEffectiveCenterTitle(themeData),

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

@@ -17,6 +17,9 @@ import 'material.dart';
 import 'theme.dart';
 import 'tooltip.dart';
 
+// Minimum logical pixel size of the IconButton.
+const double kMinButtonSize = 48.0;
+
 /// A material design icon button.
 ///
 /// An icon button is a picture printed on a [Material] widget that reacts to
@@ -30,6 +33,8 @@ import 'tooltip.dart';
 ///
 /// Requires one of its ancestors to be a [Material] widget.
 ///
+/// Will be automatically sized up to the recommended 48 logical pixels if smaller.
+///
 /// See also:
 ///
 ///  * [Icons]
@@ -42,14 +47,14 @@ class IconButton extends StatelessWidget {
   ///
   /// Requires one of its ancestors to be a [Material] widget.
   ///
-  /// The [size], [padding], and [alignment] arguments must not be null (though
+  /// The [iconSize], [padding], and [alignment] arguments must not be null (though
   /// they each have default values).
   ///
   /// The [icon] argument must be specified, and is typically either an [Icon]
   /// or an [ImageIcon].
   const IconButton({
     Key key,
-    this.size: 24.0,
+    this.iconSize: 24.0,
     this.padding: const EdgeInsets.all(8.0),
     this.alignment: FractionalOffset.center,
     @required this.icon,
@@ -69,7 +74,7 @@ class IconButton extends StatelessWidget {
   /// fit the [Icon]. If you were to set the size of the [Icon] using
   /// [Icon.size] instead, then the [IconButton] would default to 24.0 and then
   /// the [Icon] itself would likely get clipped.
-  final double size;
+  final double iconSize;
 
   /// The padding around the button's icon. The entire padded icon will react
   /// to input gestures.
@@ -136,29 +141,29 @@ class IconButton extends StatelessWidget {
       currentColor = color;
     else
       currentColor = disabledColor ?? Theme.of(context).disabledColor;
-    Widget result = new Padding(
-      padding: padding,
-      child: new LimitedBox(
-        maxWidth: size,
-        maxHeight: size,
-        child: new ConstrainedBox(
-          constraints: new BoxConstraints.loose(
-            new Size.square(math.max(size, Material.defaultSplashRadius * 2.0))
-          ),
+
+    Widget result = new ConstrainedBox(
+      constraints: new BoxConstraints(minWidth: kMinButtonSize, minHeight: kMinButtonSize),
+      child: new Padding(
+        padding: padding,
+        child: new SizedBox(
+          height: iconSize,
+          width: iconSize,
           child: new Align(
             alignment: alignment,
             child: new IconTheme.merge(
               context: context,
               data: new IconThemeData(
-                size: size,
+                size: iconSize,
                 color: currentColor
               ),
               child: icon
-            )
-          )
-        )
-      )
+            ),
+          ),
+        ),
+      ),
     );
+
     if (tooltip != null) {
       result = new Tooltip(
         message: tooltip,
@@ -168,7 +173,11 @@ class IconButton extends StatelessWidget {
     return new InkResponse(
       onTap: onPressed,
       child: result,
-      radius: math.max(size, Material.defaultSplashRadius),
+      radius: math.max(
+        Material.defaultSplashRadius,
+        (iconSize + math.min(padding.horizontal, padding.vertical)) * 0.7,
+        // x 0.5 for diameter -> radius and + 40% overflow derived from other Material apps.
+      ),
     );
   }
 

packages/flutter/test/material/app_bar_test.dart

@@ -108,8 +108,8 @@ void main() {
 
     Finder title = find.byKey(titleKey);
     expect(tester.getTopLeft(title).x, 72.0);
-    // The toolbar's contents are padded on the right by 8.0
-    expect(tester.getSize(title).width, equals(800.0 - 72.0 - 8.0));
+    // The toolbar's contents are padded on the right by 4.0
+    expect(tester.getSize(title).width, equals(800.0 - 72.0 - 4.0));
 
     actions = <Widget>[
       const SizedBox(width: 100.0),
@@ -119,13 +119,13 @@ void main() {
 
     expect(tester.getTopLeft(title).x, 72.0);
     // The title shrinks by 200.0 to allow for the actions widgets.
-    expect(tester.getSize(title).width, equals(800.0 - 72.0 - 8.0 - 200.0));
+    expect(tester.getSize(title).width, equals(800.0 - 72.0 - 4.0 - 200.0));
 
     leading = new Container(); // AppBar will constrain the width to 24.0
     await tester.pumpWidget(buildApp());
     expect(tester.getTopLeft(title).x, 72.0);
     // Adding a leading widget shouldn't effect the title's size
-    expect(tester.getSize(title).width, equals(800.0 - 72.0 - 8.0 - 200.0));
+    expect(tester.getSize(title).width, equals(800.0 - 72.0 - 4.0 - 200.0));
   });
 
   testWidgets('AppBar centerTitle:true title overflow OK ', (WidgetTester tester) async {
@@ -165,18 +165,18 @@ void main() {
 
     // Centering a title with width 620 within the 800 pixel wide test widget
     // would mean that its left edge would have to be 90. We reserve 72
-    // on the left and the padded actions occupy 90 + 8 on the right. That
-    // leaves 630, so the title is right justified but its width isn't changed.
+    // on the left and the padded actions occupy 96 + 4 on the right. That
+    // leaves 628, so the title is right justified but its width isn't changed.
 
     await tester.pumpWidget(buildApp());
     leading = null;
     titleWidth = 620.0;
     actions = <Widget>[
-      const SizedBox(width: 45.0),
-      const SizedBox(width: 45.0)
+      const SizedBox(width: 48.0),
+      const SizedBox(width: 48.0)
     ];
     await tester.pumpWidget(buildApp());
-    expect(tester.getTopLeft(title).x, 800 - 620 - 45 - 45 - 8);
+    expect(tester.getTopLeft(title).x, 800 - 620 - 48 - 48 - 4);
     expect(tester.getSize(title).width, equals(620.0));
   });
 
@@ -231,4 +231,59 @@ void main() {
     expect(yCenter(appBarKey), equals(yCenter(action1Key)));
   });
 
+  testWidgets('leading button extends to edge and is square', (WidgetTester tester) async {
+    await tester.pumpWidget(
+      new MaterialApp(
+        theme: new ThemeData(platform: TargetPlatform.android),
+        home: new Scaffold(
+          appBar: new AppBar(
+            title: new Text('X'),
+          ),
+          drawer: new Column(), // Doesn't really matter. Triggers a hamburger regardless.
+        )
+      )
+    );
+
+    Finder hamburger = find.byTooltip('Open navigation menu');
+    expect(tester.getTopLeft(hamburger), new Point(0.0, 0.0));
+    expect(tester.getSize(hamburger), new Size(56.0, 56.0));
+  });
+
+  testWidgets('test action is 4dp from edge and 48dp min', (WidgetTester tester) async {
+    await tester.pumpWidget(
+      new MaterialApp(
+        theme: new ThemeData(platform: TargetPlatform.android),
+        home: new Scaffold(
+          appBar: new AppBar(
+            title: new Text('X'),
+            actions: <Widget> [
+              new IconButton(
+                icon: new Icon(Icons.share),
+                onPressed: null,
+                tooltip: 'Share',
+                iconSize: 20.0,
+              ),
+              new IconButton(
+                icon: new Icon(Icons.add),
+                onPressed: null,
+                tooltip: 'Add',
+                iconSize: 60.0,
+              ),
+            ],
+          ),
+        )
+      )
+    );
+
+    Finder addButton = find.byTooltip('Add');
+    // Right padding is 4dp.
+    expect(tester.getTopRight(addButton), new Point(800.0 - 4.0, 0.0));
+    // It's still the size it was plus the 2 * 8dp padding from IconButton.
+    expect(tester.getSize(addButton), new Size(60.0 + 2 * 8.0, 56.0));
+
+    Finder shareButton = find.byTooltip('Share');
+    // The 20dp icon is expanded to fill the IconButton's touch target to 48dp.
+    expect(tester.getSize(shareButton), new Size(48.0, 56.0));
+  });
+
 }

packages/flutter/test/material/icon_button_test.dart

@@ -5,26 +5,169 @@
 import 'package:flutter/material.dart';
 import 'package:flutter_test/flutter_test.dart';
 
+class MockOnPressedFunction implements Function {
+  int called = 0;
+
+  void call() {
+    called++;
+  }
+}
+
 void main() {
-  testWidgets('IconButton test constrained size', (WidgetTester tester) async {
-    const double kIconSize = 80.0;
+  MockOnPressedFunction mockOnPressedFunction;
+
+  setUp(() {
+    mockOnPressedFunction = new MockOnPressedFunction();
+  });
+
+  testWidgets('test default icon buttons are sized up to 48', (WidgetTester tester) async {
+    await tester.pumpWidget(
+      new Material(
+        child: new Center(
+          child: new IconButton(
+            onPressed: mockOnPressedFunction,
+            icon: new Icon(Icons.link),
+          ),
+        ),
+      ),
+    );
+
+    RenderBox iconButton = tester.renderObject(find.byType(IconButton));
+    expect(iconButton.size, new Size(48.0, 48.0));
 
+    await tester.tap(find.byType(IconButton));
+    expect(mockOnPressedFunction.called, 1);
+  });
+
+  testWidgets('test small icons are sized up to 48dp', (WidgetTester tester) async {
+    await tester.pumpWidget(
+      new Material(
+        child: new Center(
+          child: new IconButton(
+            iconSize: 10.0,
+            onPressed: mockOnPressedFunction,
+            icon: new Icon(Icons.link),
+          ),
+        ),
+      ),
+    );
+
+    RenderBox iconButton = tester.renderObject(find.byType(IconButton));
+    expect(iconButton.size, new Size(48.0, 48.0));
+  });
+
+  testWidgets('test icons can be small when total size is >48dp', (WidgetTester tester) async {
+    await tester.pumpWidget(
+      new Material(
+        child: new Center(
+          child: new IconButton(
+            iconSize: 10.0,
+            padding: new EdgeInsets.all(30.0),
+            onPressed: mockOnPressedFunction,
+            icon: new Icon(Icons.link),
+          ),
+        ),
+      ),
+    );
+
+    RenderBox iconButton = tester.renderObject(find.byType(IconButton));
+    expect(iconButton.size, new Size(70.0, 70.0));
+  });
+
+  testWidgets('test default icon buttons are constrained', (WidgetTester tester) async {
     await tester.pumpWidget(
       new Material(
         child: new Center(
           child: new IconButton(
             padding: EdgeInsets.zero,
-            onPressed: () {},
+            onPressed: mockOnPressedFunction,
+            icon: new Icon(Icons.ac_unit),
+            iconSize: 80.0,
+          ),
+        ),
+      ),
+    );
+
+    RenderBox box = tester.renderObject(find.byType(IconButton));
+    expect(box.size, new Size(80.0, 80.0));
+  });
+
+  testWidgets(
+    'test default icon buttons can be stretched if specified',
+    (WidgetTester tester) async {
+    await tester.pumpWidget(
+      new Material(
+        child: new Row(
+          crossAxisAlignment: CrossAxisAlignment.stretch,
+          children: <Widget> [
+            new IconButton(
+              onPressed: mockOnPressedFunction,
+              icon: new Icon(Icons.ac_unit),
+            ),
+          ],
+        ),
+      ),
+    );
+
+    RenderBox box = tester.renderObject(find.byType(IconButton));
+    expect(box.size, new Size(48.0, 600.0));
+  });
+
+  testWidgets('test default padding', (WidgetTester tester) async {
+    await tester.pumpWidget(
+      new Material(
+        child: new Center(
+          child: new IconButton(
+            onPressed: mockOnPressedFunction,
             icon: new Icon(Icons.ac_unit),
-            size: kIconSize,
-          )
-        )
-      )
+            iconSize: 80.0,
+          ),
+        ),
+      ),
     );
 
     RenderBox box = tester.renderObject(find.byType(IconButton));
-    expect(box.size.width, equals(kIconSize));
-    expect(box.size.height, equals(kIconSize));
+    expect(box.size, new Size(96.0, 96.0));
+  });
+
+  testWidgets('test tooltip', (WidgetTester tester) async {
+    await tester.pumpWidget(
+      new MaterialApp(
+        home: new Material(
+          child: new Center(
+            child: new IconButton(
+              onPressed: mockOnPressedFunction,
+              icon: new Icon(Icons.ac_unit),
+            ),
+          ),
+        ),
+      ),
+    );
+
+    expect(find.byType(Tooltip), findsNothing);
+
+    // Clear the widget tree.
+    await tester.pumpWidget(new Container(key: new UniqueKey()));
+
+    await tester.pumpWidget(
+      new MaterialApp(
+        home: new Material(
+          child: new Center(
+            child: new IconButton(
+              onPressed: mockOnPressedFunction,
+              icon: new Icon(Icons.ac_unit),
+              tooltip: 'Test tooltip',
+            ),
+          ),
+        ),
+      ),
+    );
+
+    expect(find.byType(Tooltip), findsOneWidget);
+    expect(find.byTooltip('Test tooltip'), findsOneWidget);
+
+    await tester.tap(find.byTooltip('Test tooltip'));
+    expect(mockOnPressedFunction.called, 1);
   });
 
   testWidgets('IconButton AppBar size', (WidgetTester tester) async {
@@ -34,12 +177,12 @@ void main() {
           actions: <Widget>[
             new IconButton(
               padding: EdgeInsets.zero,
-              onPressed: () {},
+              onPressed: mockOnPressedFunction,
               icon: new Icon(Icons.ac_unit),
-            )
-          ]
-        )
-      )
+            ),
+          ],
+        ),
+      ),
     );
 
     RenderBox barBox = tester.renderObject(find.byType(AppBar));

packages/flutter/test/material/paginated_data_table_test.dart

@@ -42,6 +42,69 @@ class TestDataSource extends DataTableSource {
 }
 
 void main() {
+  testWidgets('PaginatedDataTable paging', (WidgetTester tester) async {
+    TestDataSource source = new TestDataSource();
+
+    List<String> log = <String>[];
+
+    await tester.pumpWidget(new MaterialApp(
+      home: new PaginatedDataTable(
+        header: new Text('Test table'),
+        source: source,
+        rowsPerPage: 2,
+        availableRowsPerPage: <int>[
+          2, 4, 8, 16,
+        ],
+        onRowsPerPageChanged: (int rowsPerPage) {
+          log.add('rows-per-page-changed: $rowsPerPage');
+        },
+        onPageChanged: (int rowIndex) {
+          log.add('page-changed: $rowIndex');
+        },
+        columns: <DataColumn>[
+          new DataColumn(label: new Text('Name')),
+          new DataColumn(label: new Text('Calories'), numeric: true),
+          new DataColumn(label: new Text('Generation')),
+        ],
+      )
+    ));
+
+    await tester.tap(find.byTooltip('Next page'));
+
+    expect(log, <String>['page-changed: 2']);
+    log.clear();
+
+    await tester.pump();
+
+    expect(find.text('Frozen yogurt (0)'), findsNothing);
+    expect(find.text('Eclair (0)'), findsOneWidget);
+    expect(find.text('Gingerbread (0)'), findsNothing);
+
+    await tester.tap(find.icon(Icons.chevron_left));
+
+    expect(log, <String>['page-changed: 0']);
+    log.clear();
+
+    await tester.pump();
+
+    expect(find.text('Frozen yogurt (0)'), findsOneWidget);
+    expect(find.text('Eclair (0)'), findsNothing);
+    expect(find.text('Gingerbread (0)'), findsNothing);
+
+    await tester.tap(find.icon(Icons.chevron_left));
+
+    expect(log, isEmpty);
+
+    await tester.tap(find.text('2'));
+    await tester.pumpUntilNoTransientCallbacks(const Duration(milliseconds: 200));
+
+    await tester.tap(find.text('8').last);
+    await tester.pumpUntilNoTransientCallbacks(const Duration(milliseconds: 200));
+
+    expect(log, <String>['rows-per-page-changed: 8']);
+    log.clear();
+  });
+
   testWidgets('PaginatedDataTable control test', (WidgetTester tester) async {
     TestDataSource source = new TestDataSource()
       ..generation = 42;
@@ -126,67 +189,4 @@ void main() {
     expect(log, <String>['action: adjust']);
     log.clear();
   });
-
-  testWidgets('PaginatedDataTable paging', (WidgetTester tester) async {
-    TestDataSource source = new TestDataSource();
-
-    List<String> log = <String>[];
-
-    await tester.pumpWidget(new MaterialApp(
-      home: new PaginatedDataTable(
-        header: new Text('Test table'),
-        source: source,
-        rowsPerPage: 2,
-        availableRowsPerPage: <int>[
-          2, 4, 8, 16,
-        ],
-        onRowsPerPageChanged: (int rowsPerPage) {
-          log.add('rows-per-page-changed: $rowsPerPage');
-        },
-        onPageChanged: (int rowIndex) {
-          log.add('page-changed: $rowIndex');
-        },
-        columns: <DataColumn>[
-          new DataColumn(label: new Text('Name')),
-          new DataColumn(label: new Text('Calories'), numeric: true),
-          new DataColumn(label: new Text('Generation')),
-        ],
-      )
-    ));
-
-    await tester.tap(find.byTooltip('Next page'));
-
-    expect(log, <String>['page-changed: 2']);
-    log.clear();
-
-    await tester.pump();
-
-    expect(find.text('Frozen yogurt (0)'), findsNothing);
-    expect(find.text('Eclair (0)'), findsOneWidget);
-    expect(find.text('Gingerbread (0)'), findsNothing);
-
-    await tester.tap(find.icon(Icons.chevron_left));
-
-    expect(log, <String>['page-changed: 0']);
-    log.clear();
-
-    await tester.pump();
-
-    expect(find.text('Frozen yogurt (0)'), findsOneWidget);
-    expect(find.text('Eclair (0)'), findsNothing);
-    expect(find.text('Gingerbread (0)'), findsNothing);
-
-    await tester.tap(find.icon(Icons.chevron_left));
-
-    expect(log, isEmpty);
-
-    await tester.tap(find.text('2'));
-    await tester.pumpUntilNoTransientCallbacks(const Duration(milliseconds: 200));
-
-    await tester.tap(find.text('8').last);
-    await tester.pumpUntilNoTransientCallbacks(const Duration(milliseconds: 200));
-
-    expect(log, <String>['rows-per-page-changed: 8']);
-    log.clear();
-  });
 }