Skip to content

NavigationView: Fix regression where navigating to the different pane items via the Tab key would crash the app #3613

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ranjeshj merged 13 commits into from
Dec 4, 2020
Merged

NavigationView: Fix regression where navigating to the different pane items via the Tab key would crash the app #3613

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
15d1644
Fix crash issue.
Felix-Dev Nov 3, 2020
5897f62
inner loop solution file cleanup
Felix-Dev Nov 16, 2020
d060c70
Modify interaction tests.
Felix-Dev Nov 16, 2020
ced59fd
Modified existing interaction test (part 2).
Felix-Dev Nov 16, 2020
323e7bd
Update NavigationViewPage.xaml
ranjeshj Nov 23, 2020
728cb2a
fix timing issue in nav view test
ranjeshj Nov 24, 2020
30e599b
Close opened flyout.
Felix-Dev Nov 24, 2020
c171238
inner loop VS automatic updates
Felix-Dev Dec 1, 2020
7cbf0d4
fix merge conflicts
Felix-Dev Dec 1, 2020
f0b5130
inner loop VS automatic update
Felix-Dev Dec 1, 2020
de4f0d5
Merge branch 'master' into user/Felix-Dev/navview-settingsitem-tab-fo…
Felix-Dev Dec 1, 2020
2845ea3
Another attempt to fix test failure.
Felix-Dev Dec 1, 2020
feb8cc3
Remove empty line.
Felix-Dev Dec 1, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 6 additions & 22 deletions MUXControlsInnerLoop.sln
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@


