From b551d6f63f1f6a9705f851a10f27d4b68560881c Mon Sep 17 00:00:00 2001 From: Chris Glein <26607885+chrisglein@users.noreply.github.com> Date: Tue, 22 Jan 2019 13:21:18 -0800 Subject: [PATCH] Keyboard Navigation: 'Back' button cannot be reached using tab order (#204) * Keyboard Navigation: 'Back' button cannot be reached using tab order Remove special case code that disabled tab navigation on the NavigationView back button. [Internal Issue](https://microsoft.visualstudio.com/OS/ft_xamlcon/_workitems/edit/20149845) * Remove test case that assumes tab navigation is disabled on the back button * Remove no longer used m_buttonHolderGettingFocusRevoker --- dev/NavigationView/NavigationView.cpp | 30 ---------- dev/NavigationView/NavigationView.h | 2 - .../NavigationViewTests.cs | 58 ------------------- 3 files changed, 90 deletions(-) diff --git a/dev/NavigationView/NavigationView.cpp b/dev/NavigationView/NavigationView.cpp index e89781b718..265a72e3cf 100644 --- a/dev/NavigationView/NavigationView.cpp +++ b/dev/NavigationView/NavigationView.cpp @@ -110,7 +110,6 @@ void NavigationView::UnhookEventsAndClearFields(bool isFromDestructor) m_paneSearchButtonClickRevoker.revoke(); m_paneSearchButton.set(nullptr); - m_buttonHolderGettingFocusRevoker.revoke(); m_buttonHolderGrid.set(nullptr); } @@ -340,17 +339,6 @@ void NavigationView::OnApplyTemplate() backButtonToolTip.Content(box_value(navigationBackButtonToolTip)); } - if (auto buttonHolderGrid = GetTemplateChildT(c_buttonHolderGrid, controlProtected)) - { - // TrySetNewFocusedElement call in OnButtonHolderGridGettingFocus is RS4 only - if (buttonHolderGrid.try_as()) - { - buttonHolderGrid.XYFocusKeyboardNavigation(winrt::XYFocusKeyboardNavigationMode::Enabled); - buttonHolderGrid.TabFocusNavigation(winrt::KeyboardNavigationMode::Once); - m_buttonHolderGettingFocusRevoker = buttonHolderGrid.GettingFocus(winrt::auto_revoke, { this, &NavigationView::OnButtonHolderGridGettingFocus }); - } - } - if (SharedHelpers::IsRS2OrHigher()) { // Get hold of the outermost grid and enable XYKeyboardNavigationMode @@ -645,24 +633,6 @@ void NavigationView::OnPaneSearchButtonClick(const winrt::IInspectable& /*sender } } -void NavigationView::OnButtonHolderGridGettingFocus(winrt::UIElement const& sender, winrt::GettingFocusEventArgs const& args) -{ - if (auto backButton = m_backButton.get()) - { - auto paneButton = m_paneToggleButton.get(); - if (paneButton && paneButton.Visibility() == winrt::Visibility::Visible) - { - // We want the back button to only be able to receive focus from - // arrowing from the pane toggle button, not from tabbing there. - if (args.NewFocusedElement() == backButton && - (args.Direction() == winrt::FocusNavigationDirection::Previous || args.Direction() == winrt::FocusNavigationDirection::Next)) - { - args.TrySetNewFocusedElement(paneButton); - } - } - } -} - void NavigationView::OpenPane() { auto scopeGuard = gsl::finally([this]() diff --git a/dev/NavigationView/NavigationView.h b/dev/NavigationView/NavigationView.h index 1df01ef852..a4befa2961 100644 --- a/dev/NavigationView/NavigationView.h +++ b/dev/NavigationView/NavigationView.h @@ -171,7 +171,6 @@ class NavigationView : void OnSettingsKeyDown(const winrt::IInspectable& sender, const winrt::KeyRoutedEventArgs& args); void OnSettingsKeyUp(const winrt::IInspectable& sender, const winrt::KeyRoutedEventArgs& args); void OnPaneSearchButtonClick(const winrt::IInspectable& sender, const winrt::RoutedEventArgs& args); - void NavigationView::OnButtonHolderGridGettingFocus(winrt::UIElement const& sender, winrt::GettingFocusEventArgs const& args); void OnItemClick(const winrt::IInspectable& sender, const winrt::ItemClickEventArgs& args); void RaiseItemInvoked(winrt::IInspectable const& item, @@ -312,7 +311,6 @@ class NavigationView : winrt::CoreApplicationViewTitleBar::LayoutMetricsChanged_revoker m_titleBarMetricsChangedRevoker{}; winrt::CoreApplicationViewTitleBar::IsVisibleChanged_revoker m_titleBarIsVisibleChangedRevoker{}; winrt::Button::Click_revoker m_backButtonClickedRevoker{}; - winrt::Button::GettingFocus_revoker m_buttonHolderGettingFocusRevoker{}; winrt::ListView::ItemClick_revoker m_leftNavListViewItemClickRevoker{}; winrt::ListView::Loaded_revoker m_leftNavListViewLoadedRevoker{}; winrt::ListView::SelectionChanged_revoker m_leftNavListViewSelectionChangedRevoker{}; diff --git a/dev/NavigationView/NavigationView_InteractionTests/NavigationViewTests.cs b/dev/NavigationView/NavigationView_InteractionTests/NavigationViewTests.cs index c7b3b60a1f..e5604837fb 100644 --- a/dev/NavigationView/NavigationView_InteractionTests/NavigationViewTests.cs +++ b/dev/NavigationView/NavigationView_InteractionTests/NavigationViewTests.cs @@ -3176,64 +3176,6 @@ public void VerifyLightDismissDoesntSendDuplicateEvents() } } - [TestMethod] - [TestProperty("NavViewTestSuite", "D")] - public void VerifyBackButtonAccessibleOnlyViaXYKeyboard() - { - var testScenarios = RegressionTestScenario.BuildLeftNavRegressionTestScenarios(); - foreach (var testScenario in testScenarios) - { - using (IDisposable page1 = new TestSetupHelper("NavigationView Tests"), - page2 = new TestSetupHelper(testScenario.TestPageName)) - { - bool doVerfications = true; - - if (!PlatformConfiguration.IsOsVersionGreaterThanOrEqual(OSVersion.Redstone4)) - { - // I want the test to still run, just to uncover any crashes that could occur - Log.Warning("This test only works on RS4, but is running without 'verifications' on RS3 and below to weasel our crashes"); - doVerfications = false; - } - - Button backButton = new Button(FindElement.ByName("NavigationViewBackButton")); - Button navButton = new Button(FindElement.ById("TogglePaneButton")); - Button systemBackButton = new Button(UIObject.Root.Descendants.Find(UICondition.CreateFromId("__BackButton"))); - UIObject searchBox = FindElement.ByNameAndClassName("PaneAutoSuggestBox", "TextBox"); - - CheckBox checkBox = new CheckBox(FindElement.ByName("BackButtonEnabledCheckbox")); - checkBox.Toggle(); - searchBox.SetFocus(); - Wait.ForIdle(); - KeyboardHelper.PressKey(Key.Tab, ModifierKey.Shift); - if (doVerfications) Verify.AreEqual(true, navButton.HasKeyboardFocus); - - KeyboardHelper.PressKey(Key.Tab, ModifierKey.Shift); - if (doVerfications) Verify.AreEqual(true, systemBackButton.HasKeyboardFocus); - - KeyboardHelper.PressKey(Key.Tab); - if (doVerfications) Verify.AreEqual(true, navButton.HasKeyboardFocus); - - KeyboardHelper.PressKey(Key.Up); - if (doVerfications) Verify.AreEqual(true, backButton.HasKeyboardFocus); - - Log.Comment("Test that it works with left/right in minimal closed mode"); - SetNavViewWidth(ControlWidth.Narrow); - Wait.ForIdle(); - - navButton.SetFocus(); - Wait.ForIdle(); - KeyboardHelper.PressKey(Key.Tab, ModifierKey.Shift); - if (doVerfications) Verify.AreEqual(true, systemBackButton.HasKeyboardFocus); - - KeyboardHelper.PressKey(Key.Tab); - if (doVerfications) Verify.AreEqual(true, navButton.HasKeyboardFocus); - - KeyboardHelper.PressKey(Key.Left); - if (doVerfications) Verify.AreEqual(true, backButton.HasKeyboardFocus); - } - } - } - [TestMethod] [TestProperty("NavViewTestSuite", "D")] public void VerifyDeselectionDisabled()