Done!

nit: Will a name like requestChildrenClose be clearer and more recognizable from closeChildren? I'm fine with the current name as well, just providing another option.

I went with closeChildrenWithRequest to indicate to the future editors that they should pay attention to the differentiator between the two functions (i.e. they both closeChildren, so users should pay attention to what makes one a request). That said, I was between those two. Your suggestion is more accurate since calling requestChildrenClose doesn't guarantee the children will close. Let me know what you prefer and I'll add it.

+1 to @dkwingsmt , I think we should prioritize readaibiity here

Done

Just to make sure, do we have a test that verifies that the root's onClose is called after the entire subtree is closed in an async way?

Like onCloseRequest() -> wait 1 second -> hideMenu()? I'll add it in if we don't have one. The dismiss test I refactored did test for async but was so convoluted that I decided to simplify it down to checking that the dismiss occurred.

It was just that, while reading the code, I wondered how onClose was invoked, which slightly concerned me if not tested since the flow is full of async (and I didn't want to read everything to verify that :) )

I added an additional test to validate that async closures occur as expected. There's actually no async code (where async is code involving futures) in RawMenuAnchor, but it can be used in an async way.

That said, done!

+1 to @dkwingsmt , I think we should prioritize readaibiity here

I know this is not introduced in this pr, why would we want to do async close here? it feels weird if the new menu opens before old one close completely

Now that you mentioned, I tested on macOS's system menu bar, and noticed that, while top menus have a tiny fade out animation, there is no fade out when switching between menus. I'm not sure if this applies to all other menus in other design languages (i.e. whether we should leave this as a flag or callback for design languages to implement), but the async closure here should indeed be wrong.

Nvm, I just noticed that this place is for repositioning menus, and I think should probably have no animations at all.

Some menus do an async close. For example, Google Drive:

I see. I guess we should somehow allow themed widget to customize this.

Edit: Actually, I think this is possible with my suggestion in #182357 (comment), which will allow submenus to know the cause of the closure, and decide the animation for themselves.

(Although I wonder a clearer way to explicitly tell this is possible, or needed at all)

😿 It does make things a bit more complicated, but it is probably for the best.

Also to be clear I mean that we should use closeChildrenWithRequest here since this allows children to use animation. They can choose to make the animation none.

add a see also to reference closeChildrenWithRequest and vice versa

Done

I think we should call this after widget.onCloseRequested (i.e. the if clause below). The onCloseRequested callback is where themed menus change the animation status, and calling closeChildrenWithRequest later allows submenus to read the animation status and know what triggers the closure, and hence decide whether to display closing animation, which helps implementing the behavior here https://github.com/flutter/flutter/pull/182357/changes#r2824951515 .

I think this is reasonable. My only concern is that closeChildren begins at the topmost menu and travels to underlying menus. This would begin closing from the bottom menu and travel to the topmost menu. I'm not sure what implications this may have.

The full closing sequence is:

• Request propagates in (parent.requestClose, child.requestClose, grandchild.requestClose, ...)
• Close propagates out (..., grandchild.close, child.close, parent.close).

It makes perfect sense to me. What do you think?

I think it makes sense... it doesn't seem to break anything so that's a good sign. I pushed the update implementing this.

Given the current content, I think we should group the trigger timing of the callback together. Something like this: (you might want to rephrase it to reduce repetitiveness)

  /// This callback can be used to add a delay or a closing animation before the menu
  /// is hidden.
  ///
  /// This callback is triggered every time [MenuController.close] is called,
  /// regardless of whether the overlay is already hidden. 
  /// This callback is also triggered a sibling [RawMenuAnchor] is opened while this menu is open
  /// This callback will also be called when the root
  /// menu anchor is scrolled and when the screen is resized while this menu anchor is open.

And I suggest the following sentence be moved after the next paragraph, since the next paragraph mentions asynchronous operation (i.e. timer).

  /// Pending timers or animations previously started in a previous call to
  /// [onCloseRequested] should be canceled when this callback is triggered to
  /// prevent them from closing the menu at an unintended time.

/// Called when a request is made to close the menu.
///
/// This callback can be used to add a delay or a closing animation before the
/// menu is hidden.
///
/// This callback is triggered every time [MenuController.close] is called
/// while this menu is open.
///
/// This callback is also triggered when a sibling [RawMenuAnchor] is opened
/// while this menu is open. In this case, the callback can be used to add a
/// delay or a closing animation while the sibling menu opens. When
/// implementing this behavior, the closing menu should immediately stop being
/// interactive, so that it doesn't interfere with the opening sibling menu.
/// Semantics, focus, and hit testing for the closing menu should be disabled
/// for the duration of the closing animation.
///
/// Pending timers or animations started in a previous call to
/// [onCloseRequested] should be canceled when this callback is triggered to
/// prevent them from closing the menu at an unintended time.
///
/// If the menu is not closed, this callback will also be called when the root
/// menu anchor is scrolled and when the screen is resized.
///
/// After a close request is intercepted and closing behaviors have completed,
/// the hideOverlay callback should be called. This callback sets
/// [MenuController.isOpen] to false and hides the menu overlay widget. If the
/// [RawMenuAnchor] is used in a themed menu that plays a closing animation,
/// hideOverlay should be called after the closing animation has ended,
/// since the animation plays on the overlay itself. This means that
/// [MenuController.isOpen] will stay true while closing animations are
/// running.
///
/// Calling hideOverlay after disposal is a no-op, meaning it will not
/// trigger [onClose] or hide the menu overlay.
///
/// Typically, [onCloseRequested] consists of the following steps:
///
/// 1. Optionally start the closing animation and wait for it to complete.
/// 2. Call hideOverlay (whose call chain eventually invokes [onClose]).
///
/// Throughout the closing sequence, menus should typically not be focusable
/// or interactive.
///
/// Defaults to a callback that immediately hides the menu.