CommandBarFlyout Narrator fix (#199)

* Adding FlowsTo and FlowsFrom to ensure that the primary commands are connected to the secondary commands.

dev/CommandBarFlyout/CommandBarFlyoutCommandBar.cpp

@@ -37,18 +37,28 @@ CommandBarFlyoutCommandBar::CommandBarFlyoutCommandBar()
 
     RegisterPropertyChangedCallback(
         winrt::AppBar::IsOpenProperty(),
-        [this](auto const&, auto const&) { UpdateUI(); });
-
-    auto vectorChangedHandler = [this](auto const&, auto const&) { UpdateUI(); };
+        [this](auto const&, auto const&)
+        {
+            UpdateFlowsFromAndFlowsTo();
+            UpdateUI();
+        });
 
     // Since we own these vectors, we don't need to cache the event tokens -
     // in fact, if we tried to remove these handlers in our destructor,
     // these properties will have already been cleared, and we'll nullref.
-    PrimaryCommands().VectorChanged({ [this](auto const&, auto const&) { UpdateUI(); } });
+    PrimaryCommands().VectorChanged({
+        [this](auto const&, auto const&)
+        {
+            UpdateFlowsFromAndFlowsTo();
+            UpdateUI();
+        }
+    });
+
     SecondaryCommands().VectorChanged({
         [this](auto const&, auto const&)
         {
             m_secondaryItemsRootSized = false;
+            UpdateFlowsFromAndFlowsTo();
             UpdateUI();
         }
     });
@@ -68,10 +78,12 @@ void CommandBarFlyoutCommandBar::OnApplyTemplate()
 
     m_primaryItemsRoot.set(GetTemplateChildT<winrt::FrameworkElement>(L"PrimaryItemsRoot", thisAsControlProtected));
     m_secondaryItemsRoot.set(GetTemplateChildT<winrt::FrameworkElement>(L"OverflowContentRoot", thisAsControlProtected));
+    m_moreButton.set(GetTemplateChildT<winrt::ButtonBase>(L"MoreButton", thisAsControlProtected));
     m_openingStoryboard.set(GetTemplateChildT<winrt::Storyboard>(L"OpeningStoryboard", thisAsControlProtected));
     m_closingStoryboard.set(GetTemplateChildT<winrt::Storyboard>(L"ClosingStoryboard", thisAsControlProtected));
 
     AttachEventHandlers();
+    UpdateFlowsFromAndFlowsTo();
     UpdateUI(false /* useTransitions */);
 }
 
@@ -186,6 +198,71 @@ void CommandBarFlyoutCommandBar::PlayCloseAnimation(std::function<void()> onComp
     }
 }
 
