From 69f96d5b441c00c9c1e954f96a476b8fdb308ea4 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sat, 9 Apr 2022 00:13:17 +0200 Subject: [PATCH 1/7] Added failing integration test for #7906. --- samples/IntegrationTestApp/MainWindow.axaml | 5 ++++- .../ElementExtensions.cs | 11 +++++++++++ .../Avalonia.IntegrationTests.Appium/MenuTests.cs | 15 +++++++++++++++ 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/samples/IntegrationTestApp/MainWindow.axaml b/samples/IntegrationTestApp/MainWindow.axaml index fe1487a7f2..0b5c20b849 100644 --- a/samples/IntegrationTestApp/MainWindow.axaml +++ b/samples/IntegrationTestApp/MainWindow.axaml @@ -87,7 +87,10 @@ - None + + None + + diff --git a/tests/Avalonia.IntegrationTests.Appium/ElementExtensions.cs b/tests/Avalonia.IntegrationTests.Appium/ElementExtensions.cs index 15e22f4424..290811d2a1 100644 --- a/tests/Avalonia.IntegrationTests.Appium/ElementExtensions.cs +++ b/tests/Avalonia.IntegrationTests.Appium/ElementExtensions.cs @@ -29,6 +29,12 @@ namespace Avalonia.IntegrationTests.Appium _ => throw new ArgumentOutOfRangeException($"Unexpected IsChecked value.") }; + public static bool GetIsFocused(this AppiumWebElement element) + { + var active = element.WrappedDriver.SwitchTo().ActiveElement() as AppiumWebElement; + return element.Id == active?.Id; + } + public static void SendClick(this AppiumWebElement element) { if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) @@ -44,6 +50,11 @@ namespace Avalonia.IntegrationTests.Appium } } + public static void MovePointerOver(this AppiumWebElement element) + { + new Actions(element.WrappedDriver).MoveToElement(element).Perform(); + } + public static string GetAttribute(AppiumWebElement element, string windows, string macOS) { return element.GetAttribute(RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? windows : macOS); diff --git a/tests/Avalonia.IntegrationTests.Appium/MenuTests.cs b/tests/Avalonia.IntegrationTests.Appium/MenuTests.cs index e9a433b975..0562611a5b 100644 --- a/tests/Avalonia.IntegrationTests.Appium/MenuTests.cs +++ b/tests/Avalonia.IntegrationTests.Appium/MenuTests.cs @@ -59,5 +59,20 @@ namespace Avalonia.IntegrationTests.Appium Assert.Equal("Ctrl+O", childMenuItem.GetAttribute("AcceleratorKey")); } + + [Fact] + public void PointerOver_Does_Not_Steal_Focus() + { + // Issue #7906 + var textBox = _session.FindElementByAccessibilityId("MenuFocusTest"); + textBox.Click(); + + Assert.True(textBox.GetIsFocused()); + + var rootMenuItem = _session.FindElementByAccessibilityId("RootMenuItem"); + rootMenuItem.MovePointerOver(); + + Assert.True(textBox.GetIsFocused()); + } } } From 5d35ac324f3deef50955ef1e4fa0f8f47b44bed6 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sat, 9 Apr 2022 01:53:57 +0200 Subject: [PATCH 2/7] Additional menu integration tests. --- samples/IntegrationTestApp/MainWindow.axaml | 3 +- .../IntegrationTestApp/MainWindow.axaml.cs | 2 + .../MenuTests.cs | 61 ++++++++++++++++++- 3 files changed, 63 insertions(+), 3 deletions(-) diff --git a/samples/IntegrationTestApp/MainWindow.axaml b/samples/IntegrationTestApp/MainWindow.axaml index 0b5c20b849..19ac68b15b 100644 --- a/samples/IntegrationTestApp/MainWindow.axaml +++ b/samples/IntegrationTestApp/MainWindow.axaml @@ -82,13 +82,14 @@ - + None + diff --git a/samples/IntegrationTestApp/MainWindow.axaml.cs b/samples/IntegrationTestApp/MainWindow.axaml.cs index b9e631a312..9a612aa94d 100644 --- a/samples/IntegrationTestApp/MainWindow.axaml.cs +++ b/samples/IntegrationTestApp/MainWindow.axaml.cs @@ -62,6 +62,8 @@ namespace IntegrationTestApp this.FindControl("BasicComboBox").SelectedIndex = 0; if (source?.Name == "ListBoxSelectionClear") this.FindControl("BasicListBox").SelectedIndex = -1; + if (source?.Name == "MenuClickedMenuItemReset") + this.FindControl("ClickedMenuItem").Text = "None"; } } } diff --git a/tests/Avalonia.IntegrationTests.Appium/MenuTests.cs b/tests/Avalonia.IntegrationTests.Appium/MenuTests.cs index 0562611a5b..763ad5f400 100644 --- a/tests/Avalonia.IntegrationTests.Appium/MenuTests.cs +++ b/tests/Avalonia.IntegrationTests.Appium/MenuTests.cs @@ -1,4 +1,7 @@ -using OpenQA.Selenium.Appium; +using System.Threading; +using OpenQA.Selenium; +using OpenQA.Selenium.Appium; +using OpenQA.Selenium.Interactions; using Xunit; namespace Avalonia.IntegrationTests.Appium @@ -15,6 +18,12 @@ namespace Avalonia.IntegrationTests.Appium var tabs = _session.FindElementByAccessibilityId("MainTabs"); var tab = tabs.FindElementByName("Menu"); tab.Click(); + + var reset = _session.FindElementByAccessibilityId("MenuClickedMenuItemReset"); + reset.Click(); + + var clickedMenuItem = _session.FindElementByAccessibilityId("ClickedMenuItem"); + Assert.Equal("None", clickedMenuItem.Text); } [Fact] @@ -48,11 +57,59 @@ namespace Avalonia.IntegrationTests.Appium Assert.Equal("_Grandchild", clickedMenuItem.Text); } + [PlatformFact(SkipOnOSX = true)] + public void Select_Child_With_Arrow_Keys() + { + new Actions(_session) + .KeyDown(Keys.Alt).KeyUp(Keys.Alt) + .SendKeys(Keys.Down + Keys.Enter) + .Perform(); + + var clickedMenuItem = _session.FindElementByAccessibilityId("ClickedMenuItem"); + Assert.Equal("_Child 1", clickedMenuItem.Text); + } + + [PlatformFact(SkipOnOSX = true)] + public void Select_Grandchild_With_Arrow_Keys() + { + new Actions(_session) + .KeyDown(Keys.Alt).KeyUp(Keys.Alt) + .SendKeys(Keys.Down + Keys.Down + Keys.Right + Keys.Enter) + .Perform(); + + var clickedMenuItem = _session.FindElementByAccessibilityId("ClickedMenuItem"); + Assert.Equal("_Grandchild", clickedMenuItem.Text); + } + + [PlatformFact(SkipOnOSX = true)] + public void Select_Child_With_Access_Keys() + { + new Actions(_session) + .KeyDown(Keys.Alt).KeyUp(Keys.Alt) + .SendKeys("rc") + .Perform(); + + var clickedMenuItem = _session.FindElementByAccessibilityId("ClickedMenuItem"); + Assert.Equal("_Child 1", clickedMenuItem.Text); + } + + [PlatformFact(SkipOnOSX = true)] + public void Select_Grandchild_With_Access_Keys() + { + new Actions(_session) + .KeyDown(Keys.Alt).KeyUp(Keys.Alt) + .SendKeys("rhg") + .Perform(); + + var clickedMenuItem = _session.FindElementByAccessibilityId("ClickedMenuItem"); + Assert.Equal("_Grandchild", clickedMenuItem.Text); + } + [PlatformFact(SkipOnOSX = true)] public void Child_AcceleratorKey() { var rootMenuItem = _session.FindElementByAccessibilityId("RootMenuItem"); - + rootMenuItem.SendClick(); var childMenuItem = _session.FindElementByAccessibilityId("Child1MenuItem"); From 5f51c99c13759656a8fb5b3f584a35bf973de171 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sat, 9 Apr 2022 02:44:45 +0200 Subject: [PATCH 3/7] Don't steal focus when hovering menu bar. Don't focus the menu item when simply hovering over a top-level menu item. The item should only be focused when the containing menu is open. Fixes #7906 --- src/Avalonia.Controls/MenuItem.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Avalonia.Controls/MenuItem.cs b/src/Avalonia.Controls/MenuItem.cs index c03c6000d8..955af8888b 100644 --- a/src/Avalonia.Controls/MenuItem.cs +++ b/src/Avalonia.Controls/MenuItem.cs @@ -644,7 +644,9 @@ namespace Avalonia.Controls /// The property change event. private void IsSelectedChanged(AvaloniaPropertyChangedEventArgs e) { - if ((bool)e.NewValue!) + var parentMenu = Parent as Menu; + + if ((bool)e.NewValue! && (parentMenu is null || parentMenu.IsOpen)) { Focus(); } From 909e9c57da089652bc15be4adda8f00cb1e06be7 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sat, 9 Apr 2022 11:40:13 +0200 Subject: [PATCH 4/7] Add failing menu tests for click + arrow select. --- .../MenuTests.cs | 36 ++++++++++++++++--- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/tests/Avalonia.IntegrationTests.Appium/MenuTests.cs b/tests/Avalonia.IntegrationTests.Appium/MenuTests.cs index 763ad5f400..777ecd7e2b 100644 --- a/tests/Avalonia.IntegrationTests.Appium/MenuTests.cs +++ b/tests/Avalonia.IntegrationTests.Appium/MenuTests.cs @@ -58,7 +58,7 @@ namespace Avalonia.IntegrationTests.Appium } [PlatformFact(SkipOnOSX = true)] - public void Select_Child_With_Arrow_Keys() + public void Select_Child_With_Alt_Arrow_Keys() { new Actions(_session) .KeyDown(Keys.Alt).KeyUp(Keys.Alt) @@ -70,7 +70,7 @@ namespace Avalonia.IntegrationTests.Appium } [PlatformFact(SkipOnOSX = true)] - public void Select_Grandchild_With_Arrow_Keys() + public void Select_Grandchild_With_Alt_Arrow_Keys() { new Actions(_session) .KeyDown(Keys.Alt).KeyUp(Keys.Alt) @@ -82,7 +82,7 @@ namespace Avalonia.IntegrationTests.Appium } [PlatformFact(SkipOnOSX = true)] - public void Select_Child_With_Access_Keys() + public void Select_Child_With_Alt_Access_Keys() { new Actions(_session) .KeyDown(Keys.Alt).KeyUp(Keys.Alt) @@ -94,7 +94,7 @@ namespace Avalonia.IntegrationTests.Appium } [PlatformFact(SkipOnOSX = true)] - public void Select_Grandchild_With_Access_Keys() + public void Select_Grandchild_With_Alt_Access_Keys() { new Actions(_session) .KeyDown(Keys.Alt).KeyUp(Keys.Alt) @@ -105,6 +105,34 @@ namespace Avalonia.IntegrationTests.Appium Assert.Equal("_Grandchild", clickedMenuItem.Text); } + [PlatformFact(SkipOnOSX = true)] + public void Select_Child_With_Click_Arrow_Keys() + { + var rootMenuItem = _session.FindElementByAccessibilityId("RootMenuItem"); + rootMenuItem.SendClick(); + + new Actions(_session) + .SendKeys(Keys.Down + Keys.Enter) + .Perform(); + + var clickedMenuItem = _session.FindElementByAccessibilityId("ClickedMenuItem"); + Assert.Equal("_Child 1", clickedMenuItem.Text); + } + + [PlatformFact(SkipOnOSX = true)] + public void Select_Grandchild_With_Click_Arrow_Keys() + { + var rootMenuItem = _session.FindElementByAccessibilityId("RootMenuItem"); + rootMenuItem.SendClick(); + + new Actions(_session) + .SendKeys(Keys.Down + Keys.Down + Keys.Right + Keys.Enter) + .Perform(); + + var clickedMenuItem = _session.FindElementByAccessibilityId("ClickedMenuItem"); + Assert.Equal("_Grandchild", clickedMenuItem.Text); + } + [PlatformFact(SkipOnOSX = true)] public void Child_AcceleratorKey() { From bbf7bee56a8d050428a4685c0cda43d480439901 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sat, 9 Apr 2022 12:01:31 +0200 Subject: [PATCH 5/7] Fix unopened toplevel menu selection with down key. --- .../Platform/DefaultMenuInteractionHandler.cs | 11 ++++++++--- .../Platform/DefaultMenuInteractionHandlerTests.cs | 13 +++++++++++++ 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/src/Avalonia.Controls/Platform/DefaultMenuInteractionHandler.cs b/src/Avalonia.Controls/Platform/DefaultMenuInteractionHandler.cs index 23ecdb2e7b..150b4f0792 100644 --- a/src/Avalonia.Controls/Platform/DefaultMenuInteractionHandler.cs +++ b/src/Avalonia.Controls/Platform/DefaultMenuInteractionHandler.cs @@ -149,13 +149,18 @@ namespace Avalonia.Controls.Platform case Key.Up: case Key.Down: { - if (item?.IsTopLevel == true) + if (item?.IsTopLevel == true && item.HasSubMenu) { - if (item.HasSubMenu && !item.IsSubMenuOpen) + if (!item.IsSubMenuOpen) { Open(item, true); - e.Handled = true; } + else + { + item.MoveSelection(NavigationDirection.First, true); + } + + e.Handled = true; } else { diff --git a/tests/Avalonia.Controls.UnitTests/Platform/DefaultMenuInteractionHandlerTests.cs b/tests/Avalonia.Controls.UnitTests/Platform/DefaultMenuInteractionHandlerTests.cs index 9eedd17716..d22a67f389 100644 --- a/tests/Avalonia.Controls.UnitTests/Platform/DefaultMenuInteractionHandlerTests.cs +++ b/tests/Avalonia.Controls.UnitTests/Platform/DefaultMenuInteractionHandlerTests.cs @@ -53,6 +53,19 @@ namespace Avalonia.Controls.UnitTests.Platform Assert.True(e.Handled); } + [Fact] + public void Down_Selects_First_Item_Of_Already_Opened_Submenu() + { + var target = new DefaultMenuInteractionHandler(false); + var item = Mock.Of(x => x.IsTopLevel == true && x.HasSubMenu == true && x.IsSubMenuOpen); + var e = new KeyEventArgs { Key = Key.Down, Source = item }; + + target.KeyDown(item, e); + + Mock.Get(item).Verify(x => x.MoveSelection(NavigationDirection.First, true)); + Assert.True(e.Handled); + } + [Fact] public void Right_Selects_Next_MenuItem() { From 20d50b348a41f06e9989999f7309901e442f54ea Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sat, 9 Apr 2022 12:07:13 +0200 Subject: [PATCH 6/7] Don't close and re-open menu if selection unchanged. --- .../Platform/DefaultMenuInteractionHandler.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Avalonia.Controls/Platform/DefaultMenuInteractionHandler.cs b/src/Avalonia.Controls/Platform/DefaultMenuInteractionHandler.cs index 150b4f0792..6e9ac537f1 100644 --- a/src/Avalonia.Controls/Platform/DefaultMenuInteractionHandler.cs +++ b/src/Avalonia.Controls/Platform/DefaultMenuInteractionHandler.cs @@ -252,7 +252,8 @@ namespace Avalonia.Controls.Platform // new menu. if (item.IsSubMenuOpen && item.Parent is IMenu && - item.Parent.SelectedItem is object) + item.Parent.SelectedItem is object && + item.Parent.SelectedItem != item) { item.Close(); Open(item.Parent.SelectedItem, true); From 898b0fa2857833e74736fadb033fd7d442ac227d Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sat, 9 Apr 2022 14:00:17 +0200 Subject: [PATCH 7/7] Disable focus test on mac. Because I can't work out how to implement the extremely advanced use-case of finding out whether a control is focused there /s --- .../ElementExtensions.cs | 12 ++++++++++-- tests/Avalonia.IntegrationTests.Appium/MenuTests.cs | 2 +- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/tests/Avalonia.IntegrationTests.Appium/ElementExtensions.cs b/tests/Avalonia.IntegrationTests.Appium/ElementExtensions.cs index 290811d2a1..3eb8646835 100644 --- a/tests/Avalonia.IntegrationTests.Appium/ElementExtensions.cs +++ b/tests/Avalonia.IntegrationTests.Appium/ElementExtensions.cs @@ -31,8 +31,16 @@ namespace Avalonia.IntegrationTests.Appium public static bool GetIsFocused(this AppiumWebElement element) { - var active = element.WrappedDriver.SwitchTo().ActiveElement() as AppiumWebElement; - return element.Id == active?.Id; + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + var active = element.WrappedDriver.SwitchTo().ActiveElement() as AppiumWebElement; + return element.Id == active?.Id; + } + else + { + // https://stackoverflow.com/questions/71807788/check-if-element-is-focused-in-appium + throw new NotSupportedException("Couldn't work out how to check if an element is focused on mac."); + } } public static void SendClick(this AppiumWebElement element) diff --git a/tests/Avalonia.IntegrationTests.Appium/MenuTests.cs b/tests/Avalonia.IntegrationTests.Appium/MenuTests.cs index 777ecd7e2b..98fb335061 100644 --- a/tests/Avalonia.IntegrationTests.Appium/MenuTests.cs +++ b/tests/Avalonia.IntegrationTests.Appium/MenuTests.cs @@ -145,7 +145,7 @@ namespace Avalonia.IntegrationTests.Appium Assert.Equal("Ctrl+O", childMenuItem.GetAttribute("AcceleratorKey")); } - [Fact] + [PlatformFact(SkipOnOSX = true)] public void PointerOver_Does_Not_Steal_Focus() { // Issue #7906