From b29a29b8b626fcd0888f17981e92341dfada8155 Mon Sep 17 00:00:00 2001
From: Karen Lai <7976322+karenbtlai@users.noreply.github.com>
Date: Tue, 9 Nov 2021 11:40:06 -0800
Subject: [PATCH] Exclude MoreButton if Not using PrimaryCommands (#6044)

* exclude MoreButton if not using primaryCommands

* workaround for commandBarFlyout no primary keyboard focus

* new test wip

* add new test

* add new test case

* fix failing rs2 test

* fix failing tests

* update tests

* set up test for dynamic primary commands scenario

* add dynamic add primary command test
---
 .../CommandBarFlyoutCommandBar.cpp            | 13 ++-
 .../InteractionTests/CommandBarFlyoutTests.cs | 92 +++++++++++++++++++
 .../TestUI/CommandBarFlyoutPage.xaml          | 10 +-
 .../TestUI/CommandBarFlyoutPage.xaml.cs       | 86 ++++++++++++++++-
 4 files changed, 195 insertions(+), 6 deletions(-)

diff --git a/dev/CommandBarFlyout/CommandBarFlyoutCommandBar.cpp b/dev/CommandBarFlyout/CommandBarFlyoutCommandBar.cpp
index ca1a8fc02b..fae82e0e3f 100644
--- a/dev/CommandBarFlyout/CommandBarFlyoutCommandBar.cpp
+++ b/dev/CommandBarFlyout/CommandBarFlyoutCommandBar.cpp
@@ -678,7 +678,7 @@ void CommandBarFlyoutCommandBar::UpdateTemplateSettings()
             collapsedWidth = static_cast<float>(expandedWidth);
         }
 
-        flyoutTemplateSettings->WidthExpansionDelta(collapsedWidth - expandedWidth);
+        flyoutTemplateSettings->WidthExpansionDelta(0);
         flyoutTemplateSettings->WidthExpansionAnimationStartPosition(-flyoutTemplateSettings->WidthExpansionDelta() / 2.0);
         flyoutTemplateSettings->WidthExpansionAnimationEndPosition(-flyoutTemplateSettings->WidthExpansionDelta());
         flyoutTemplateSettings->ContentClipRect({ 0, 0, static_cast<float>(expandedWidth), primaryItemsRootDesiredSize.Height });
@@ -896,7 +896,9 @@ void CommandBarFlyoutCommandBar::PopulateAccessibleControls()
         m_verticallyAccessibleControls.Clear();
     }
 
-    for (winrt::ICommandBarElement const& command : PrimaryCommands())
+    const auto primaryCommands = PrimaryCommands();
+
+    for (winrt::ICommandBarElement const& command : primaryCommands)
     {
         if (auto const& commandAsControl = command.try_as<winrt::Control>())
         {
@@ -907,8 +909,11 @@ void CommandBarFlyoutCommandBar::PopulateAccessibleControls()
 
     if (auto const& moreButton = m_moreButton.get())
     {
-        m_horizontallyAccessibleControls.Append(moreButton);
-        m_verticallyAccessibleControls.Append(moreButton);
+        if (primaryCommands.Size() > 0)
+        {
+            m_horizontallyAccessibleControls.Append(moreButton);
+            m_verticallyAccessibleControls.Append(moreButton);
+        }
     }
 
     for (winrt::ICommandBarElement const& command : SecondaryCommands())
diff --git a/dev/CommandBarFlyout/InteractionTests/CommandBarFlyoutTests.cs b/dev/CommandBarFlyout/InteractionTests/CommandBarFlyoutTests.cs
index 3e1968f0a5..243818dc44 100644
--- a/dev/CommandBarFlyout/InteractionTests/CommandBarFlyoutTests.cs
+++ b/dev/CommandBarFlyout/InteractionTests/CommandBarFlyoutTests.cs
@@ -992,5 +992,97 @@ public void VerifyDynamicSecondaryCommandLabel()
                 Verify.AreEqual(finalBoundingRectangle.Height, initialBoundingRectangle.Height);
             }
         }
