Commit d1b222be authored by perlatus's avatar perlatus Committed by Ian Hickson

PopupMenuButton: create IconButton if child is Icon (#10230)

* PopupMenuButton: create IconButton if child is Icon

Otherwise the resulting button has an abnormally small and rectangular
area. With multiple PopupMenuButton(child: Icon) they get squished
together in the AppBar.

* Add separate icon argument to PopupMenuButton

* Fix style issues and tweak dartdocs

* Add tests for icon argument to PopupMenuButton

* Group icon tests and fix broken test, analyzer warnings

* Test that the correct custom icon is present

* Apply De Morgan's to work around dart analyzer bug

see: https://github.com/dart-lang/sdk/issues/30288
parent cfcf6d44
...@@ -617,8 +617,13 @@ typedef List<PopupMenuEntry<T>> PopupMenuItemBuilder<T>(BuildContext context); ...@@ -617,8 +617,13 @@ typedef List<PopupMenuEntry<T>> PopupMenuItemBuilder<T>(BuildContext context);
/// Displays a menu when pressed and calls [onSelected] when the menu is dismissed /// Displays a menu when pressed and calls [onSelected] when the menu is dismissed
/// because an item was selected. The value passed to [onSelected] is the value of /// because an item was selected. The value passed to [onSelected] is the value of
/// the selected menu item. If child is null then a standard 'navigation/more_vert' /// the selected menu item.
/// icon is created. ///
/// One of [child] or [icon] may be provided, but not both. If [icon] is provided,
/// then [PopupMenuButton] behaves like an [IconButton].
///
/// If both are null, then a standard overflow icon is created (depending on the
/// platform).
/// ///
/// ## Sample code /// ## Sample code
/// ///
...@@ -672,8 +677,10 @@ class PopupMenuButton<T> extends StatefulWidget { ...@@ -672,8 +677,10 @@ class PopupMenuButton<T> extends StatefulWidget {
this.tooltip: 'Show menu', this.tooltip: 'Show menu',
this.elevation: 8.0, this.elevation: 8.0,
this.padding: const EdgeInsets.all(8.0), this.padding: const EdgeInsets.all(8.0),
this.child this.child,
this.icon,
}) : assert(itemBuilder != null), }) : assert(itemBuilder != null),
assert(!(child != null && icon != null)), // fails if passed both parameters
super(key: key); super(key: key);
/// Called when the button is pressed to create the items to show in the menu. /// Called when the button is pressed to create the items to show in the menu.
...@@ -702,9 +709,12 @@ class PopupMenuButton<T> extends StatefulWidget { ...@@ -702,9 +709,12 @@ class PopupMenuButton<T> extends StatefulWidget {
/// to set the padding to zero. /// to set the padding to zero.
final EdgeInsets padding; final EdgeInsets padding;
/// The widget below this widget in the tree. /// If provided, the widget used for this button.
final Widget child; final Widget child;
/// If provided, the icon used for this button.
final Icon icon;
@override @override
_PopupMenuButtonState<T> createState() => new _PopupMenuButtonState<T>(); _PopupMenuButtonState<T> createState() => new _PopupMenuButtonState<T>();
} }
...@@ -747,17 +757,16 @@ class _PopupMenuButtonState<T> extends State<PopupMenuButton<T>> { ...@@ -747,17 +757,16 @@ class _PopupMenuButtonState<T> extends State<PopupMenuButton<T>> {
@override @override
Widget build(BuildContext context) { Widget build(BuildContext context) {
if (widget.child == null) { return widget.child != null
return new IconButton( ? new InkWell(
icon: _getIcon(Theme.of(context).platform), onTap: showButtonMenu,
child: widget.child,
)
: new IconButton(
icon: widget.icon ?? _getIcon(Theme.of(context).platform),
padding: widget.padding, padding: widget.padding,
tooltip: widget.tooltip, tooltip: widget.tooltip,
onPressed: showButtonMenu, onPressed: showButtonMenu,
); );
} }
return new InkWell(
onTap: showButtonMenu,
child: widget.child,
);
}
} }
...@@ -90,4 +90,46 @@ void main() { ...@@ -90,4 +90,46 @@ void main() {
expect(find.byIcon(Icons.more_vert), findsNothing); expect(find.byIcon(Icons.more_vert), findsNothing);
expect(find.byIcon(Icons.more_horiz), findsOneWidget); expect(find.byIcon(Icons.more_horiz), findsOneWidget);
}); });
group('PopupMenuButton with Icon', () {
// Helper function to create simple and valid popup menus.
List<PopupMenuItem<int>> simplePopupMenuItemBuilder(BuildContext context) {
return <PopupMenuItem<int>>[
const PopupMenuItem<int>(
value: 1,
child: const Text('1'),
),
];
}
testWidgets('PopupMenuButton fails when given both child and icon', (WidgetTester tester) async {
expect(() {
new PopupMenuButton<int>(
child: const Text('heyo'),
icon: const Icon(Icons.view_carousel),
itemBuilder: simplePopupMenuItemBuilder,
);
}, throwsA(new isInstanceOf<AssertionError>()));
});
testWidgets('PopupMenuButton creates IconButton when given an icon', (WidgetTester tester) async {
final PopupMenuButton<int> button = new PopupMenuButton<int>(
icon: const Icon(Icons.view_carousel),
itemBuilder: simplePopupMenuItemBuilder,
);
await tester.pumpWidget(new MaterialApp(
home: new Scaffold(
appBar: new AppBar(
actions: <Widget>[button],
),
),
),
);
expect(find.byType(IconButton), findsOneWidget);
expect(find.byIcon(Icons.view_carousel), findsOneWidget);
});
});
} }
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