Microsoft Visual Studio Solution File, Format Version 12.00
# Visual Studio Version 16
VisualStudioVersion = 16.0.29009.5
Expand Down Expand Up @@ -618,14 +618,10 @@ Global
dev\Common\Common.vcxitems*{ad0c90b0-4845-4d4b-88f1-86f653f8171b}*SharedItemsImports = 4
dev\DropDownButton\DropDownButton.vcxitems*{ad0c90b0-4845-4d4b-88f1-86f653f8171b}*SharedItemsImports = 4
dev\Effects\Microsoft.UI.Composition.Effects.vcxitems*{ad0c90b0-4845-4d4b-88f1-86f653f8171b}*SharedItemsImports = 4
dev\Expander\Expander.vcxitems*{ad0c90b0-4845-4d4b-88f1-86f653f8171b}*SharedItemsImports = 4
dev\IconSource\IconSource.vcxitems*{ad0c90b0-4845-4d4b-88f1-86f653f8171b}*SharedItemsImports = 4
dev\InfoBar\InfoBar.vcxitems*{ad0c90b0-4845-4d4b-88f1-86f653f8171b}*SharedItemsImports = 4
dev\Lights\Lights.vcxitems*{ad0c90b0-4845-4d4b-88f1-86f653f8171b}*SharedItemsImports = 4
dev\Materials\Acrylic\AcrylicBrush.vcxitems*{ad0c90b0-4845-4d4b-88f1-86f653f8171b}*SharedItemsImports = 4
dev\Materials\Reveal\RevealBrush.vcxitems*{ad0c90b0-4845-4d4b-88f1-86f653f8171b}*SharedItemsImports = 4
dev\NumberBox\NumberBox.vcxitems*{ad0c90b0-4845-4d4b-88f1-86f653f8171b}*SharedItemsImports = 4
dev\Repeater\Repeater.vcxitems*{ad0c90b0-4845-4d4b-88f1-86f653f8171b}*SharedItemsImports = 4
dev\ResourceHelper\ResourceHelper.vcxitems*{ad0c90b0-4845-4d4b-88f1-86f653f8171b}*SharedItemsImports = 4
dev\SplitButton\SplitButton.vcxitems*{ad0c90b0-4845-4d4b-88f1-86f653f8171b}*SharedItemsImports = 4
dev\Telemetry\Telemetry.vcxitems*{ad0c90b0-4845-4d4b-88f1-86f653f8171b}*SharedItemsImports = 4
Expand Down Expand Up @@ -667,10 +663,6 @@ Global
dev\TreeView\APITests\TreeView_APITests.projitems*{de885c66-929c-464e-bac4-3e076ec46483}*SharedItemsImports = 13
dev\Pivot\TestUI\Pivot_TestUI.projitems*{deb3fa60-e4a7-4735-89f2-363c7c56b428}*SharedItemsImports = 13
dev\CommonManaged\CommonManaged.projitems*{dedc1e4f-cfa5-4443-83eb-e79d425df7e7}*SharedItemsImports = 4
dev\Expander\TestUI\Expander_TestUI.projitems*{dedc1e4f-cfa5-4443-83eb-e79d425df7e7}*SharedItemsImports = 4
dev\InfoBar\TestUI\InfoBar_TestUI.projitems*{dedc1e4f-cfa5-4443-83eb-e79d425df7e7}*SharedItemsImports = 4
dev\NumberBox\APITests\NumberBox_APITests.projitems*{dedc1e4f-cfa5-4443-83eb-e79d425df7e7}*SharedItemsImports = 4
dev\NumberBox\TestUI\NumberBox_TestUI.projitems*{dedc1e4f-cfa5-4443-83eb-e79d425df7e7}*SharedItemsImports = 4
test\TestAppUtils\TestAppUtils.projitems*{dedc1e4f-cfa5-4443-83eb-e79d425df7e7}*SharedItemsImports = 4
dev\SplitButton\InteractionTests\SplitButton_InteractionTests.projitems*{e1c861e2-c4d9-41e1-aed7-5e203451bd4d}*SharedItemsImports = 13
dev\DatePicker\TestUI\DatePicker_TestUI.projitems*{e20f725c-3a53-463b-ada9-ff2088aaca4d}*SharedItemsImports = 13
Expand All @@ -688,10 +680,6 @@ Global
dev\Materials\Acrylic\InteractionTests\AcrylicBrush_InteractionTests.projitems*{f601284a-00c1-49f9-99b3-70d45585f784}*SharedItemsImports = 13
dev\SplitButton\SplitButton.vcxitems*{faf114dd-af1f-4d9f-a511-354c19912aad}*SharedItemsImports = 9
dev\CommonManaged\CommonManaged.projitems*{fbc396f5-26dd-4ca3-981e-c7bc9fea4546}*SharedItemsImports = 4
dev\Expander\TestUI\Expander_TestUI.projitems*{fbc396f5-26dd-4ca3-981e-c7bc9fea4546}*SharedItemsImports = 4
dev\InfoBar\TestUI\InfoBar_TestUI.projitems*{fbc396f5-26dd-4ca3-981e-c7bc9fea4546}*SharedItemsImports = 4
dev\NumberBox\APITests\NumberBox_APITests.projitems*{fbc396f5-26dd-4ca3-981e-c7bc9fea4546}*SharedItemsImports = 4
dev\NumberBox\TestUI\NumberBox_TestUI.projitems*{fbc396f5-26dd-4ca3-981e-c7bc9fea4546}*SharedItemsImports = 4
test\TestAppUtils\TestAppUtils.projitems*{fbc396f5-26dd-4ca3-981e-c7bc9fea4546}*SharedItemsImports = 4
dev\Slider\Slider.vcxitems*{fc2178ca-7f72-40f0-916c-a2b3750bbb6c}*SharedItemsImports = 9
dev\LayoutPanel\LayoutPanel.vcxitems*{fd3c1a00-0d07-4849-a3b9-646f0ff21d7b}*SharedItemsImports = 9
Expand Down Expand Up @@ -1139,19 +1127,15 @@ Global
{990BBC65-1114-435D-B5CC-1EB8AAAD72DD} = {633B8D7A-3207-4BD3-8159-B65710AEA6FF}
{128E6F7A-578C-48DC-BD3F-750EC662C268} = {633B8D7A-3207-4BD3-8159-B65710AEA6FF}
{6F7831A4-48F8-41E8-A573-C567A1223CB5} = {05CB5DBD-A481-4DFF-B1A3-642F049D165C}
{93FAB536-1139-4FD1-B0BF-97C0C6FAB378} = {67599AD5-51EC-44CB-85CE-B60CD8CBA270}
{EC3B6F65-32C6-4BC8-8902-EE0B397E2787} = {93FAB536-1139-4FD1-B0BF-97C0C6FAB378}
{50C1F1D3-20AA-49A8-9E4C-CF4E5811A1D8} = {93FAB536-1139-4FD1-B0BF-97C0C6FAB378}
{D6DF4AB9-FACC-4E51-8C57-6B1F96919365} = {93FAB536-1139-4FD1-B0BF-97C0C6FAB378}
{E7C2104F-770D-4375-8D3A-D173F222273A} = {67599AD5-51EC-44CB-85CE-B60CD8CBA270}
{AB3261A7-9A8D-4A27-AEA2-3AAC0419C889} = {E7C2104F-770D-4375-8D3A-D173F222273A}
{225C4174-3141-49B8-ADE2-C7D3408D5103} = {E7C2104F-770D-4375-8D3A-D173F222273A}
{4F64C819-664F-436A-BCDB-8AB3019B9DD5} = {E7C2104F-770D-4375-8D3A-D173F222273A}
{CB2352E2-D633-41A3-8CDC-B28731A4C490} = {E7C2104F-770D-4375-8D3A-D173F222273A}
{0EC260CC-03C7-4790-B16A-43428EBCF5AD} = {67599AD5-51EC-44CB-85CE-B60CD8CBA270}
{3566798E-9E24-44EF-B89D-2A62AE8F697A} = {0EC260CC-03C7-4790-B16A-43428EBCF5AD}
{675121BF-CABC-48E7-9C9D-4571BC507406} = {0EC260CC-03C7-4790-B16A-43428EBCF5AD}
{675373CE-6ACD-4C4B-A009-09A3C9B218E6} = {0EC260CC-03C7-4790-B16A-43428EBCF5AD}
{93FAB536-1139-4FD1-B0BF-97C0C6FAB378} = {67599AD5-51EC-44CB-85CE-B60CD8CBA270}
{EC3B6F65-32C6-4BC8-8902-EE0B397E2787} = {93FAB536-1139-4FD1-B0BF-97C0C6FAB378}
{50C1F1D3-20AA-49A8-9E4C-CF4E5811A1D8} = {93FAB536-1139-4FD1-B0BF-97C0C6FAB378}
{D6DF4AB9-FACC-4E51-8C57-6B1F96919365} = {93FAB536-1139-4FD1-B0BF-97C0C6FAB378}
{1AD0CB4F-47F0-432B-8D4F-CE33FA3EB8A9} = {67599AD5-51EC-44CB-85CE-B60CD8CBA270}
{CCC102B7-F5EF-479D-94F1-008D189448B1} = {1AD0CB4F-47F0-432B-8D4F-CE33FA3EB8A9}
{F470A64E-780E-45AA-ABB7-73A8734E51D7} = {1AD0CB4F-47F0-432B-8D4F-CE33FA3EB8A9}
Expand All @@ -1160,4 +1144,4 @@ Global
GlobalSection(ExtensibilityGlobals) = postSolution
SolutionGuid = {D93836AB-52D3-4DE2-AE25-23F26F55ECED}
EndGlobalSection
EndGlobal
EndGlobal
5 changes: 5 additions & 0 deletions dev/NavigationView/NavigationView.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1018,6 +1018,11 @@ winrt::ItemsRepeater NavigationView::GetParentRootItemsRepeaterForContainer(cons
while (!IsRootItemsRepeater(parentIR))
{
currentNvib = GetParentNavigationViewItemForContainer(currentNvib);
if (!currentNvib)
{
return nullptr;
}

parentIR = GetParentItemsRepeaterForContainer(currentNvib);
}
return parentIR;
Expand Down
197 changes: 148 additions & 49 deletions dev/NavigationView/NavigationView_InteractionTests/FocusBehaviorTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,57 +188,123 @@ public void TabNavigationTest()
{
using (var setup = new TestSetupHelper(new[] { "NavigationView Tests", testScenario.TestPageName }))
{
//TODO: Update RS3 and below tab behavior to match RS4+
// TODO: Update RS3 and below tab behavior to match RS4+
if (!PlatformConfiguration.IsOsVersionGreaterThanOrEqual(OSVersion.Redstone4))
{
Log.Warning("Test is disabled because the repeater tab behavior is current different rs3 and below.");
Log.Warning("Test is disabled because the repeater tab behavior is currently different on RS3 and below.");
return;
}

SetNavViewWidth(ControlWidth.Wide);

Button togglePaneButton = new Button(FindElement.ById("TogglePaneButton"));
UIObject searchBox = FindElement.ByNameAndClassName("PaneAutoSuggestBox", "TextBox");
UIObject firstItem = FindElement.ByName("Home");
UIObject searchBox = FindElement.ByNameAndClassName("PaneAutoSuggestBox", "TextBox");
UIObject settingsItem = FindElement.ByName("Settings");
togglePaneButton.SetFocus();
Wait.ForIdle();

Log.Comment("Verify that pressing tab while TogglePaneButton has focus moves to the search box");
KeyboardHelper.PressKey(Key.Tab);
Wait.ForIdle();
Verify.IsTrue(searchBox.HasKeyboardFocus);

Log.Comment("Verify that pressing tab while the search box has focus moves to the first menu item");
KeyboardHelper.PressKey(Key.Tab);
Wait.ForIdle();
Verify.IsTrue(firstItem.HasKeyboardFocus);

Log.Comment("Verify that pressing tab twice more will move focus to the settings item");
KeyboardHelper.PressKey(Key.Tab, ModifierKey.None, 2);
Wait.ForIdle();
Verify.IsTrue(settingsItem.HasKeyboardFocus);
VerifyTabNavigationWithoutMenuItemSelected();
VerifyTabNavigationWithMenuItemSelected();

// TODO: Re-enable test part and remove workaround once saving tab state is fixed

Log.Comment("Move Focus to first item");
firstItem.SetFocus();
Wait.ForIdle();

//Log.Comment("Verify that pressing SHIFT+tab twice will move focus to the first menu item");
//KeyboardHelper.PressKey(Key.Tab, ModifierKey.Shift, 2);
//Wait.ForIdle();
//Verify.IsTrue(firstItem.HasKeyboardFocus);
void VerifyTabNavigationWithoutMenuItemSelected()
{
Log.Comment("Verify Tab navigation without a selected menu item");

// Clear any item selection
var clearSelectedItemButton = new Button(FindElement.ByName("ClearSelectedItemButton"));
clearSelectedItemButton.Click();
Wait.ForIdle();

Verify.AreEqual("null", GetSelectedItem());

UIObject firstMenuItem = FindElement.ByName("Home");
UIObject lastMenuItem = FindElement.ByName("HasChildItem");

// Set focus on the pane's toggle button.
togglePaneButton.SetFocus();
Wait.ForIdle();

Log.Comment("Verify that pressing tab while TogglePaneButton has focus moves to the search box");
KeyboardHelper.PressKey(Key.Tab);
Wait.ForIdle();
Verify.IsTrue(searchBox.HasKeyboardFocus);

Log.Comment("Verify that pressing tab while the search box has focus moves to the first menu item");
KeyboardHelper.PressKey(Key.Tab);
Wait.ForIdle();
Verify.IsTrue(firstMenuItem.HasKeyboardFocus);

Log.Comment("Verify that pressing tab thrice more will move focus to the settings item");
KeyboardHelper.PressKey(Key.Tab, ModifierKey.None, 3);
Wait.ForIdle();
Verify.IsTrue(settingsItem.HasKeyboardFocus);

Log.Comment("Verify that pressing SHIFT+tab thrice will move focus to the last menu item");
KeyboardHelper.PressKey(Key.Tab, ModifierKey.Shift, 3);
Wait.ForIdle();
Verify.IsTrue(lastMenuItem.HasKeyboardFocus);

Log.Comment("Verify that pressing SHIFT+tab will move focus to the search box");
KeyboardHelper.PressKey(Key.Tab, ModifierKey.Shift, 1);
Wait.ForIdle();
Verify.IsTrue(searchBox.HasKeyboardFocus);

Log.Comment("Verify that pressing SHIFT+tab will move focus to the TogglePaneButton");
KeyboardHelper.PressKey(Key.Tab, ModifierKey.Shift, 1);
Wait.ForIdle();
Verify.IsTrue(togglePaneButton.HasKeyboardFocus);
}

Log.Comment("Verify that pressing SHIFT+tab will move focus to the search box");
KeyboardHelper.PressKey(Key.Tab, ModifierKey.Shift, 1);
Wait.ForIdle();
Verify.IsTrue(searchBox.HasKeyboardFocus);
void VerifyTabNavigationWithMenuItemSelected()
{
Log.Comment("Verify Tab navigation with a selected menu item");

// Select a menu item (preferably not the first or last menu item)
UIObject thirdMenuItem = FindElement.ByName("Games");

var selectedItemComboBox = new ComboBox(FindElement.ById("SelectedItemCombobox"));
selectedItemComboBox.SelectItemByName("Games");
Wait.ForIdle();

Verify.IsTrue(Convert.ToBoolean(thirdMenuItem.GetProperty(UIProperty.Get("SelectionItem.IsSelected"))));

// Set focus on the pane's toggle button.
togglePaneButton.SetFocus();
Wait.ForIdle();

Log.Comment("Verify that pressing tab while TogglePaneButton has focus moves to the search box");
KeyboardHelper.PressKey(Key.Tab);
Wait.ForIdle();
Verify.IsTrue(searchBox.HasKeyboardFocus);

Log.Comment("Verify that pressing tab while the search box has focus moves to the selected menu item");
KeyboardHelper.PressKey(Key.Tab);
Wait.ForIdle();
Verify.IsTrue(thirdMenuItem.HasKeyboardFocus);

Log.Comment("Verify that pressing tab thrice more will move focus to the settings item");
KeyboardHelper.PressKey(Key.Tab, ModifierKey.None, 3);
Wait.ForIdle();
Verify.IsTrue(settingsItem.HasKeyboardFocus);

Log.Comment("Verify that pressing SHIFT+tab thrice will move focus to the selected menu item");
KeyboardHelper.PressKey(Key.Tab, ModifierKey.Shift, 3);
Wait.ForIdle();
Verify.IsTrue(thirdMenuItem.HasKeyboardFocus);

Log.Comment("Verify that pressing SHIFT+tab will move focus to the search box");
KeyboardHelper.PressKey(Key.Tab, ModifierKey.Shift, 1);
Wait.ForIdle();
Verify.IsTrue(searchBox.HasKeyboardFocus);

Log.Comment("Verify that pressing SHIFT+tab will move focus to the TogglePaneButton");
KeyboardHelper.PressKey(Key.Tab, ModifierKey.Shift, 1);
Wait.ForIdle();
Verify.IsTrue(togglePaneButton.HasKeyboardFocus);
}
}

Log.Comment("Verify that pressing SHIFT+tab will move focus to the TogglePaneButton");
KeyboardHelper.PressKey(Key.Tab, ModifierKey.Shift, 1);
Wait.ForIdle();
Verify.IsTrue(togglePaneButton.HasKeyboardFocus);
string GetSelectedItem()
{
return FindElement.ByName("SelectionChangedItemType").GetText();
}
}
}
Expand Down Expand Up @@ -286,9 +352,16 @@ public void ArrowKeyNavigationTest()

