Skip to content

Commit

Permalink
Adds PopupMenuPosition position to the PopupMenuThemeData (#110268)
Browse files Browse the repository at this point in the history
  • Loading branch information
@ValentinVignal
ValentinVignal committed Sep 6, 2022
1 parent 22cef48 commit 27896a7
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 38 deletions.
21 changes: 8 additions & 13 deletions packages/flutter/lib/src/material/popup_menu.dart
View file
Expand Up @@ -38,14 +38,6 @@ const double _kMenuVerticalPadding = 8.0;
const double _kMenuWidthStep = 56.0;
const double _kMenuScreenPadding = 8.0;

/// Used to configure how the [PopupMenuButton] positions its popup menu.
enum PopupMenuPosition {
/// Menu is positioned over the anchor.
over,
/// Menu is positioned under the anchor.
under,
}

/// A base class for entries in a Material Design popup menu.
///
/// The popup menu widget uses this interface to interact with the menu items.
Expand Down Expand Up @@ -1025,7 +1017,7 @@ class PopupMenuButton<T> extends StatefulWidget {
this.color,
this.enableFeedback,
this.constraints,
this.position = PopupMenuPosition.over,
this.position,
this.clipBehavior = Clip.none,
}) : assert(itemBuilder != null),
assert(enabled != null),
Expand Down Expand Up @@ -1157,9 +1149,11 @@ class PopupMenuButton<T> extends StatefulWidget {
/// [offset] is used to change the position of the popup menu relative to the
/// position set by this parameter.
///
/// When not set, the position defaults to [PopupMenuPosition.over] which makes the
/// popup menu appear directly over the button that was used to create it.
final PopupMenuPosition position;
/// If this property is `null`, then [PopupMenuThemeData.position] is used. If
/// [PopupMenuThemeData.position] is also `null`, then the position defaults
/// to [PopupMenuPosition.over] which makes the popup menu appear directly
/// over the button that was used to create it.
final PopupMenuPosition? position;

/// {@macro flutter.material.Material.clipBehavior}
///
Expand Down Expand Up @@ -1189,8 +1183,9 @@ class PopupMenuButtonState<T> extends State<PopupMenuButton<T>> {
final PopupMenuThemeData popupMenuTheme = PopupMenuTheme.of(context);
final RenderBox button = context.findRenderObject()! as RenderBox;
final RenderBox overlay = Navigator.of(context).overlay!.context.findRenderObject()! as RenderBox;
final PopupMenuPosition popupMenuPosition = widget.position ?? popupMenuTheme.position ?? PopupMenuPosition.over;
final Offset offset;
switch (widget.position) {
switch (popupMenuPosition) {
case PopupMenuPosition.over:
offset = widget.offset;
break;
Expand Down
23 changes: 22 additions & 1 deletion packages/flutter/lib/src/material/popup_menu_theme.dart
View file
Expand Up @@ -13,6 +13,14 @@ import 'theme.dart';
// Examples can assume:
// late BuildContext context;

/// Used to configure how the [PopupMenuButton] positions its popup menu.
enum PopupMenuPosition {
/// Menu is positioned over the anchor.
over,
/// Menu is positioned under the anchor.
under,
}

/// Defines the visual properties of the routes used to display popup menus
/// as well as [PopupMenuItem] and [PopupMenuDivider] widgets.
///
Expand Down Expand Up @@ -43,6 +51,7 @@ class PopupMenuThemeData with Diagnosticable {
this.textStyle,
this.enableFeedback,
this.mouseCursor,
this.position,
});

/// The background color of the popup menu.
Expand All @@ -67,6 +76,12 @@ class PopupMenuThemeData with Diagnosticable {
/// If specified, overrides the default value of [PopupMenuItem.mouseCursor].
final MaterialStateProperty<MouseCursor?>? mouseCursor;

/// Whether the popup menu is positioned over or under the popup menu button.
///
/// When not set, the position defaults to [PopupMenuPosition.over] which makes the
/// popup menu appear directly over the button that was used to create it.
final PopupMenuPosition? position;

/// Creates a copy of this object with the given fields replaced with the
/// new values.
PopupMenuThemeData copyWith({
Expand All @@ -76,6 +91,7 @@ class PopupMenuThemeData with Diagnosticable {
TextStyle? textStyle,
bool? enableFeedback,
MaterialStateProperty<MouseCursor?>? mouseCursor,
PopupMenuPosition? position,
}) {
return PopupMenuThemeData(
color: color ?? this.color,
Expand All @@ -84,6 +100,7 @@ class PopupMenuThemeData with Diagnosticable {
textStyle: textStyle ?? this.textStyle,
enableFeedback: enableFeedback ?? this.enableFeedback,
mouseCursor: mouseCursor ?? this.mouseCursor,
position: position ?? this.position,
);
}

Expand All @@ -104,6 +121,7 @@ class PopupMenuThemeData with Diagnosticable {
textStyle: TextStyle.lerp(a?.textStyle, b?.textStyle, t),
enableFeedback: t < 0.5 ? a?.enableFeedback : b?.enableFeedback,
mouseCursor: t < 0.5 ? a?.mouseCursor : b?.mouseCursor,
position: t < 0.5 ? a?.position : b?.position,
);
}

Expand All @@ -115,6 +133,7 @@ class PopupMenuThemeData with Diagnosticable {
textStyle,
enableFeedback,
mouseCursor,
position,
);

@override
Expand All @@ -131,7 +150,8 @@ class PopupMenuThemeData with Diagnosticable {
&& other.shape == shape
&& other.textStyle == textStyle
&& other.enableFeedback == enableFeedback
&& other.mouseCursor == mouseCursor;
&& other.mouseCursor == mouseCursor
&& other.position == position;
}

@override
Expand All @@ -143,6 +163,7 @@ class PopupMenuThemeData with Diagnosticable {
properties.add(DiagnosticsProperty<TextStyle>('text style', textStyle, defaultValue: null));
properties.add(DiagnosticsProperty<bool>('enableFeedback', enableFeedback, defaultValue: null));
properties.add(DiagnosticsProperty<MaterialStateProperty<MouseCursor?>>('mouseCursor', mouseCursor, defaultValue: null));
properties.add(EnumProperty<PopupMenuPosition?>('position', position, defaultValue: null));
}
}

Expand Down
80 changes: 56 additions & 24 deletions packages/flutter/test/material/popup_menu_theme_test.dart
View file
Expand Up @@ -13,6 +13,7 @@ PopupMenuThemeData _popupMenuTheme() {
shape: BeveledRectangleBorder(borderRadius: BorderRadius.all(Radius.circular(12))),
elevation: 12.0,
textStyle: TextStyle(color: Color(0xffffffff), textBaseline: TextBaseline.alphabetic),
position: PopupMenuPosition.under,
);
}

Expand All @@ -29,6 +30,7 @@ void main() {
expect(popupMenuTheme.elevation, null);
expect(popupMenuTheme.textStyle, null);
expect(popupMenuTheme.mouseCursor, null);
expect(popupMenuTheme.position, null);
});

testWidgets('Default PopupMenuThemeData debugFillProperties', (WidgetTester tester) async {
Expand All @@ -51,6 +53,7 @@ void main() {
elevation: 2.0,
textStyle: TextStyle(color: Color(0xffffffff)),
mouseCursor: MaterialStateMouseCursor.clickable,
position: PopupMenuPosition.over,
).debugFillProperties(builder);

final List<String> description = builder.properties
Expand All @@ -64,6 +67,7 @@ void main() {
'elevation: 2.0',
'text style: TextStyle(inherit: true, color: Color(0xffffffff))',
'mouseCursor: MaterialStateMouseCursor(clickable)',
'position: over'
]);
});

Expand All @@ -78,16 +82,21 @@ void main() {
home: Material(
child: Column(
children: <Widget>[
PopupMenuButton<void>(
key: popupButtonKey,
itemBuilder: (BuildContext context) {
return <PopupMenuEntry<void>>[
PopupMenuItem<void>(
key: popupItemKey,
child: const Text('Example'),
),
];
},
Padding(
// The padding makes sure the menu as enough space to around to
// get properly aligned when displayed (`_kMenuScreenPadding`).
padding: const EdgeInsets.all(8.0),
child: PopupMenuButton<void>(
key: popupButtonKey,
itemBuilder: (BuildContext context) {
return <PopupMenuEntry<void>>[
PopupMenuItem<void>(
key: popupItemKey,
child: const Text('Example'),
),
];
},
),
),
],
),
Expand Down Expand Up @@ -123,6 +132,11 @@ void main() {
);
expect(text.style.fontFamily, 'Roboto');
expect(text.style.color, const Color(0xdd000000));
expect(text.style.color, const Color(0xdd000000));

final Offset topLeftButton = tester.getTopLeft(find.byType(PopupMenuButton<void>));
final Offset topLeftMenu = tester.getTopLeft(find.byWidget(button));
expect(topLeftMenu, topLeftButton);
});

testWidgets('Popup menu uses values from PopupMenuThemeData', (WidgetTester tester) async {
Expand All @@ -138,6 +152,10 @@ void main() {
child: Column(
children: <Widget>[
PopupMenuButton<void>(
// The padding is used in the positioning of the menu when the
// position is `PopupMenuPosition.under`. Setting it to zero makes
// it easier to test.
padding: EdgeInsets.zero,
key: popupButtonKey,
itemBuilder: (BuildContext context) {
return <PopupMenuEntry<Object>>[
Expand Down Expand Up @@ -181,6 +199,10 @@ void main() {
).last,
);
expect(text.style, popupMenuTheme.textStyle);

final Offset bottomLeftButton = tester.getBottomLeft(find.byType(PopupMenuButton<void>));
final Offset topLeftMenu = tester.getTopLeft(find.byWidget(button));
expect(topLeftMenu, bottomLeftButton);
});

testWidgets('Popup menu widget properties take priority over theme', (WidgetTester tester) async {
Expand All @@ -202,20 +224,26 @@ void main() {
home: Material(
child: Column(
children: <Widget>[
PopupMenuButton<void>(
key: popupButtonKey,
elevation: elevation,
color: color,
shape: shape,
itemBuilder: (BuildContext context) {
return <PopupMenuEntry<void>>[
PopupMenuItem<void>(
key: popupItemKey,
textStyle: textStyle,
child: const Text('Example'),
),
];
},
Padding(
// The padding makes sure the menu as enough space to around to
// get properly aligned when displayed (`_kMenuScreenPadding`).
padding: const EdgeInsets.all(8.0),
child: PopupMenuButton<void>(
key: popupButtonKey,
elevation: elevation,
color: color,
shape: shape,
position: PopupMenuPosition.over,
itemBuilder: (BuildContext context) {
return <PopupMenuEntry<void>>[
PopupMenuItem<void>(
key: popupItemKey,
textStyle: textStyle,
child: const Text('Example'),
),
];
},
),
),
],
),
Expand Down Expand Up @@ -250,6 +278,10 @@ void main() {
).last,
);
expect(text.style, textStyle);

final Offset topLeftButton = tester.getTopLeft(find.byType(PopupMenuButton<void>));
final Offset topLeftMenu = tester.getTopLeft(find.byWidget(button));
expect(topLeftMenu, topLeftButton);
});

testWidgets('ThemeData.popupMenuTheme properties are utilized', (WidgetTester tester) async {
Expand Down

0 comments on commit 27896a7

Please sign in to comment.