Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add vertical alignment offset to the MenuAnchor widget when overflowing #123740

Merged
merged 11 commits into from Apr 6, 2023
Merged

Add vertical alignment offset to the MenuAnchor widget when overflowing #123740

merged 11 commits into from Apr 6, 2023

Conversation

whiskeyPeak
Copy link
Contributor

@whiskeyPeak whiskeyPeak commented Mar 30, 2023 •
edited

Adds a vertical offset to the MenuAnchor widget when the menu cannot be fitted below the anchor and there is enough space above it. Previously, the vertical offset would be ignored in such conditions. Still needs a test.

simplescreenrecorder-2023-03-30_01.45.29.mp4
Repro code: 
import 'package:flutter/material.dart';

void main() {
  runApp(const MyApp());
}

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

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      title: 'Flutter Demo',
      theme: ThemeData(
        //useMaterial3: true,
        colorScheme: const ColorScheme.dark(),
      ),
      home: const Directionality(
          textDirection: TextDirection.ltr,
          child: MyHomePage(title: 'Flutter Demo Home Page')),
    );
  }
}

class MyHomePage extends StatefulWidget {
  const MyHomePage({super.key, required this.title});

  final String title;

  @override
  State<MyHomePage> createState() => _MyHomePageState();
}

class _MyHomePageState extends State<MyHomePage> {
  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(
        title: Text(MediaQuery.of(context).size.height.toString()),
        centerTitle: true,
      ),
      body: const Row(
        mainAxisAlignment: MainAxisAlignment.spaceEvenly,
        children: [
          Column(
            children: [
              ButtonAnchorExample(),
              Expanded(child: SizedBox()),
            ],
          ),
          Column(
            children: [
              Expanded(child: SizedBox()),
              ButtonAnchorExample(),
            ],
          ),
        ],
      ),
    );
  }
}

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

  @override
  State<ButtonAnchorExample> createState() => _ButtonAnchorExampleState();
}

class _ButtonAnchorExampleState extends State<ButtonAnchorExample> {

  MenuController controller = MenuController();

