firka/flutter
3
0
Fork 1
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix MenuItemButton overflow (#143932)

Browse Source 
fixes [MenuItemButton does not constrain its child](https://github.com/flutter/flutter/issues/129439)
fixes [DropdownMenuEntry Text Overflow when width of DropdownMenu is not specified](https://github.com/flutter/flutter/issues/140596)

### Description

- This PR continues the fix from https://github.com/flutter/flutter/pull/141314#issuecomment-1945804640 and adds controlled widths for the `MenuBar` children to fix the unbounded width issue which blocked the PR earlier. (Widgets  with non-zero flex value cannot be laid out in a horizontal scroll view which is created by `MenuBar` widget)
- Added tests coverage.
- Added documentation.

### Code sample

<details>
<summary>expand to view the code sample</summary> 

```dart
import 'package:flutter/material.dart';

void main() => runApp(const MyApp());

class MyApp extends StatefulWidget {
  const MyApp({super.key});

  @override
  State<MyApp> createState() => _MyAppState();
}

class _MyAppState extends State<MyApp> {
  MenuController menuController = MenuController();

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      debugShowCheckedModeBanner: false,
      home: Scaffold(
        body: Padding(
          padding: const EdgeInsets.all(16.0),
          child: Column(
            mainAxisAlignment: MainAxisAlignment.spaceEvenly,
            children: [
              DropdownMenu<int>(
                expandedInsets: EdgeInsets.zero,
                dropdownMenuEntries: <DropdownMenuEntry<int>>[
                  DropdownMenuEntry<int>(
                    value: 0,
                    label:
                        'This is a long text that is multiplied by 10. ' * 10,
                    style: const ButtonStyle(
                      textStyle: MaterialStatePropertyAll(
                        TextStyle(overflow: TextOverflow.ellipsis),
                      ),
                    ),
                  ),
                ],
              ),
              SizedBox(
                width: 200,
                child: MenuItemButton(
                  onPressed: () {},
                  leadingIcon: const Icon(Icons.menu),
                  trailingIcon: const Icon(Icons.arrow_forward_ios),
                  child: const Text(
                    'This is a very long text that will wrap to the multiple lines.',
                    maxLines: 1,
                    overflow: TextOverflow.ellipsis,
                  ),
                ),
              ),
              // MenuBar(
              //   children: [
              //     MenuItemButton(
              //       onPressed: () {

              //       },
              //       child: Text('Short Text Menu'),
              //     ),
              //     MenuItemButton(
              //       onPressed: () {},
              //       child: Text('Very very very very very long text menu'),
              //     ),
              //   ],
              // ),
            ],
          ),
        ),
      ),
    );
  }
}

```

</details>

### Before

![before](https://github.com/flutter/flutter/assets/48603081/27879cf6-4567-442d-a355-7f8492612fa4)

### After
![after](https://github.com/flutter/flutter/assets/48603081/25e5ab90-e2a1-4080-a7e1-51cd98ff0a77)
This commit is contained in:
Taha Tesser
2024-04-02 20:27:26 +03:00
committed by GitHub
parent 174e0b1ac6
commit 8d9dcc48ec
4 changed files with 181 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files

76
packages/flutter/lib/src/material/menu_anchor.dart
View File

@@ -854,6 +854,7 @@ class MenuItemButton extends StatefulWidget {
this.leadingIcon,
this.trailingIcon,
this.closeOnActivate = true,
this.overflowAxis = Axis.horizontal,
required this.child,
});
@@ -923,6 +924,18 @@ class MenuItemButton extends StatefulWidget {
/// {@endtemplate}
final bool closeOnActivate;
/// The direction in which the menu item expands.
///
/// If the menu item button is a descendent of [MenuAnchor] or [MenuBar], then
/// this property is ignored.
///
/// If [overflowAxis] is [Axis.vertical], the menu will be expanded vertically.
/// If [overflowAxis] is [Axis.horizontal], then the menu will be
/// expanded horizontally.
///
/// Defaults to [Axis.horizontal].
final Axis overflowAxis;
/// The widget displayed in the center of this button.
///
/// Typically this is the button's label, using a [Text] widget.
@@ -1052,6 +1065,7 @@ class _MenuItemButtonState extends State<MenuItemButton> {
// If a focus node isn't given to the widget, then we have to manage our own.
FocusNode? _internalFocusNode;
FocusNode get _focusNode => widget.focusNode ?? _internalFocusNode!;
_MenuAnchorState? get _anchor => _MenuAnchorState._maybeOf(context);
@override
void initState() {
@@ -1107,6 +1121,7 @@ class _MenuItemButtonState extends State<MenuItemButton> {
shortcut: widget.shortcut,
trailingIcon: widget.trailingIcon,
hasSubmenu: false,
overflowAxis: _anchor?._orientation ?? widget.overflowAxis,
child: widget.child!,
),
);
@@ -2958,6 +2973,7 @@ class _MenuItemLabel extends StatelessWidget {
this.leadingIcon,
this.trailingIcon,
this.shortcut,
this.overflowAxis = Axis.vertical,
required this.child,
});
@@ -2981,6 +2997,9 @@ class _MenuItemLabel extends StatelessWidget {
/// the shortcut.
final MenuSerializableShortcut? shortcut;
/// The direction in which the menu item expands.
final Axis overflowAxis;
/// The required label child widget.
final Widget child;
@@ -2991,19 +3010,44 @@ class _MenuItemLabel extends StatelessWidget {
_kLabelItemMinSpacing,
_kLabelItemDefaultSpacing + density.horizontal * 2,
);
Widget leadings;
if (overflowAxis == Axis.vertical) {
leadings = Expanded(
child: ClipRect(
child: Row(
mainAxisSize: MainAxisSize.min,
children: <Widget>[
if (leadingIcon != null) leadingIcon!,
Expanded(
child: ClipRect(
child: Padding(
padding: leadingIcon != null ? EdgeInsetsDirectional.only(start: horizontalPadding) : EdgeInsets.zero,
child: child,
),
),
),
],
),
),
);
} else {
leadings = Row(
mainAxisSize: MainAxisSize.min,
children: <Widget>[
if (leadingIcon != null) leadingIcon!,
Padding(
padding: leadingIcon != null ? EdgeInsetsDirectional.only(start: horizontalPadding) : EdgeInsets.zero,
child: child,
),
],
);
}
return Row(
mainAxisAlignment: MainAxisAlignment.spaceBetween,
children: <Widget>[
Row(
mainAxisSize: MainAxisSize.min,
children: <Widget>[
if (leadingIcon != null) leadingIcon!,
Padding(
padding: leadingIcon != null ? EdgeInsetsDirectional.only(start: horizontalPadding) : EdgeInsets.zero,
child: child,
),
],
),
leadings,
if (trailingIcon != null)
Padding(
padding: EdgeInsetsDirectional.only(start: horizontalPadding),
@@ -3337,6 +3381,16 @@ class _MenuPanelState extends State<_MenuPanel> {
}
}
// If the menu panel is horizontal, then the children should be wrapped in
// an IntrinsicWidth widget to ensure that the children are as wide as the
// widest child.
List<Widget> children = widget.children;
if (widget.orientation == Axis.horizontal) {
children = children.map<Widget>((Widget child) {
return IntrinsicWidth(child: child);
}).toList();
}
Widget menuPanel = _intrinsicCrossSize(
child: Material(
elevation: elevation,
@@ -3366,7 +3420,7 @@ class _MenuPanelState extends State<_MenuPanel> {
textDirection: Directionality.of(context),
direction: widget.orientation,
mainAxisSize: MainAxisSize.min,
children: widget.children,
children: children,
),
),
),

23
packages/flutter/test/material/dropdown_menu_test.dart
View File

@@ -2108,6 +2108,29 @@ void main() {
expect(called, 3);
expect(controller.text, 'Green');
});
// This is a regression test for https://github.com/flutter/flutter/issues/140596.
testWidgets('Long text item does not overflow', (WidgetTester tester) async {
await tester.pumpWidget(MaterialApp(
home: Scaffold(
body: DropdownMenu<int>(
dropdownMenuEntries: <DropdownMenuEntry<int>>[
DropdownMenuEntry<int>(
value: 0,
label: 'This is a long text that is multiplied by 4 so it can overflow. ' * 4,
),
],
),
),
));
await tester.pump();
await tester.tap(find.byType(DropdownMenu<int>));
await tester.pumpAndSettle();
// No exception should be thrown.
expect(tester.takeException(), isNull);
});
}
enum TestMenu {

101
packages/flutter/test/material/menu_anchor_test.dart
View File

@@ -662,7 +662,7 @@ void main() {
expect(tester.getRect(find.byType(MenuBar)), equals(const Rect.fromLTRB(0, 0, 800, 48)));
expect(
tester.getRect(find.text(TestMenu.subMenu10.label)),
equals(const Rect.fromLTRB(124.0, 73.0, 278.0, 87.0)),
equals(const Rect.fromLTRB(124.0, 73.0, 314.0, 87.0)),
);
expect(
tester.getRect(
@@ -730,7 +730,7 @@ void main() {
expect(tester.getRect(find.byType(MenuBar)), equals(const Rect.fromLTRB(0, 0, 800, 48)));
expect(
tester.getRect(find.text(TestMenu.subMenu10.label)),
equals(const Rect.fromLTRB(522.0, 73.0, 676.0, 87.0)),
equals(const Rect.fromLTRB(486.0, 73.0, 676.0, 87.0)),
);
expect(
tester.getRect(
@@ -941,7 +941,7 @@ void main() {
expect(tester.getRect(find.byType(MenuBar)), equals(const Rect.fromLTRB(22.0, 22.0, 778.0, 70.0)));
expect(
tester.getRect(find.text(TestMenu.subMenu10.label)),
equals(const Rect.fromLTRB(146.0, 95.0, 300.0, 109.0)),
equals(const Rect.fromLTRB(146.0, 95.0, 336.0, 109.0)),
);
expect(
tester.getRect(find.ancestor(of: find.text(TestMenu.subMenu10.label), matching: find.byType(Material)).at(1)),
@@ -997,7 +997,7 @@ void main() {
expect(tester.getRect(find.byType(MenuBar)), equals(const Rect.fromLTRB(22.0, 22.0, 778.0, 70.0)));
expect(
tester.getRect(find.text(TestMenu.subMenu10.label)),
equals(const Rect.fromLTRB(500.0, 95.0, 654.0, 109.0)),
equals(const Rect.fromLTRB(464.0, 95.0, 654.0, 109.0)),
);
expect(
tester.getRect(find.ancestor(of: find.text(TestMenu.subMenu10.label), matching: find.byType(Material)).at(1)),
@@ -2070,7 +2070,7 @@ void main() {
expect(closed, unorderedEquals(<TestMenu>[TestMenu.mainMenu1, TestMenu.subMenu11]));
expect(opened, isEmpty);
});
});
}, skip: kIsWeb && !isCanvasKit); // https://github.com/flutter/flutter/issues/145527
group('MenuItemButton', () {
testWidgets('Shortcut mnemonics are displayed', (WidgetTester tester) async {
@@ -2191,7 +2191,10 @@ void main() {
expect(mnemonic1.data, equals('Fn'));
mnemonic2 = tester.widget(findMnemonic(TestMenu.subSubMenu112.label));
expect(mnemonic2.data, equals('↵'));
}, variant: TargetPlatformVariant.all());
},
variant: TargetPlatformVariant.all(),
skip: kIsWeb && !isCanvasKit, // https://github.com/flutter/flutter/issues/145527
);
testWidgets('leadingIcon is used when set', (WidgetTester tester) async {
await tester.pumpWidget(
@@ -2247,7 +2250,7 @@ void main() {
await tester.pump();
expect(find.text('trailingIcon'), findsOneWidget);
});
}, skip: kIsWeb && !isCanvasKit); // https://github.com/flutter/flutter/issues/145527
testWidgets('SubmenuButton uses supplied controller', (WidgetTester tester) async {
final MenuController submenuController = MenuController();
@@ -2436,6 +2439,61 @@ void main() {
await tester.pump();
expect(find.byType(MenuItemButton), findsNWidgets(1));
});
// This is a regression test for https://github.com/flutter/flutter/issues/129439.
testWidgets('MenuItemButton does not overflow when child is long', (WidgetTester tester) async {
await tester.pumpWidget(MaterialApp(
home: Scaffold(
body: SizedBox(
width: 200,
child: MenuItemButton(
overflowAxis: Axis.vertical,
onPressed: () {},
child: const Text('MenuItem Button does not overflow when child is long'),
),
),
),
));
// No exception should be thrown.
expect(tester.takeException(), isNull);
});
testWidgets('MenuItemButton layout is updated by overflowAxis', (WidgetTester tester) async {
Widget buildMenuButton({ required Axis overflowAxis, bool constrainedLayout = false }) {
return MaterialApp(
home: Scaffold(
body: SizedBox(
width: constrainedLayout ? 200 : null,
child: MenuItemButton(
overflowAxis: overflowAxis,
onPressed: () {},
child: const Text('This is a very long text that will wrap to the multiple lines.'),
),
),
),
);
}
// Test a long MenuItemButton in an unconstrained layout with vertical overflow axis.
await tester.pumpWidget(buildMenuButton(overflowAxis: Axis.vertical));
expect(tester.getSize(find.byType(MenuItemButton)), const Size(800.0, 48.0));
// Test a long MenuItemButton in an unconstrained layout with horizontal overflow axis.
await tester.pumpWidget(buildMenuButton(overflowAxis: Axis.horizontal));
expect(tester.getSize(find.byType(MenuItemButton)), const Size(800.0, 48.0));
// Test a long MenuItemButton in a constrained layout with vertical overflow axis.
await tester.pumpWidget(buildMenuButton(overflowAxis: Axis.vertical, constrainedLayout: true));
expect(tester.getSize(find.byType(MenuItemButton)), const Size(200.0, 120.0));
// Test a long MenuItemButton in a constrained layout with horizontal overflow axis.
await tester.pumpWidget(buildMenuButton(overflowAxis: Axis.horizontal, constrainedLayout: true));
expect(tester.getSize(find.byType(MenuItemButton)), const Size(200.0, 48.0));
// This should throw an error.
final AssertionError exception = tester.takeException() as AssertionError;
expect(exception, isAssertionError);
}, skip: kIsWeb && !isCanvasKit); // https://github.com/flutter/flutter/issues/99933
});
group('Layout', () {
@@ -3159,7 +3217,7 @@ void main() {
expect(find.text(allExpected), findsOneWidget);
expect(find.text(charExpected), findsOneWidget);
}, variant: TargetPlatformVariant.all());
});
}, skip: kIsWeb && !isCanvasKit); // https://github.com/flutter/flutter/issues/145527
group('CheckboxMenuButton', () {
testWidgets('tapping toggles checkbox', (WidgetTester tester) async {
@@ -3566,7 +3624,7 @@ void main() {
expect(material.textStyle?.fontStyle, menuTextStyle.fontStyle);
expect(material.textStyle?.wordSpacing, menuTextStyle.wordSpacing);
expect(material.textStyle?.decoration, menuTextStyle.decoration);
});
}, skip: kIsWeb && !isCanvasKit); // https://github.com/flutter/flutter/issues/145527
testWidgets('SubmenuButton.onFocusChange is respected', (WidgetTester tester) async {
final FocusNode focusNode = FocusNode();
@@ -3605,6 +3663,31 @@ void main() {
expect(focusNode.hasFocus, false);
expect(onFocusChangeCalled, 2);
});
testWidgets('Horizontal _MenuPanel wraps children with IntrinsicWidth', (WidgetTester tester) async {
await tester.pumpWidget(
MaterialApp(
home: Material(
child: MenuBar(
children: <Widget>[
MenuItemButton(
onPressed: () {},
child: const Text('Menu Item'),
),
],
),
),
),
);
// Horizontal _MenuPanel wraps children with IntrinsicWidth to ensure MenuItemButton
// with vertical overflow axis is as wide as the widest child.
final Finder intrinsicWidthFinder = find.ancestor(
of: find.byType(MenuItemButton),
matching: find.byType(IntrinsicWidth),
);
expect(intrinsicWidthFinder, findsOneWidget);
});
}
List<Widget> createTestMenus({

2
packages/flutter/test/material/menu_style_test.dart
View File

@@ -280,7 +280,7 @@ void main() {
expect(tester.getRect(find.byType(MenuBar)), equals(const Rect.fromLTRB(228.0, 0.0, 572.0, 48.0)));
expect(
tester.getRect(find.text(TestMenu.subMenu10.label)),
equals(const Rect.fromLTRB(366.0, 68.0, 520.0, 82.0)),
equals(const Rect.fromLTRB(366.0, 68.0, 559.0, 82.0)),
);
expect(
tester.getRect(find.ancestor(of: find.text(TestMenu.subMenu10.label), matching: find.byType(Material)).at(1)),