SetNavViewWidth(ControlWidth.Wide);

// Clear any item selection
var clearSelectedItemButton = new Button(FindElement.ByName("ClearSelectedItemButton"));
clearSelectedItemButton.Click();
Wait.ForIdle();

Verify.AreEqual("null", GetSelectedItem());

Button togglePaneButton = new Button(FindElement.ById("TogglePaneButton"));
togglePaneButton.SetFocus();
Wait.ForIdle();
Wait.ForIdle();

// Grab references to all the menu items in the test UI
UIObject searchBox = FindElement.ByNameAndClassName("PaneAutoSuggestBox", "TextBox");
Expand All @@ -298,6 +371,9 @@ public void ArrowKeyNavigationTest()
UIObject item4 = FindElement.ByName("Music");
UIObject item5 = FindElement.ByName("Movies");
UIObject item6 = FindElement.ByName("TV");
UIObject item7 = FindElement.ByName("Volume");
UIObject item8 = FindElement.ByName("Integer");
UIObject item9 = FindElement.ByName("HasChildItem");
UIObject settingsItem = FindElement.ByName("Settings");

Log.Comment("Verify that tab from the TogglePaneButton goes to the search box");
Expand Down Expand Up @@ -331,23 +407,41 @@ public void ArrowKeyNavigationTest()
Wait.ForIdle();
Verify.IsTrue(item6.HasKeyboardFocus);