+void CommandBarFlyoutCommandBar::UpdateFlowsFromAndFlowsTo()
+{
+    if (m_currentPrimaryItemsEndElement)
+    {
+        winrt::AutomationProperties::GetFlowsTo(m_currentPrimaryItemsEndElement.get()).Clear();
+        m_currentPrimaryItemsEndElement.set(nullptr);
+    }
+
+    if (m_currentSecondaryItemsStartElement)
+    {
+        winrt::AutomationProperties::GetFlowsFrom(m_currentSecondaryItemsStartElement.get()).Clear();
+        m_currentSecondaryItemsStartElement.set(nullptr);
+    }
+
+    // If we're not open, then we don't want to do anything special - the only time we do need to do something special
+    // is when the secondary commands are showing, in which case we want to connect the primary and secondary command lists.
+    if (IsOpen())
+    {
+        auto isElementFocusable = [](winrt::ICommandBarElement const& element)
+        {
+            winrt::Control primaryCommandAsControl = element.try_as<winrt::Control>();
+            return
+                primaryCommandAsControl &&
+                primaryCommandAsControl.Visibility() == winrt::Visibility::Visible &&
+                (primaryCommandAsControl.IsEnabled() || primaryCommandAsControl.AllowFocusWhenDisabled()) &&
+                primaryCommandAsControl.IsTabStop();
+        };
+
+        // If we have a more button, then that's the last element in our primary items list.
+        // Otherwise, we'll find the last focusable element in the primary commands.
+        if (m_moreButton)
+        {
+            m_currentPrimaryItemsEndElement.set(m_moreButton.get());
+        }
+        else
+        {
+            auto primaryCommands = PrimaryCommands();
+            for (int i = static_cast<int>(primaryCommands.Size() - 1); i >= 0; i--)
+            {
+                auto primaryCommand = primaryCommands.GetAt(i);
+                if (isElementFocusable(primaryCommand))
+                {
+                    m_currentPrimaryItemsEndElement.set(primaryCommand.try_as<winrt::FrameworkElement>());
+                    break;
+                }
+            }
+        }
+
+        for (const auto& secondaryCommand : SecondaryCommands())
+        {
+            if (isElementFocusable(secondaryCommand))
+            {
+                m_currentSecondaryItemsStartElement.set(secondaryCommand.try_as<winrt::FrameworkElement>());
+                break;
+            }
+        }
+
+        if (m_currentPrimaryItemsEndElement && m_currentSecondaryItemsStartElement)
+        {
+            winrt::AutomationProperties::GetFlowsTo(m_currentPrimaryItemsEndElement.get()).Append(m_currentSecondaryItemsStartElement.get());
+            winrt::AutomationProperties::GetFlowsFrom(m_currentSecondaryItemsStartElement.get()).Append(m_currentPrimaryItemsEndElement.get());
+        }
+    }
+}
+
 void CommandBarFlyoutCommandBar::UpdateUI(bool useTransitions)
 {
     UpdateTemplateSettings();

dev/CommandBarFlyout/CommandBarFlyoutCommandBar.h

@@ -28,17 +28,26 @@ class CommandBarFlyoutCommandBar :
     void AttachEventHandlers();
     void DetachEventHandlers(bool useSafeGet = false);
 
+    void UpdateFlowsFromAndFlowsTo();
     void UpdateUI(bool useTransitions = true);
     void UpdateVisualState(bool useTransitions);
     void UpdateTemplateSettings();
 
     tracker_ref<winrt::FrameworkElement> m_primaryItemsRoot{ this };
     tracker_ref<winrt::FrameworkElement> m_secondaryItemsRoot{ this };
+    tracker_ref<winrt::ButtonBase> m_moreButton{ this };
     weak_ref<winrt::CommandBarFlyout> m_owningFlyout{ nullptr };
     winrt::FrameworkElement::SizeChanged_revoker m_secondaryItemsRootSizeChangedRevoker{};
     winrt::IInspectable m_keyDownHandler{ nullptr };
     winrt::FrameworkElement::Loaded_revoker m_firstSecondaryItemLoadedRevoker{};
 
+    // We need to manually connect the end element of the primary items to the start element of the secondary items
+    // for the purposes of UIA items navigation. To ensure that we only have the current start and end elements registered
+    // (e.g., if the app adds a new start element to the secondary commands, we want to unregister the previous start element),
+    // we'll save references to those elements.
+    tracker_ref<winrt::FrameworkElement> m_currentPrimaryItemsEndElement{ this };
+    tracker_ref<winrt::FrameworkElement> m_currentSecondaryItemsStartElement{ this };
+
     tracker_ref<winrt::Storyboard> m_openingStoryboard{ this };
     tracker_ref<winrt::Storyboard> m_closingStoryboard{ this };
     winrt::Storyboard::Completed_revoker m_openingStoryboardCompletedRevoker{};

dev/CommandBarFlyout/InteractionTests/CommandBarFlyoutTests.cs

@@ -35,6 +35,10 @@ namespace Windows.UI.Xaml.Tests.MUXControls.InteractionTests
     [TestClass]
     public class CommandBarFlyoutTests
     {
+        // Values taken from https://docs.microsoft.com/en-us/windows/desktop/winauto/uiauto-automation-element-propids
+        private const int UIA_FlowsFromPropertyId = 30148;
+        private const int UIA_FlowsToPropertyId = 30106;
+
         [ClassInitialize]
         [TestProperty("RunAs", "User")]
         [TestProperty("Classification", "Integration")]
@@ -198,5 +202,55 @@ public void CanTapOnSecondaryItems()
                 Verify.IsTrue(isFlyoutOpenCheckBox.ToggleState == ToggleState.Off);
             }
         }
+
+        [TestMethod]
+        public void VerifyFlowsToAndFromConnectsPrimaryAndSecondaryCommands()
+        {
+            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");
+                ToggleButton isFlyoutOpenCheckBox = FindElement.ById<ToggleButton>("IsFlyoutOpenCheckBox");
+
+                Log.Comment("Tapping on a button to show the CommandBarFlyout.");
+                InputHelper.Tap(showCommandBarFlyoutButton);
+
+                // Pre-RS5, CommandBarFlyouts always open expanded,
+                // so we don't need to tap on the more button in that case.
+                if (PlatformConfiguration.IsOsVersionGreaterThanOrEqual(OSVersion.Redstone5))
+                {
+                    Log.Comment("Expanding the CommandBar by invoking the more button.");
+                    FindElement.ById<Button>("MoreButton").InvokeAndWait();
+                }
+
+                Log.Comment("Retrieving the more button and undo button's automation element objects.");
+                FindElement.ById("MoreButton").SetFocus();
+                Wait.ForIdle();
+                var moreButtonElement = AutomationElement.FocusedElement;
+
+                FindElement.ById("UndoButton1").SetFocus();
+                Wait.ForIdle();
+                var undoButtonElement = AutomationElement.FocusedElement;
+
+                Log.Comment("Verifying that the two elements point at each other using FlowsTo and FlowsFrom.");
+                var flowsToCollection = (AutomationElementCollection)moreButtonElement.GetCurrentPropertyValue(AutomationProperty.LookupById(UIA_FlowsToPropertyId));
+
+                Verify.AreEqual(1, flowsToCollection.Count);
+                Verify.AreEqual(undoButtonElement, flowsToCollection[0]);
+
+                var flowsFromCollection = (AutomationElementCollection)undoButtonElement.GetCurrentPropertyValue(AutomationProperty.LookupById(UIA_FlowsFromPropertyId));
+
+                Verify.AreEqual(1, flowsFromCollection.Count);
+                Verify.AreEqual(moreButtonElement, flowsFromCollection[0]);
+
+                Log.Comment("Tapping on a button to hide the CommandBarFlyout.");
+                InputHelper.Tap(showCommandBarFlyoutButton);
+            }
+        }
     }
 }