From e19d61d6793ee32703753dbd5bf9c4038bfa7902 Mon Sep 17 00:00:00 2001
From: Barbara Kudiess <bakudies@microsoft.com>
Date: Tue, 12 Apr 2022 09:58:09 -0700
Subject: [PATCH] MenuBar Keyboard Accessibility Fixes (#6953)

* Fixes Bug 36685596: [WinUI3] [Notepad] Users is unable to navigate to 'Zoom' sub-menu using 'Right' arrow key.

* Fixes Bug 36629906: [WinUI2] [Notepad]Focus is not visible on 'View' menu-item, upon activating 'Status bar' & 'Word wrap' present in 'View' menu item.

* Fixes Bug 38190286: [WinUI2] [Notepad>Home]: Keyboard focus is not moving in cycle among the 'File', 'Edit' & 'View' menu-items, while navigating via 'Left/Right' arrow keys.

* Add comment

* Add tests

* update handled of key press
---
 dev/MenuBar/MenuBarItem.cpp                   | 52 ++++++++++++++++++-
 dev/MenuBar/MenuBarItem.h                     |  1 +
 .../MenuBar_InteractionTests/MenuBarTests.cs  | 20 +++++++
 3 files changed, 72 insertions(+), 1 deletion(-)

diff --git a/dev/MenuBar/MenuBarItem.cpp b/dev/MenuBar/MenuBarItem.cpp
index 673408885a..762ab3abfc 100755
--- a/dev/MenuBar/MenuBarItem.cpp
+++ b/dev/MenuBar/MenuBarItem.cpp
@@ -160,10 +160,43 @@ void MenuBarItem::OnMenuBarItemKeyDown( winrt::IInspectable const& sender, winrt
     {
         ShowMenuFlyout();
     }
+    else if (key == winrt::VirtualKey::Right)
+    {
+        if (FlowDirection() == winrt::FlowDirection::RightToLeft)
+        {
+            MoveFocusTo(FlyoutLocation::Left);
+        }
+        else
+        {
+            MoveFocusTo(FlyoutLocation::Right);
+        }
+        args.Handled(TRUE);
+    }
+    else if (key == winrt::VirtualKey::Left)
+    {
+        if (FlowDirection() == winrt::FlowDirection::RightToLeft)
+        {
+            MoveFocusTo(FlyoutLocation::Right);
+        }
+        else
+        {
+            MoveFocusTo(FlyoutLocation::Left);
+        }
+        args.Handled(TRUE);
+    }
 }
 
 void MenuBarItem::OnPresenterKeyDown( winrt::IInspectable const& sender, winrt::KeyRoutedEventArgs const& args)
 {
+    // If the event came from a MenuFlyoutSubItem it means right/left arrow will open it, so we should not handle them to not override default behaviour
+    if (auto const& subitem = args.OriginalSource().try_as<winrt::MenuFlyoutSubItem>())
+    {
+        if (subitem.Items().GetAt(0))
+        {
+            return;
+        }
+    }
+
     const auto key = args.Key();
     if (key == winrt::VirtualKey::Right)
     {
@@ -269,6 +302,23 @@ void MenuBarItem::OpenFlyoutFrom(FlyoutLocation location)
     }
 }
 
+void MenuBarItem::MoveFocusTo(FlyoutLocation location)
+{
+    if (auto menuBar = m_menuBar.get())
+    {
+        uint32_t index = 0;
+        menuBar.Items().IndexOf(*this, index);
+        if (location == FlyoutLocation::Left)
+        {
+            winrt::get_self<MenuBarItem>(menuBar.Items().GetAt(((index - 1) + menuBar.Items().Size()) % menuBar.Items().Size()))->Focus(winrt::FocusState::Programmatic);
+        }
+        else
+        {
+            winrt::get_self<MenuBarItem>(menuBar.Items().GetAt((index + 1) % menuBar.Items().Size()))->Focus(winrt::FocusState::Programmatic);
+        }
+    }
+}
+
 void MenuBarItem::AddPassThroughElement(const winrt::DependencyObject& element)
 {
     m_passThroughElement = winrt::make_weak(element);
@@ -306,7 +356,7 @@ void MenuBarItem::OnFlyoutClosed( winrt::IInspectable const& sender, winrt::IIns
 
 void MenuBarItem::OnFlyoutOpening( winrt::IInspectable const& sender, winrt::IInspectable const& args)
 {
-    Focus(winrt::FocusState::Pointer);
+    Focus(winrt::FocusState::Programmatic);
 
     m_isFlyoutOpen = true;
 
diff --git a/dev/MenuBar/MenuBarItem.h b/dev/MenuBar/MenuBarItem.h
index ed62137f3a..7dd3108306 100755
--- a/dev/MenuBar/MenuBarItem.h
+++ b/dev/MenuBar/MenuBarItem.h
@@ -39,6 +39,7 @@ class MenuBarItem :
     void AttachEventHandlers();
     void DetachEventHandlers(bool useSafeGet = false);
     void OpenFlyoutFrom(FlyoutLocation location);
+    void MoveFocusTo(FlyoutLocation location);
 
     void OnVisualPropertyChanged(const winrt::DependencyObject& sender, const winrt::DependencyProperty& args);
     void UpdateVisualStates();
diff --git a/dev/MenuBar/MenuBar_InteractionTests/MenuBarTests.cs b/dev/MenuBar/MenuBar_InteractionTests/MenuBarTests.cs
index 89e5e33b44..5c88dea610 100644
--- a/dev/MenuBar/MenuBar_InteractionTests/MenuBarTests.cs
+++ b/dev/MenuBar/MenuBar_InteractionTests/MenuBarTests.cs
@@ -158,11 +158,31 @@ public void KeyboardNavigationWithArrowKeysTest()
 
                 if (ApiInformation.IsTypePresent("Windows.UI.Xaml.IUIElement5")) // XYFocusNavigation is only availabe from IUElement5 foward
                 {
+                    KeyboardHelper.PressKey(Key.Left);
+                    VerifyElement.NotFound("Word Wrap", FindBy.Name);
+
+                    KeyboardHelper.PressKey(Key.Enter);
+                    VerifyElement.Found("Word Wrap", FindBy.Name);
+
+                    KeyboardHelper.PressKey(Key.Escape);
+                    KeyboardHelper.PressKey(Key.Right);
+
                     KeyboardHelper.PressKey(Key.Right);
                     VerifyElement.NotFound("Undo", FindBy.Name);
 
                     KeyboardHelper.PressKey(Key.Enter);
                     VerifyElement.Found("Undo", FindBy.Name);
+
+                    KeyboardHelper.PressKey(Key.Down);
+                    KeyboardHelper.PressKey(Key.Down);
+                    KeyboardHelper.PressKey(Key.Down);
+                    KeyboardHelper.PressKey(Key.Down);
+                    KeyboardHelper.PressKey(Key.Down);
+                    VerifyElement.NotFound("Item 1", FindBy.Name);
+
+                    KeyboardHelper.PressKey(Key.Right);
+                    VerifyElement.Found("Item 1", FindBy.Name);
+
                 }
             }
         }