One thing that isn't clear to me (I probably just missed it in the 8000+ lines 😝): Why do we still need the button rect to position the menu? I thought the LayerLink stuff would take care of most of that...

We still need the button rect because the custom layout needs to be able to position the menu by looking at the global coordinates of the menu button: it needs to know if the menu is too close to the edge of the view or not. The LayerLink takes care of when the view is resized or scrolled after it is positioned, but it doesn't know if the menu is near an edge or not. If you have a menu up and scroll and get the menu too close to an edge, it won't relocate the menu (it'll just scroll off the edge), but that's an edge case that we probably don't need to worry about.

We still need the button rect because the custom layout needs to be able to position the menu by looking at the global coordinates of the menu button: it needs to know if the menu is too close to the edge of the view or not. The LayerLink takes care of when the view is resized or scrolled after it is positioned, but it doesn't know if the menu is near an edge or not. If you have a menu up and scroll and get the menu too close to an edge, it won't relocate the menu (it'll just scroll off the edge), but that's an edge case that we probably don't need to worry about.

Can you document this (and the limitation you describe) in the code somewhere? This may be useful for a future reader.

Just as I stamped this LGTM I actually had one more idea that may improve the API and may make it easier to integrate this with OverlayPortal and/or multi window in the future. I think it would do away with MenuHandle and createMaterialMenu altogether and would avoid passing contexts around. Rough API sketch:

final MenuController controller = MenuController();

@override
Widget build(BuildContext context) {
  return MenuAnchor(
    controller: controller,
    // ... other config options that would have been passed to createMaterialMenu before
    menuBuilder: (BuildContext context) {
      return <Widget>[
        MenuItemButton(...),
        SubMenuButton(...),
        // ...
      ],
    },
    child: TextButton(
     onPressed: () {
       if (!controller.isOpen) {
         // maybe open needs to take an optional offset param to allow specifying where within the MenuAnchor's bounding box the menu should be shown...
         controller.open();
       } else {
         controller.close();
       }
     },
     child: Text('Toggle menu'),
    )
  );
}

Basically, the menu is always anchored at the MenuAnchor. It's context is used for positioning (so you want to wrap it tightly e.g. around a button or whatever UI element is supposed to be aligned with the menu). Now the widgets that make up the menu entries actually look like they are part of the widget tree and when we move menues over to OverlayPortal or multi window it is immediately obvious that they can access all inherited widgets above the MenuAnchor, because that's where they are anchored.

Also, I think this will make it easier to move to OverlayPortal later (and multi window as well) because for both you kinda need an anchor in the widget tree.

It also simplifies the API by removing concepts like MenuHandle and createMaterialMenu from the public API.

This is similar to how Tooltips already work today (except that you get a controller to decide when to bring up the menu) and parallels the API we are designing for OverlayPortal and multi window.

Curious to hear what you think...

Curious to hear what you think...

This might make it easier to integrate, but it disables one of the use cases that we can currently allow: creating context menus that are placed with a global position (often from a pointer position, not a widget position), which is what the create_material_menu.1.dart example shows.

Also, unless you need the global positioning use case, we already have most of what you're talking about: you don't need to use createMaterialMenu or a MenuAnchor if you use MenuBar or SubmenuButton directly. The only time you need the lower level API is if you want to have a menu on something other than a plain button, or want to specify the opening location yourself.

However, we might be able to change the MenuAnchor so that it has an API closer to what you propose, because I do like that the menu description appears to be part of the widget tree in the same way as MenuBar. I worry a little that there will be ways in which it won't act the same, and thus be surprising, since it's actually drawn in the Overlay, at least until we get OverlayPortal (I'm mainly thinking of cases where they begin invalidating the widgets at the wrong time in the build).

If we eliminate createMaterialMenu and move its attributes to MenuAnchor, but still allow an optional global position to be specified by the open(), maybe that is sufficient?

I am a bit sad about not committing this yet...but I also don't want to remove API mere days after introducing it.

This might make it easier to integrate, but it disables one of the use cases that we can currently allow: creating context menus that are placed with a global position (often from a pointer position, not a widget position), which is what the create_material_menu.1.dart example shows.

For that my thinking was that you'd wrap the entire area in which you want the context menu to appear in the MenuAnchor and then the .open call could take an optional offset into the boundingbox defined by the MenuAnchor to position it based on a PointerEvent.

Michael and I talked about this, and I'm going to give it a try and see if I can refine the API surface to match his idea more closely.

Closing in favor of #112239 since GitHub is just getting too slow to be able to handle this PR with more comments. Moving to a new PR in the hope that it'll be faster without the history, and also to preserve this iteration.