+
+        [TestMethod]
+        public void VerifyIsFlyoutKeyboardAccessibleWithNoPrimaryCommands()
+        {
+            if (PlatformConfiguration.IsOSVersionLessThan(OSVersion.Redstone2))
+            {
+                Log.Warning("Test is disabled pre-RS2 because CommandBarFlyout is not supported pre-RS2");
+                return;
+            }
+
+            using (var setup = new CommandBarFlyoutTestSetupHelper())
+            {
+                Button showCommandBarFlyoutButtonWithNoPrimaryCommands = FindElement.ByName<Button>("Show CommandBarFlyout with no primary commands");
+
+                Log.Comment("Tap on a button to show the CommandBarFlyout.");
+                showCommandBarFlyoutButtonWithNoPrimaryCommands.InvokeAndWait();
+
+                Button undoButton6 = FindElement.ById<Button>("UndoButton6");
+                var undoButtonElement = AutomationElement.FocusedElement;
+                Verify.AreEqual(undoButtonElement.Current.AutomationId, undoButton6.AutomationId);
+                
+                KeyboardHelper.PressKey(Key.Up);
+                Wait.ForIdle();
+                
+                if (PlatformConfiguration.IsOSVersionLessThan(OSVersion.Redstone3))
+                {
+                    // rs2 does not loop through menuflyout items, so focus stays on the first button.
+                    Log.Comment("Press Up key to make sure it does not go to hidden controls.");
+                    Verify.AreEqual(undoButtonElement.Current.AutomationId, undoButton6.AutomationId);
+                }
+                else
+                {
+                    Log.Comment("Press Up key to make sure commands loops focus to last secondary command: Favorite.");
+                    Button favoriteToggleButton6 = FindElement.ById<Button>("FavoriteToggleButton6");
+                    var favoriteToggleElement = AutomationElement.FocusedElement;
+                    Verify.AreEqual(favoriteToggleElement.Current.AutomationId, favoriteToggleButton6.AutomationId);
+                }
+            }
+        }
+
+        [TestMethod]
+        public void VerifyAddPrimaryCommandsDynamically()
+        {
+            if (PlatformConfiguration.IsOSVersionLessThan(OSVersion.Redstone2))
+            {
+                Log.Warning("Test is disabled pre-RS2 because CommandBarFlyout is not supported pre-RS2");
+                return;
+            }
+
+            using (var setup = new CommandBarFlyoutTestSetupHelper())
+            {
+                Button showCommandBarFlyoutButton = FindElement.ByName<Button>("Show CommandBarFlyout with no primary commands");
+                ToggleButton addPrimaryCommandDynamicallyCheckBox = FindElement.ById<ToggleButton>("AddPrimaryCommandDynamicallyCheckBox");
+                ToggleButton clearPrimaryCommandsCheckBox = FindElement.ById<ToggleButton>("ClearPrimaryCommandsCheckBox");
+                ToggleButton primaryCommandDynamicallyAddedCheckBox = FindElement.ById<ToggleButton>("PrimaryCommandDynamicallyAddedCheckBox");
+                Edit dynamicLabelTimerIntervalTextBox = new Edit(FindElement.ById("DynamicLabelTimerIntervalTextBox"));
+                Edit dynamicLabelChangeCountTextBox = new Edit(FindElement.ById("DynamicLabelChangeCountTextBox"));
+
+                Log.Comment("Setting DynamicLabelTimerIntervalTextBox to 1s");
+                dynamicLabelTimerIntervalTextBox.SetValue("1000");
+
+                Log.Comment("Setting DynamicLabelChangeCountTextBox to 1 single change");
+                dynamicLabelChangeCountTextBox.SetValue("1");
+                Wait.ForIdle();
+
+                Log.Comment("Set Flyout6 to add Primary Commands dynamically");
+                addPrimaryCommandDynamicallyCheckBox.Check();
+                Wait.ForIdle();
+
+                Log.Comment("Invoke FlyoutTarget 6 to Show CommandBarFlyout with no primary commands");
+                showCommandBarFlyoutButton.Click();
+
+                Log.Comment("Waiting for SecondaryCommandDynamicLabelChangedCheckBox becoming checked indicating the asynchronous Label property change occurred");
+                primaryCommandDynamicallyAddedCheckBox.GetToggledWaiter().Wait();
+                Wait.ForIdle();
+
+                KeyboardHelper.PressKey(Key.Tab);
+                Wait.ForIdle();
+
+                KeyboardHelper.PressKey(Key.Right);
+                Wait.ForIdle();
+
+                Log.Comment("Verifying Primary Commands is added and MoreButton is actionable");
+
+                Button moreButton = FindElement.ById<Button>("MoreButton");
+                var moreButtonElement = AutomationElement.FocusedElement;
+                Verify.AreEqual(moreButtonElement.Current.AutomationId, moreButton.AutomationId);
+
+                Log.Comment("Dismissing flyout");
+                KeyboardHelper.PressKey(Key.Escape);
+            }
+        }
     }
 }
