CommandBarFlyout: Submenu persists when CommandBar closes

[marcelwgn]

Describe the bug

When closing a CommandBarFlyout while a submenu is open that belongs the the CommandBarFlyout, the SubMenu is still visible.
Steps to reproduce the bug

Steps to reproduce the behavior:

1. Open MUXControlsTestApp
2. Go to "Base CommandBarFlyout Tests"
3. Press "Show CommandBarFlyout with sub-menu" and open the Submenu
4. Click outside the COmmandBarFlyout and submenu

Expected behavior

The submenu disappears with the CommandBarFlyout.
Screenshots

Version Info

NuGet package version:

Latest commit on master.

Windows 10 version	Saw the problem?
Insider Build (19536)	Yes
May 2019 Update (18362)
October 2018 Update (17763)
April 2018 Update (17134)
Fall Creators Update (16299)
Creators Update (15063)

Device form factor	Saw the problem?
Desktop	Yes
Mobile
Xbox
Surface Hub
IoT

Additional context

[ranjeshj]

@llongley can you please investigate ?

[marcelwgn]

@llongley Are you currently looking into this? Or would you be fine with me looking into this? @ranjeshj FYI.

Also looking at the code for the page, it seems that the CommandBarFlyout is not the owner of the popup, but rather the AppBarButton is. Should we add some code in the closed event to iterate over all the primary and secondary commands, and check if those are buttons, and hide the flyout if they have one?

[ranjeshj]

@chingucoding go for it. Is this an issue with the control or the app code ? I would not have expected to write app code to close the submenu items.

[marcelwgn]

I think it's a grey area here. On the one hand, the CommandBarFlyout is not really aware of the flyout as the secondary commands open it, which in theory don't have to be a button, they could be anything that opens some form of flyout through the Button.Flyout property. In that case we literally have no idea how to close existing flyouts without walking the visual tree.

In the case of Button.Flyout, we can iterate over all the commands, check if they are a button, and if they are and they have a flyout, close the flyouts.

[llongley]

Hey, thanks for offering to take this on!

One interesting thing I should note is that if you uncheck "Long animations disabled" in the test app's main page, the issue stops reproducing, at least for me - in that case, the MenuFlyout closes when the CommandBarFlyout closes. So there may be an issue where we're assuming that an animation will complete somewhere before we close the child flyout. We should definitely in general be closing child flyouts when the parent closes.

[marcelwgn]

That's a very good observation. As it turns out, when we disable long animations, this code does not get called:

microsoft-ui-xaml/dev/CommandBarFlyout/CommandBarFlyout.cpp

Lines 158 to 165 in 2ec9b1c

	commandBar->PlayCloseAnimation(
	[this]()
	{
	m_isClosingAfterCloseAnimation = true;
	Hide();
	m_isClosingAfterCloseAnimation = false;
	});
	commandBar->IsOpen(false);

Adding an commandBar->IsOpen(false); as an else case fixes the issue. I'll create a PR for this soon.