Log.Comment("Verify that tab twice from the last menu item goes to the settings item");
KeyboardHelper.PressKey(Key.Tab, ModifierKey.None, 2);
KeyboardHelper.PressKey(Key.Down);
Wait.ForIdle();
Verify.IsTrue(settingsItem.HasKeyboardFocus);
Verify.IsTrue(item7.HasKeyboardFocus);

// TODO: Re-enable test part and remove workaround once saving tab state is fixed
KeyboardHelper.PressKey(Key.Down);
Wait.ForIdle();
Verify.IsTrue(item8.HasKeyboardFocus);

Log.Comment("Move Focus to TV item");
item6.SetFocus();
KeyboardHelper.PressKey(Key.Down);
Wait.ForIdle();
Verify.IsTrue(item9.HasKeyboardFocus);

//Log.Comment("Verify that shift+tab twice from the settings item goes to the last menu item");
//KeyboardHelper.PressKey(Key.Tab, ModifierKey.Shift, 2);
//Wait.ForIdle();
//Verify.IsTrue(item6.HasKeyboardFocus);
Log.Comment("Verify that tab thrice from the last menu item goes to the settings item");
KeyboardHelper.PressKey(Key.Tab, ModifierKey.None, 3);
Wait.ForIdle();
Verify.IsTrue(settingsItem.HasKeyboardFocus);

Log.Comment("Verify that shift+tab thrice from the settings item goes to the last menu item");
KeyboardHelper.PressKey(Key.Tab, ModifierKey.Shift, 3);
Wait.ForIdle();
Verify.IsTrue(item9.HasKeyboardFocus);

Log.Comment("Verify that up arrow can navigate through all items");
KeyboardHelper.PressKey(Key.Up);
Wait.ForIdle();
Verify.IsTrue(item8.HasKeyboardFocus);

KeyboardHelper.PressKey(Key.Up);
Wait.ForIdle();
Verify.IsTrue(item7.HasKeyboardFocus);

KeyboardHelper.PressKey(Key.Up);
Wait.ForIdle();
Verify.IsTrue(item6.HasKeyboardFocus);

KeyboardHelper.PressKey(Key.Up);
Wait.ForIdle();
Verify.IsTrue(item5.HasKeyboardFocus);
Expand Down Expand Up @@ -378,6 +472,11 @@ public void ArrowKeyNavigationTest()
Wait.ForIdle();
Verify.IsTrue(togglePaneButton.HasKeyboardFocus);
}

string GetSelectedItem()
{
return FindElement.ByName("SelectionChangedItemType").GetText();
}
}
}

Expand Down
Loading