diff --git a/dev/CommandBarFlyout/TestUI/CommandBarFlyoutPage.xaml b/dev/CommandBarFlyout/TestUI/CommandBarFlyoutPage.xaml
index 854321d855..5799dd5ea0 100644
--- a/dev/CommandBarFlyout/TestUI/CommandBarFlyoutPage.xaml
+++ b/dev/CommandBarFlyout/TestUI/CommandBarFlyoutPage.xaml
@@ -209,12 +209,20 @@
                 <Button x:Name="FlyoutTarget3" Content="Show tall CommandBarFlyout" Margin="10" Click="OnFlyoutTarget3Click" />
                 <Button x:Name="FlyoutTarget4" Content="Show max-width CommandBarFlyout" Margin="10" Click="OnFlyoutTarget4Click" />
                 <Button x:Name="FlyoutTarget5" Content="Show CommandBarFlyout with sub-menu" Margin="10" Click="OnFlyoutTarget5Click" />
-                <Button x:Name="FlyoutTarget6" Content="Show CommandBarFlyout with no primary commands" Margin="10" Click="OnFlyoutTarget6Click" />
+                <StackPanel Orientation="Horizontal">
+                    <Button x:Name="FlyoutTarget6" Content="Show CommandBarFlyout with no primary commands" Margin="10" Click="OnFlyoutTarget6Click" />
+                    <Button x:Name="EditCommandCount6" Content="Add / Remove Primary Command" Click="OnEditCommandCount6Click" />
+                </StackPanel>
                 <Button x:Name="FlyoutTarget7" Content="Show CommandBarFlyout with non-focusable primary commands" Margin="10" Click="OnFlyoutTarget7Click" />
                 <Button x:Name="FlyoutTarget8" Content="Show CommandBarFlyout with no secondary commands" Margin="10" Click="OnFlyoutTarget8Click" />
                 <Button x:Name="FlyoutTarget9" Content="Show CommandBarFlyout with AlwaysExpanded" Margin="10" Click="OnFlyoutTarget9Click" />
                 <contract13Present:Button x:Name="FlyoutTarget10" Content="Show Windows.UI.Xaml.Controls.CommandBarFlyout" Margin="10" Click="OnFlyoutTarget10Click" />
                 <CheckBox x:Name="IsRTLCheckBox" Content="Is Page in RightToLeft FlowDirection?" AutomationProperties.AutomationId="IsRTLCheckBox" Margin="10,10,10,2" Checked="IsRTLCheckBox_Checked" Unchecked="IsRTLCheckBox_Unchecked" />
+                <StackPanel Orientation="Horizontal" Margin="10,0,10,2">
+                    <CheckBox x:Name="AddPrimaryCommandDynamicallyCheckBox" Content="Add Primary Commands Dynamically?" AutomationProperties.AutomationId="AddPrimaryCommandDynamicallyCheckBox" />
+                    <CheckBox x:Name="PrimaryCommandDynamicallyAddedCheckBox" Content="Primary Commands Added?" AutomationProperties.AutomationId="PrimaryCommandDynamicallyAddedCheckBox" Margin="5,0,0,0" />
+                </StackPanel>
+                <CheckBox x:Name="ClearPrimaryCommandsCheckBox" Content="Clear Primary Commands Asynchronously?" AutomationProperties.AutomationId="ClearPrimaryCommandsCheckBox" Margin="10,0,10,2" />
                 <StackPanel Orientation="Horizontal" Margin="10,0,10,2">
                     <CheckBox x:Name="UseSecondaryCommandDynamicLabelCheckBox" Content="Use Secondary Command with Dynamic Label?" AutomationProperties.AutomationId="UseSecondaryCommandDynamicLabelCheckBox" />
                     <CheckBox x:Name="SecondaryCommandDynamicLabelChangedCheckBox" Content="Label Changed?" AutomationProperties.AutomationId="SecondaryCommandDynamicLabelChangedCheckBox" Margin="5,0,0,0" />
