Fix menu item overflow by QuncCccccc · Pull Request #141314 · flutter/flutter

QuncCccccc · 2024-01-10T22:06:09Z

Fixes #129439 and #140596

This PR is to fix the overflow issue when the menu item contains long content in MenuItemButton and DropdownMenu.

Before:

After:

Pre-launch Checklist

I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
I read the [Tree Hygiene] wiki page, which explains my responsibilities.
I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
I signed the [CLA].
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is [test-exempt].
All existing and new tests are passing.

TahaTesser · 2024-02-13T21:51:06Z

Hans told me you got a lot on your plate. I'll help finish with this.

I'll push commits to this draft.

QuncCccccc · 2024-02-13T21:55:34Z

Hans told me you got a lot on your plate. I'll help finish with this.

I'll push commits to this draft.

Thanks so much for the help! Really appreciated!

Currently for this PR, I think it can fix the issues it links, but some problems still happen. Please try to run some MenuBar examples. I think there're some problems.

TahaTesser · 2024-02-14T11:29:41Z

@QuncCccccc
Could please point to the example? I tried both MenuBar examples with very very long strings and i couldn't produce the overflow on desktop with this PR code.

QuncCccccc · 2024-02-14T17:20:08Z

@QuncCccccc Could please point to the example? I tried both MenuBar examples with very very long strings and i couldn't produce the overflow on desktop with this PR code.

Ah interesting. Let me have a quick look. It's been a while that I didn't work on this issue.

QuncCccccc · 2024-02-14T20:24:31Z

Basically this menuDirection property means whether the button is placed on a vertical menu or a horizontal menu. And usually MenuItemButton should be on a vertical menu. So I set a default value menuDirection = Axis.vertical for MenuItemButton. But if we put a MenuItemButton on MenuBar and don't explicitly set menuDirection: Axis.horizontal, then it will go to the Expanded(child: ClipRect(...)) part and throw exception. This will break existing apps.

code sample

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

/// Flutter code sample for [MenuAcceleratorLabel].

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

class MyMenuBar extends StatelessWidget {
  const MyMenuBar({super.key});

  @override
  Widget build(BuildContext context) {
    return Column(
      children: <Widget>[
        Row(
          mainAxisSize: MainAxisSize.min,
          children: <Widget>[
            MenuBar(
              children: <Widget>[
                MenuItemButton(
                  // menuDirection: Axis.horizontal,
                  onPressed: () {
                    showAboutDialog(
                      context: context,
                      applicationName: 'MenuBar Sample',
                      applicationVersion: '1.0.0',
                    );
                  },
                  child: Text('&About'),
                ),
                // SubmenuButton(
                //   menuChildren: <Widget>[
                //     // MenuItemButton(
                //     //   onPressed: () {
                //     //     showAboutDialog(
                //     //       context: context,
                //     //       applicationName: 'MenuBar Sample',
                //     //       applicationVersion: '1.0.0',
                //     //     );
                //     //   },
                //     //   child: Text('&About'),
                //     // ),
                //     // MenuItemButton(
                //     //   onPressed: () {
                //     //     ScaffoldMessenger.of(context).showSnackBar(
                //     //       const SnackBar(
                //     //         content: Text('Saved!'),
                //     //       ),
                //     //     );
                //     //   },
                //     //   child: Text('&Save'),
                //     // ),
                //     // MenuItemButton(
                //     //   onPressed: () {
                //     //     ScaffoldMessenger.of(context).showSnackBar(
                //     //       const SnackBar(
                //     //         content: Text('Quit!'),
                //     //       ),
                //     //     );
                //     //   },
                //     //   child: Text('&Quit'),
                //     // ),
                //   ],
                //   child: Text('&File'),
                // ),
                // SubmenuButton(
                //   menuChildren: <Widget>[
                //     // MenuItemButton(
                //     //   onPressed: () {
                //     //     ScaffoldMessenger.of(context).showSnackBar(
                //     //       const SnackBar(
                //     //         content: Text('Magnify!'),
                //     //       ),
                //     //     );
                //     //   },
                //     //   child: Text('&Magnify'),
                //     // ),
                //     // MenuItemButton(
                //     //   onPressed: () {
                //     //     ScaffoldMessenger.of(context).showSnackBar(
                //     //       const SnackBar(
                //     //         content: Text('Minify!'),
                //     //       ),
                //     //     );
                //     //   },
                //     //   child: Text('Mi&nify'),
                //     // ),
                //   ],
                //   child: Text('&View'),
                // ),
              ],
            ),
          ],
        ),
        Expanded(
          child: FlutterLogo(
            size: MediaQuery.of(context).size.shortestSide * 0.5,
          ),
        ),
      ],
    );
  }
}

class MenuAcceleratorApp extends StatelessWidget {
  const MenuAcceleratorApp({super.key});

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      theme: ThemeData(useMaterial3: true),
      home: Shortcuts(
        shortcuts: <ShortcutActivator, Intent>{
          const SingleActivator(LogicalKeyboardKey.keyT, control: true): VoidCallbackIntent(() {
            debugDumpApp();
          }),
        },
        child: const Scaffold(body: MyMenuBar()),
      ),
    );
  }
}

TahaTesser · 2024-02-15T10:38:47Z

I can see the issue. Thanks for the details.

Since this PR has conflicts and some challenges in order to land this. I'll close this PR then investigate the problem, write tests, and file a PR myself.

@OverRide

fixes [MenuItemButton does not constrain its child](#129439) fixes [DropdownMenuEntry Text Overflow when width of DropdownMenu is not specified](#140596) ### Description - This PR continues the fix from #141314 (comment) 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)

github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels Jan 10, 2024

QuncCccccc added 7 commits January 11, 2024 18:19

Fix long text overflow

a5e8f96

Update

20a14e7

Remove unused import

4b5fcff

Remove unused import

e9e3ade

update

eec76d0

Update test

4f65f5b

Update

c992679

QuncCccccc force-pushed the fix_menu_overflow branch from ef1f3ec to c992679 Compare January 12, 2024 02:19

TahaTesser self-requested a review February 13, 2024 21:50

TahaTesser closed this Feb 15, 2024

TahaTesser mentioned this pull request Feb 22, 2024

Fix MenuItemButton overflow #143932

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix menu item overflow#141314

Fix menu item overflow#141314
QuncCccccc wants to merge 7 commits intoflutter:masterfrom
QuncCccccc:fix_menu_overflow

QuncCccccc commented Jan 10, 2024

Uh oh!

TahaTesser commented Feb 13, 2024 •

edited

Loading

Uh oh!

QuncCccccc commented Feb 13, 2024

Uh oh!

TahaTesser commented Feb 14, 2024 •

edited

Loading

Uh oh!

QuncCccccc commented Feb 14, 2024

Uh oh!

QuncCccccc commented Feb 14, 2024

Uh oh!

TahaTesser commented Feb 15, 2024 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

QuncCccccc commented Jan 10, 2024

Pre-launch Checklist

Uh oh!

TahaTesser commented Feb 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

QuncCccccc commented Feb 13, 2024

Uh oh!

TahaTesser commented Feb 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

QuncCccccc commented Feb 14, 2024

Uh oh!

QuncCccccc commented Feb 14, 2024

Uh oh!

TahaTesser commented Feb 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

TahaTesser commented Feb 13, 2024 •

edited

Loading

TahaTesser commented Feb 14, 2024 •

edited

Loading

TahaTesser commented Feb 15, 2024 •

edited

Loading