  @override
  Widget build(BuildContext context) {
    return MenuAnchor(
      controller: controller,
      clipBehavior: Clip.none,
      style: MenuStyle(
        //maximumSize: MaterialStateProperty.all(const Size(500, 300)),
        //visualDensity: VisualDensity.compact,
        //alignment: AlignmentDirectional.topEnd
        //shape: MaterialStatePropertyAll<RoundedRectangleBorder>(RoundedRectangleBorder(borderRadius: BorderRadius.circular(40))),
        //shadowColor: MaterialStatePropertyAll<Color>(Colors.transparent),
      ),
      // This offset is not correct when menu direction is reversed
      alignmentOffset: const Offset(0, 20),
      builder: (context, controller, child) {
        return FilledButton.tonal(
          onPressed: () {
            if (controller.isOpen) {
              controller.close();
            } else {
              controller.open();
            }
          },
          child: const Text('Show menu'),
        );
      },
      menuChildren: [
        MenuItemButton(
          //keepMenuOpen: true,
          leadingIcon: const Icon(Icons.people_alt_outlined),
          child: const Text('Item 1'),
          onPressed: () {
            Future.delayed(Duration(milliseconds: 1000), () {
              setState(() {
                controller.close();
              });
            });
          },
        ),
        MenuItemButton(
          leadingIcon: const Icon(Icons.remove_red_eye_outlined),
          child: const Text('Item 2'),
          onPressed: () {},
        ),
        MenuItemButton(
          leadingIcon: const Icon(Icons.refresh),
          onPressed: () {},
          child: const Text('Item 3'),
        ),
        SubmenuButton(
          menuStyle: MenuStyle(
        //maximumSize: MaterialStateProperty.all(const Size(42, 200)),
        //visualDensity: VisualDensity.compact,
        //alignment: AlignmentDirectional.topEnd
        shape: MaterialStatePropertyAll<RoundedRectangleBorder>(RoundedRectangleBorder(borderRadius: BorderRadius.circular(40))),
        //shadowColor: MaterialStatePropertyAll<Color>(Colors.transparent),
      ),
          trailingIcon: const Icon(Icons.add),

          alignmentOffset: const Offset(8, 0),
          menuChildren: [
            MenuItemButton(
              leadingIcon: const Icon(Icons.people_alt_outlined),
              child: const Text('Item 1'),
              onPressed: () {},
            ),
            MenuItemButton(
              leadingIcon: const Icon(Icons.remove_red_eye_outlined),
              child: const Text('Item 2'),
              onPressed: () {},
            ),
            MenuItemButton(
              leadingIcon: const Icon(Icons.refresh),
              onPressed: () {},
              child: const Text('Item 3'),
            ),
          ],
          child: const Text("SubMenuButton"),
        ),
        SubmenuButton(
          menuStyle: const MenuStyle(
            visualDensity: VisualDensity.compact,
          ),
          alignmentOffset: const Offset(8, 0),
          menuChildren: [
            MenuItemButton(
              leadingIcon: const Icon(Icons.people_alt_outlined),
              child: const Text('Item 1'),
              onPressed: () {},
            ),
            MenuItemButton(
              leadingIcon: const Icon(Icons.remove_red_eye_outlined),
              child: const Text('Item 2'),
              onPressed: () {},
            ),
            MenuItemButton(
              leadingIcon: const Icon(Icons.refresh),
              onPressed: () {},
              child: const Text('Item 3'),
            ),
          ],
          child: const Text("SubMenuButton"),
        ),
        SubmenuButton(
          menuStyle: const MenuStyle(
            visualDensity: VisualDensity.compact,
          ),
          alignmentOffset: const Offset(8, 0),
          menuChildren: [
            MenuItemButton(
              leadingIcon: const Icon(Icons.people_alt_outlined),
              child: const Text('Item 1'),
              onPressed: () {},
            ),
            MenuItemButton(
              leadingIcon: const Icon(Icons.remove_red_eye_outlined),
              child: const Text('Item 2'),
              onPressed: () {},
            ),
            MenuItemButton(
              leadingIcon: const Icon(Icons.refresh),
              onPressed: () {},
              child: const Text('Item 3'),
            ),
          ],
          child: const Text("SubMenuButton"),
        ),
        SubmenuButton(
          menuStyle: const MenuStyle(
            visualDensity: VisualDensity.compact,
          ),
          alignmentOffset: const Offset(8, 0),
          menuChildren: [
            MenuItemButton(
              leadingIcon: const Icon(Icons.people_alt_outlined),
              child: const Text('Item 1'),
              onPressed: () {},
            ),
            MenuItemButton(
              leadingIcon: const Icon(Icons.remove_red_eye_outlined),
              child: const Text('Item 2'),
              onPressed: () {},
            ),
            MenuItemButton(
              leadingIcon: const Icon(Icons.refresh),
              onPressed: () {},
              child: const Text('Item 3'),
            ),
          ],
          child: const Text("SubMenuButton"),
        ),
        SubmenuButton(
          menuStyle: const MenuStyle(
            visualDensity: VisualDensity.compact,
          ),
          alignmentOffset: const Offset(8, 0),
          menuChildren: [
            MenuItemButton(
              leadingIcon: const Icon(Icons.people_alt_outlined),
              child: const Text('Item 1'),
              onPressed: () {},
            ),
            MenuItemButton(
              leadingIcon: const Icon(Icons.remove_red_eye_outlined),
              child: const Text('Item 2'),
              onPressed: () {},
            ),
            MenuItemButton(
              leadingIcon: const Icon(Icons.refresh),
              onPressed: () {},
              child: const Text('Item 3'),
            ),
          ],
          child: const Text("SubMenuButton"),
        ),
        SubmenuButton(
          menuStyle: const MenuStyle(
            visualDensity: VisualDensity.compact,
          ),
          alignmentOffset: const Offset(8, 0),
          menuChildren: [
            SubmenuButton(
              menuStyle: const MenuStyle(
                visualDensity: VisualDensity.compact,
              ),
              alignmentOffset: const Offset(8, 0),
              menuChildren: [
                MenuItemButton(
                  leadingIcon: const Icon(Icons.people_alt_outlined),
                  child: const Text('Item 1'),
                  onPressed: () {},
                ),
                MenuItemButton(
                  leadingIcon: const Icon(Icons.remove_red_eye_outlined),
                  child: const Text('Item 2'),
                  onPressed: () {},
                ),
                MenuItemButton(
                  leadingIcon: const Icon(Icons.refresh),
                  onPressed: () {},
                  child: const Text('Item 3'),
                ),
              ],
              child: const Text("SubMenuButton"),
            ),
            SubmenuButton(
              menuStyle: const MenuStyle(
                visualDensity: VisualDensity.compact,
              ),
              alignmentOffset: const Offset(8, 0),
              menuChildren: [
                MenuItemButton(
                  leadingIcon: const Icon(Icons.people_alt_outlined),
                  child: const Text('Item 1'),
                  onPressed: () {},
                ),
                MenuItemButton(
                  leadingIcon: const Icon(Icons.remove_red_eye_outlined),
                  child: const Text('Item 2'),
                  onPressed: () {},
                ),
                MenuItemButton(
                  leadingIcon: const Icon(Icons.refresh),
                  onPressed: () {},
                  child: const Text('Item 3'),
                ),
              ],
              child: const Text("SubMenuButton"),
            ),
            SubmenuButton(
              menuStyle: const MenuStyle(
                visualDensity: VisualDensity.compact,
              ),
              alignmentOffset: const Offset(8, 0),
              menuChildren: [
                MenuItemButton(
                  leadingIcon: const Icon(Icons.people_alt_outlined),
                  child: const Text('Item 1'),
                  onPressed: () {},
                ),
                MenuItemButton(
                  leadingIcon: const Icon(Icons.remove_red_eye_outlined),
                  child: const Text('Item 2'),
                  onPressed: () {},
                ),
                MenuItemButton(
                  leadingIcon: const Icon(Icons.refresh),
                  onPressed: () {},
                  child: const Text('Item 3'),
                ),
              ],
              child: const Text("SubMenuButton"),
            ),
            SubmenuButton(
              menuStyle: const MenuStyle(
                visualDensity: VisualDensity.compact,
              ),
              alignmentOffset: const Offset(8, 0),
              menuChildren: [
                MenuItemButton(
                  leadingIcon: const Icon(Icons.people_alt_outlined),
                  child: const Text('Item 1'),
                  onPressed: () {},
                ),
                MenuItemButton(
                  leadingIcon: const Icon(Icons.remove_red_eye_outlined),
                  child: const Text('Item 2'),
                  onPressed: () {},
                ),
                MenuItemButton(
                  leadingIcon: const Icon(Icons.refresh),
                  onPressed: () {},
                  child: const Text('Item 3'),
                ),
              ],
              child: const Text("SubMenuButton"),
            ),
            SubmenuButton(
              menuStyle: const MenuStyle(
                visualDensity: VisualDensity.compact,
              ),
              alignmentOffset: const Offset(8, 0),
              menuChildren: [
                MenuItemButton(
                  leadingIcon: const Icon(Icons.people_alt_outlined),
                  child: const Text('Item 1'),
                  onPressed: () {},
                ),
                MenuItemButton(
                  leadingIcon: const Icon(Icons.remove_red_eye_outlined),
                  child: const Text('Item 2'),
                  onPressed: () {},
                ),
                SubmenuButton(
                  menuStyle: const MenuStyle(
                    visualDensity: VisualDensity.compact,
                  ),
                  alignmentOffset: const Offset(8, 0),
                  menuChildren: [
                    MenuItemButton(
                      leadingIcon: const Icon(Icons.people_alt_outlined),
                      child: const Text('Item 1'),
                      onPressed: () {},
                    ),
                    MenuItemButton(
                      leadingIcon: const Icon(Icons.remove_red_eye_outlined),
                      child: const Text('Item 2'),
                      onPressed: () {},
                    ),
                    MenuItemButton(
                      leadingIcon: const Icon(Icons.refresh),
                      onPressed: () {},
                      child: const Text('Item 3'),
                    ),
                  ],
                  child: const Text("SubMenuButton"),
                ),
                SubmenuButton(
                  menuStyle: const MenuStyle(
                    visualDensity: VisualDensity.compact,
                  ),
                  alignmentOffset: const Offset(8, 0),
                  menuChildren: [
                    MenuItemButton(
                      leadingIcon: const Icon(Icons.people_alt_outlined),
                      child: const Text('Item 1'),
                      onPressed: () {},
                    ),
                    MenuItemButton(
                      leadingIcon: const Icon(Icons.remove_red_eye_outlined),
                      child: const Text('Item 2'),
                      onPressed: () {},
                    ),
                    MenuItemButton(
                      leadingIcon: const Icon(Icons.refresh),
                      onPressed: () {},
                      child: const Text('Item 3'),
                    ),
                  ],
                  child: const Text("SubMenuButton"),
                ),
                SubmenuButton(
                  menuStyle: const MenuStyle(
                    visualDensity: VisualDensity.compact,
                  ),
                  alignmentOffset: const Offset(8, 0),
                  menuChildren: [
                    MenuItemButton(
                      leadingIcon: const Icon(Icons.people_alt_outlined),
                      child: const Text('Item 1'),
                      onPressed: () {},
                    ),
                    MenuItemButton(
                      leadingIcon: const Icon(Icons.remove_red_eye_outlined),
                      child: const Text('Item 2'),
                      onPressed: () {},
                    ),
                    MenuItemButton(
                      leadingIcon: const Icon(Icons.refresh),
                      onPressed: () {},
                      child: const Text('Item 3'),
                    ),
                  ],
                  child: const Text("SubMenuButton"),
                ),
                SubmenuButton(
                  menuStyle: const MenuStyle(
                    visualDensity: VisualDensity.compact,
                  ),
                  alignmentOffset: const Offset(8, 0),
                  menuChildren: [
                    MenuItemButton(
                      leadingIcon: const Icon(Icons.people_alt_outlined),
                      child: const Text('Item 1'),
                      onPressed: () {},
                    ),
                    SubmenuButton(
                      menuStyle: const MenuStyle(
                        visualDensity: VisualDensity.compact,
                      ),
                      alignmentOffset: const Offset(8, 0),
                      menuChildren: [
                        MenuItemButton(
                          leadingIcon: const Icon(Icons.people_alt_outlined),
                          child: const Text('Item 1'),
                          onPressed: () {},
                        ),
                        MenuItemButton(
                          leadingIcon:
                          const Icon(Icons.remove_red_eye_outlined),
                          child: const Text('Item 2'),
                          onPressed: () {},
                        ),
                        MenuItemButton(
                          leadingIcon: const Icon(Icons.refresh),
                          onPressed: () {},
                          child: const Text('Item 3'),
                        ),
                      ],
                      child: const Text("SubMenuButton"),
                    ),
                    SubmenuButton(
                      menuStyle: const MenuStyle(
                        visualDensity: VisualDensity.compact,
                      ),
                      alignmentOffset: const Offset(8, 0),
                      menuChildren: [
                        MenuItemButton(
                          leadingIcon: const Icon(Icons.people_alt_outlined),
                          child: const Text('Item 1'),
                          onPressed: () {},
                        ),
                        MenuItemButton(
                          leadingIcon:
                          const Icon(Icons.remove_red_eye_outlined),
                          child: const Text('Item 2'),
                          onPressed: () {},
                        ),
                        MenuItemButton(
                          leadingIcon: const Icon(Icons.refresh),
                          onPressed: () {},
                          child: const Text('Item 3'),
                        ),
                      ],
                      child: const Text("SubMenuButton"),
                    ),
                    MenuItemButton(
                      leadingIcon: const Icon(Icons.remove_red_eye_outlined),
                      child: const Text('Item 2'),
                      onPressed: () {},
                    ),
                    MenuItemButton(
                      leadingIcon: const Icon(Icons.refresh),
                      onPressed: () {},
                      child: const Text('Item 3'),
                    ),
                  ],
                  child: const Text("SubMenuButton"),
                ),
                MenuItemButton(
                  leadingIcon: const Icon(Icons.refresh),
                  onPressed: () {},
                  child: const Text('Item 3'),
                ),
              ],
              child: const Text("SubMenuButton"),
            ),
            MenuItemButton(
              leadingIcon: const Icon(Icons.people_alt_outlined),
              child: const Text('Item 1'),
              onPressed: () {},
            ),
            MenuItemButton(
              leadingIcon: const Icon(Icons.remove_red_eye_outlined),
              child: const Text('Item 2'),
              onPressed: () {},
            ),
            MenuItemButton(
              leadingIcon: const Icon(Icons.refresh),
              onPressed: () {},
              child: const Text('Item 3'),
            ),
          ],
          child: const Text("SubMenuButton"),
        ),
        SubmenuButton(
          menuStyle: const MenuStyle(
            visualDensity: VisualDensity.compact,
          ),
          alignmentOffset: const Offset(8, 0),
          menuChildren: [
            MenuItemButton(
              leadingIcon: const Icon(Icons.people_alt_outlined),
              child: const Text('Item 1'),
              onPressed: () {},
            ),
            MenuItemButton(
              leadingIcon: const Icon(Icons.remove_red_eye_outlined),
              child: const Text('Item 2'),
              onPressed: () {},
            ),
            MenuItemButton(
              leadingIcon: const Icon(Icons.refresh),
              onPressed: () {},
              child: const Text('Item 3'),
            ),
          ],
          child: const Text("SubMenuButton"),
        ),
        SubmenuButton(
          menuStyle: const MenuStyle(
            visualDensity: VisualDensity.compact,
          ),
          alignmentOffset: const Offset(8, 0),
          menuChildren: [
            MenuItemButton(
              leadingIcon: const Icon(Icons.people_alt_outlined),
              child: const Text('Item 1'),
              onPressed: () {},
            ),
            MenuItemButton(
              leadingIcon: const Icon(Icons.remove_red_eye_outlined),
              child: const Text('Item 2'),
              onPressed: () {},
            ),
            MenuItemButton(
              leadingIcon: const Icon(Icons.refresh),
              onPressed: () {},
              child: const Text('Item 3'),
            ),
            SubmenuButton(
              menuStyle: const MenuStyle(
                visualDensity: VisualDensity.compact,
              ),
              alignmentOffset: const Offset(8, 0),
              menuChildren: [
                MenuItemButton(
                  leadingIcon: const Icon(Icons.people_alt_outlined),
                  child: const Text('Item 1'),
                  onPressed: () {},
                ),
                MenuItemButton(
                  leadingIcon: const Icon(Icons.remove_red_eye_outlined),
                  child: const Text('Item 2'),
                  onPressed: () {},
                ),
                MenuItemButton(
                  leadingIcon: const Icon(Icons.refresh),
                  onPressed: () {},
                  child: const Text('Item 3'),
                ),
              ],
              child: const Text("SubMenuButton"),
            ),
            SubmenuButton(
              menuStyle: const MenuStyle(
                visualDensity: VisualDensity.compact,
              ),
              alignmentOffset: const Offset(8, 0),
              menuChildren: [
                MenuItemButton(
                  leadingIcon: const Icon(Icons.people_alt_outlined),
                  child: const Text('Item 1'),
                  onPressed: () {},
                ),
                MenuItemButton(
                  leadingIcon: const Icon(Icons.remove_red_eye_outlined),
                  child: const Text('Item 2'),
                  onPressed: () {},
                ),
                MenuItemButton(
                  leadingIcon: const Icon(Icons.refresh),
                  onPressed: () {},
                  child: const Text('Item 3'),
                ),
              ],
              child: const Text("SubMenuButton"),
            ),
            SubmenuButton(
              menuStyle: const MenuStyle(
                visualDensity: VisualDensity.compact,
              ),
              alignmentOffset: const Offset(8, 0),
              menuChildren: [
                MenuItemButton(
                  leadingIcon: const Icon(Icons.people_alt_outlined),
                  child: const Text('Item 1'),
                  onPressed: () {},
                ),
                MenuItemButton(
                  leadingIcon: const Icon(Icons.remove_red_eye_outlined),
                  child: const Text('Item 2'),
                  onPressed: () {},
                ),
                MenuItemButton(
                  leadingIcon: const Icon(Icons.refresh),
                  onPressed: () {},
                  child: const Text('Item 3'),
                ),
              ],
              child: const Text("SubMenuButton"),
            ),
          ],
          child: const Text("SubMenuButton"),
        ),
        SubmenuButton(
          menuStyle: const MenuStyle(
            visualDensity: VisualDensity.compact,
          ),
          alignmentOffset: const Offset(8, 0),
          menuChildren: [
            MenuItemButton(
              leadingIcon: const Icon(Icons.people_alt_outlined),
              child: const Text('Item 1'),
              onPressed: () {},
            ),
            MenuItemButton(
              leadingIcon: const Icon(Icons.remove_red_eye_outlined),
              child: const Text('Item 2'),
              onPressed: () {},
            ),
            MenuItemButton(
              leadingIcon: const Icon(Icons.refresh),
              onPressed: () {},
              child: const Text('Item 3'),
            ),
          ],
          child: const Text("SubMenuButton"),
        ),
      ],
    );
  }
}