diff --git a/dev/CommandBarFlyout/TestUI/CommandBarFlyoutPage.xaml.cs b/dev/CommandBarFlyout/TestUI/CommandBarFlyoutPage.xaml.cs
index 44f2279850..50e0f3583d 100644
--- a/dev/CommandBarFlyout/TestUI/CommandBarFlyoutPage.xaml.cs
+++ b/dev/CommandBarFlyout/TestUI/CommandBarFlyoutPage.xaml.cs
@@ -3,6 +3,7 @@
 
 using Common;
 using System;
+using System.Linq;
 using Windows.Foundation.Metadata;
 using Windows.System;
 using Windows.UI.Xaml;
@@ -20,8 +21,13 @@ public sealed partial class CommandBarFlyoutPage : TestPage
         private DispatcherTimer clearSecondaryCommandsTimer = new DispatcherTimer();
         private CommandBarFlyout clearSecondaryCommandsFlyout;
 
+        private DispatcherTimer clearPrimaryCommandsTimer = new DispatcherTimer();
+        private CommandBarFlyout clearPrimaryCommandsFlyout;
+
         private DispatcherTimer dynamicLabelTimer = new DispatcherTimer();
+        private DispatcherTimer dynamicCommandTimer = new DispatcherTimer();
         private AppBarButton dynamicLabelSecondaryCommand;
+        private CommandBarFlyout dynamicCommandBarFlyout;
         private string originalLabelSecondaryCommand;
         private int dynamicLabelChangeCount;
 
@@ -31,9 +37,14 @@ public CommandBarFlyoutPage()
 
             dynamicLabelTimer.Tick += DynamicLabelTimer_Tick;
 
+            dynamicCommandTimer.Tick += DynamicCommandTimer_Tick;
+
             clearSecondaryCommandsTimer.Interval = new TimeSpan(0, 0, 3 /*sec*/);
             clearSecondaryCommandsTimer.Tick += ClearSecondaryCommandsTimer_Tick;
 
+            clearSecondaryCommandsTimer.Interval = new TimeSpan(0, 0, 3 /*sec*/);
+            clearPrimaryCommandsTimer.Tick += ClearPrimaryCommandsTimer_Tick;
+
             if (ApiInformation.IsTypePresent("Windows.UI.Xaml.Input.KeyboardAccelerator"))
             {
                 UndoButton1.KeyboardAccelerators.Add(new KeyboardAccelerator() { Key = VirtualKey.Z, Modifiers = VirtualKeyModifiers.Control });
@@ -180,8 +191,10 @@ private void ShowFlyoutAt(FlyoutBase flyout, FrameworkElement targetElement, Fly
         {
             bool useSecondaryCommandDynamicLabel = (bool)UseSecondaryCommandDynamicLabelCheckBox.IsChecked;
             bool clearSecondaryCommands = (bool)ClearSecondaryCommandsCheckBox.IsChecked;
+            bool addPrimaryCommandDynamicallyCheckBox = (bool)AddPrimaryCommandDynamicallyCheckBox.IsChecked;
+            bool clearPrimaryCommands = (bool)ClearPrimaryCommandsCheckBox.IsChecked;
 
-            if (useSecondaryCommandDynamicLabel || clearSecondaryCommands)
+            if (useSecondaryCommandDynamicLabel || addPrimaryCommandDynamicallyCheckBox || clearSecondaryCommands || clearPrimaryCommands)
             {
                 CommandBarFlyout commandBarFlyout = flyout as CommandBarFlyout;
 
@@ -199,6 +212,17 @@ private void ShowFlyoutAt(FlyoutBase flyout, FrameworkElement targetElement, Fly
                             SetClearSecondaryCommandsFlyout(commandBarFlyout);
                         }
                     }
+
+                    if (addPrimaryCommandDynamicallyCheckBox)
+                    {
+                        dynamicCommandBarFlyout = commandBarFlyout;
+                        SetDynamicPrimaryCommand();
+                    }
+
+                    if (clearPrimaryCommands && commandBarFlyout.PrimaryCommands != null && commandBarFlyout.PrimaryCommands.Count > 0)
+                    {
+                        SetClearPrimaryCommandsFlyout(commandBarFlyout);
+                    }
                 }
             }
 
@@ -227,6 +251,20 @@ private void SetClearSecondaryCommandsFlyout(CommandBarFlyout commandBarFlyout)
             }
         }
 
