Add check to only open flyout if a menubaritem has items

dev/MenuBar/MenuBarItem.cpp

@@ -214,29 +214,32 @@ void MenuBarItem::OnMenuBarItemAccessKeyInvoked(winrt::IInspectable const& sende
 // Menu Flyout actions
 void MenuBarItem::ShowMenuFlyout()
 {
-    if (auto button = m_button.get())
+    if (Items().Size() != 0)
     {
-        const auto width = static_cast<float>(button.ActualWidth());
-        const auto height = static_cast<float>(button.ActualHeight());
-
-        if (SharedHelpers::IsFlyoutShowOptionsAvailable())
-        {
-            // Sets an exclusion rect over the button that generates the flyout so that even if the menu opens upwards
-            // (which is the default in touch mode) it doesn't cover the menu bar button.
-            winrt::FlyoutShowOptions options{};
-            options.Position(winrt::Point(0, height));
-            options.Placement(winrt::FlyoutPlacementMode::Bottom);
-            options.ExclusionRect(winrt::Rect(0, 0, width, height));
-            m_flyout.get().ShowAt(button, options);
-        }
-        else
+        if (auto button = m_button.get())
         {
-            m_flyout.get().ShowAt(button, winrt::Point(0, height));
+            const auto width = static_cast<float>(button.ActualWidth());
+            const auto height = static_cast<float>(button.ActualHeight());
+
+            if (SharedHelpers::IsFlyoutShowOptionsAvailable())
+            {
+                // Sets an exclusion rect over the button that generates the flyout so that even if the menu opens upwards
+                // (which is the default in touch mode) it doesn't cover the menu bar button.
+                winrt::FlyoutShowOptions options{};
+                options.Position(winrt::Point(0, height));
+                options.Placement(winrt::FlyoutPlacementMode::Bottom);
+                options.ExclusionRect(winrt::Rect(0, 0, width, height));
+                m_flyout.get().ShowAt(button, options);
+            }
+            else
+            {
+                m_flyout.get().ShowAt(button, winrt::Point(0, height));
+            }
+
+            // Attach keyboard event handler
+            auto presenter = winrt::get_self<MenuBarItemFlyout>(m_flyout.get())->m_presenter.get();
+            m_presenterKeyDownRevoker = presenter.KeyDown(winrt::auto_revoke, { this,  &MenuBarItem::OnPresenterKeyDown });
         }
-
-        // Attach keyboard event handler
-        auto presenter = winrt::get_self<MenuBarItemFlyout>(m_flyout.get())->m_presenter.get();
-        m_presenterKeyDownRevoker = presenter.KeyDown(winrt::auto_revoke, { this,  &MenuBarItem::OnPresenterKeyDown });
     }
 }
 

dev/MenuBar/MenuBar_InteractionTests/MenuBarTests.cs

@@ -375,6 +375,30 @@ public void TabTest()
             }
         }
 
+        [TestMethod]
+        public void EmptyMenuBarItemNoPopupTest()
+        {
+            if (PlatformConfiguration.IsDevice(DeviceType.Phone))
+            {
+                Log.Comment("Skipping tests on phone, because menubar is not supported.");
+                return;
+            }
+            using (var setup = new TestSetupHelper("MenuBar Tests"))
+            {
+                FindElement.ByName<Button>("NoChildrenFlyout").Click();
+                VerifyElement.NotFound("Popup",FindBy.Name);
+
+                FindElement.ByName<Button>("OneChildrenFlyout").Click();
+                VerifyElement.Found("Popup", FindBy.Name);
+
+                // Click twice to close flyout
+                FindElement.ByName<Button>("RemoveItemsFromOneChildrenItem").Click();
+                FindElement.ByName<Button>("RemoveItemsFromOneChildrenItem").Click();
+
+                FindElement.ByName<Button>("OneChildrenFlyout").Click();
+                VerifyElement.NotFound("Popup", FindBy.Name);
+            }
+        }
 
         private T GetElement<T>(ref T element, string elementName) where T : UIObject
         {

dev/MenuBar/MenuBar_TestUI/MenuBarPage.xaml

@@ -55,6 +55,11 @@
                     <MenuFlyoutItem Text="Word Wrap"/>
                     <MenuFlyoutItem Text="Font..."/>
                 </muxc:MenuBarItem>
+
+                <muxc:MenuBarItem Title="No children" AutomationProperties.Name="NoChildrenFlyout"/>
+                <muxc:MenuBarItem Title="One children" AutomationProperties.Name="OneChildrenFlyout" x:Name="OneChildrenFlyoutMenuBarItem">
+                    <MenuFlyoutItem Text="Sometext"/>
+                </muxc:MenuBarItem>
             </muxc:MenuBar>
 
             <TextBlock Text="Custom sized menu bar" Margin="0,6,0,0"/>
@@ -78,6 +83,7 @@
             <Button x:Name="AddFlyoutItemButton" AutomationProperties.Name="AddFlyoutItemButton" Click="AddFlyoutItem_Click" Content="Add Flyout Item"/>
             <Button x:Name="RemoveFlyoutItemButton" AutomationProperties.Name="RemoveFlyoutItemButton" Click="RemoveFlyoutItem_Click" Content="Remove Flyout Item"/>
             <Button x:Name="AddItemsToEmptyMenuBar" AutomationProperties.Name="AddItemsToEmptyMenuBar" Click="AddMenuBarToEmptyMenuBarItem_Click" Content="Add a menu item to the empty menu bar"/>
+            <Button x:Name="RemoveItemsFromOneChildrenItem" AutomationProperties.Name="RemoveItemsFromOneChildrenItem" Click="RemoveItemsFromOneChildrenItem_Click" Content="Remove items from One Children item"/>
         </StackPanel>
         <StackPanel Grid.Row="3">
             <TextBlock Text="Test output" Style="{ThemeResource StandardGroupHeader}"/>

dev/MenuBar/MenuBar_TestUI/MenuBarPage.xaml.cs

@@ -108,5 +108,9 @@ private void AddMenuBarToEmptyMenuBarItem_Click(object sender, RoutedEventArgs e
             EmptyMenuBar.Items.Add(mainMenuBarHelp);
         }
 
+        private void RemoveItemsFromOneChildrenItem_Click(object sender, RoutedEventArgs e)
+        {
+            OneChildrenFlyoutMenuBarItem.Items.Clear();
+        }
     }
 }