Fixes #122096

@flutter-dashboard flutter-dashboard bot added f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. labels Mar 30, 2023
@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@whiskeyPeak whiskeyPeak changed the title Add the vertical alignment offset to the MenuAnchor widget Add vertical alignment offset to the MenuAnchor widget when overflowing Mar 30, 2023
@whiskeyPeak
Copy link
Contributor Author

@gspencergoog @justinmc @TahaTesser This is ready for review :)

justinmc
justinmc approved these changes Mar 31, 2023
View reviewed changes
Copy link
Contributor

@justinmc justinmc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍 . Thanks again for the menu bug fix!

But @gspencergoog should also take a look before merge.

packages/flutter/lib/src/material/menu_anchor.dart Outdated
@@ -3156,7 +3156,12 @@ class _MenuLayout extends SingleChildLayoutDelegate {
} else if (offBottom(y)) {
final double newY = anchorRect.top - childSize.height;
if (!offTop(newY)) {
y = newY;
// Only move the menu up if it is the root menu
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Period at the end of the sentence here and in the comments in the test below.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

packages/flutter/test/material/menu_anchor_test.dart
await tester.tap(find.text('Tap me'));
await tester.pumpAndSettle();

await tester.pumpWidget(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I would typically split this into 2 separate tests. The main reason is better feedback when there is a failure in one of them. But it's up to you if you think that's useful here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When does it make sense to have one test have multiple tester.pumpWidget widgets? In a previous PR, @TahaTesser told me to place both the default and explicit test cases in a single test since the separate test would only test the default behaviour.

Refactored nonetheless.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TahaTesser told me to place both the default and explicit test cases in a single test since the separate test would only test the default behaviour.

That was the clip behavior test, which is a much simpler test. Just checking the clip property.

gspencergoog
gspencergoog approved these changes Apr 3, 2023
View reviewed changes
Copy link
Contributor

@gspencergoog gspencergoog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

32384589-a60f0e74-c078-11e7-9bc1-e5b5287aea9d

packages/flutter/lib/src/material/menu_anchor.dart Outdated
@@ -3177,7 +3177,12 @@ class _MenuLayout extends SingleChildLayoutDelegate {
} else if (offBottom(y)) {
final double newY = anchorRect.top - childSize.height;
if (!offTop(newY)) {
y = newY;
// Only move the menu up if it is the root menu.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment doesn't appear to be related to the conditional, since there can be menus that are horizontal that aren't root menus. Sure, it's weird, but it's possible (you could have a menu bar that appears when you open another menu item).

Probably you just want to adjust the comment to talk about horizontal menus, but if you did want to actually only do it if it's the root menu, you would have to pass that information in to this layout widget.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, updated the comment. I tested the behaviour of having a MenuBar as a child of a SubmenuButton in a MenuAnchor widget and it's behaving as it should.

@justinmc
Copy link
Contributor

justinmc commented Apr 5, 2023 •
edited

Google testing infrastructure is currently broken. We'll have to wait for a fix for now.

@justinmc justinmc merged commit 73996ea into flutter:master Apr 6, 2023
72 checks passed
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Apr 7, 2023
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 7, 2023
@engine-flutter-autoroll
73996ea40 Add vertical alignment offset to the MenuAnchor widge…
3f28cf7 
…t when overflowing (flutter/flutter#123740)
@HansMuller HansMuller mentioned this pull request Apr 7, 2023
Open
exaby73 pushed a commit to NevercodeHQ/flutter that referenced this pull request Apr 17, 2023
@whiskeyPeak @exaby73
Add vertical alignment offset to the MenuAnchor widget when ove…
31be495 
…rflowing (flutter#123740)

Positioning of cascading menus.
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023
@engine-flutter-autoroll
73996ea40 Add vertical alignment offset to the MenuAnchor widge…
85704a8 
…t when overflowing (flutter/flutter#123740)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023
@engine-flutter-autoroll
73996ea40 Add vertical alignment offset to the MenuAnchor widge…
b59f5b2 
…t when overflowing (flutter/flutter#123740)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TahaTesser TahaTesser TahaTesser left review comments

@justinmc justinmc justinmc approved these changes

@gspencergoog gspencergoog gspencergoog approved these changes

Assignees
No one assigned
Labels
f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

MenuAnchor alignmentOffset is not calculated correct when direction is reversed
4 participants
@whiskeyPeak @justinmc @gspencergoog @TahaTesser