+        private void SetClearPrimaryCommandsFlyout(CommandBarFlyout commandBarFlyout)
+        {
+            if (commandBarFlyout == null)
+            {
+                clearPrimaryCommandsFlyout = null;
+                clearPrimaryCommandsTimer.Stop();
+            }
+            else
+            {
+                clearPrimaryCommandsFlyout = commandBarFlyout;
+                clearPrimaryCommandsTimer.Start();
+            }
+        }
+
         private void SetDynamicSecondaryCommand(AppBarButton appBarButton)
         {
             if (appBarButton == null)
@@ -255,6 +293,13 @@ private void SetDynamicSecondaryCommand(AppBarButton appBarButton)
             }
         }
 
+        private void SetDynamicPrimaryCommand()
+        {
+            dynamicCommandTimer.Interval = new TimeSpan(0, 0, 0, 0, int.Parse(DynamicLabelTimerIntervalTextBox.Text) /*msec*/);
+            dynamicLabelChangeCount = int.Parse(DynamicLabelChangeCountTextBox.Text);
+            dynamicCommandTimer.Start();
+        }
+
         private void ClearSecondaryCommandsTimer_Tick(object sender, object e)
         {
             if (clearSecondaryCommandsFlyout != null)
@@ -263,6 +308,14 @@ private void ClearSecondaryCommandsTimer_Tick(object sender, object e)
             }
         }
 
+        private void ClearPrimaryCommandsTimer_Tick(object sender, object e)
+        {
+            if (clearPrimaryCommandsFlyout != null)
+            {
+                clearPrimaryCommandsFlyout.PrimaryCommands.Clear();
+            }
+        }
+
         private void DynamicLabelTimer_Tick(object sender, object e)
         {
             if (dynamicLabelSecondaryCommand != null)
@@ -287,6 +340,21 @@ private void DynamicLabelTimer_Tick(object sender, object e)
             }
         }
 
+
+        private void DynamicCommandTimer_Tick(object sender, object e)
+        {
+            dynamicCommandBarFlyout.PrimaryCommands.Add(new AppBarButton() {
+                Content = new TextBlock() { Text = "Test" }
+            });
+
+            PrimaryCommandDynamicallyAddedCheckBox.IsChecked = true;
+
+            if (--dynamicLabelChangeCount == 0)
+            {
+                dynamicCommandTimer.Stop();
+            }
+        }
+
         private void IsRTLCheckBox_Checked(object sender, RoutedEventArgs e)
         {
             FlowDirection = FlowDirection.RightToLeft;
@@ -296,5 +364,21 @@ private void IsRTLCheckBox_Unchecked(object sender, RoutedEventArgs e)
         {
             FlowDirection = FlowDirection.LeftToRight;
         }
+
+        private void OnEditCommandCount6Click(object sender, RoutedEventArgs e)
+        {
+            var flyout6 = Flyout6 as CommandBarFlyout;
+
+            if (flyout6.PrimaryCommands.Count() == 0)
+            {
+                flyout6.PrimaryCommands.Add(new AppBarButton() {
+                    Content = new TextBlock() { Text = "Test" }
+                });
+            }
+            else
+            {
+                flyout6.PrimaryCommands.RemoveAt(0);
+            }
+        }
     }
 }