Skip to content

Commit

Permalink
Fix issue with hovering over MenuBarItem not working on items added t…
Browse files Browse the repository at this point in the history
…hrough code behind (microsoft#2025)

* Add test UI

* Fix issue with manually added MenuBarItems not detecting hover correctly

* CR feedback

* Add interaction test

* Update interaction test

* Update hover behavior test - Add1 element detection code

* Skip HoveringBehaviorTest for RS4

* Add link to tracking issue
  • Loading branch information
marcelwgn authored Mar 20, 2020
1 parent 4409667 commit dd67c1d
Show file tree
Hide file tree
Showing 6 changed files with 93 additions and 10 deletions.
11 changes: 6 additions & 5 deletions dev/MenuBar/MenuBar.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,6 @@ winrt::AutomationPeer MenuBar::OnCreateAutomationPeer()
void MenuBar::OnApplyTemplate()
{
SetUpTemplateParts();

for (auto const& menuBarItem : Items())
{
winrt::get_self<MenuBarItem>(menuBarItem)->AddPassThroughElement(m_layoutRoot.get());
}
}

void MenuBar::SetUpTemplateParts()
Expand All @@ -54,6 +49,12 @@ void MenuBar::SetUpTemplateParts()
}
}

void MenuBar::RequestPassThroughElement(const winrt::Microsoft::UI::Xaml::Controls::MenuBarItem& menuBarItem)
{
// To enable switching flyout on hover, every menubar item needs the MenuBar root to include it for hit detection with flyouts open
winrt::get_self<MenuBarItem>(menuBarItem)->AddPassThroughElement(m_layoutRoot.get());
}

void MenuBar::IsFlyoutOpen(bool state)
{
m_isFlyoutOpen = state;
Expand Down
2 changes: 2 additions & 0 deletions dev/MenuBar/MenuBar.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ class MenuBar :
bool IsFlyoutOpen() { return m_isFlyoutOpen; };
void IsFlyoutOpen(bool state);

void RequestPassThroughElement(const winrt::Microsoft::UI::Xaml::Controls::MenuBarItem& menuBarItem);

public:
// IUIElement / IUIElementOverridesHelper
winrt::AutomationPeer OnCreateAutomationPeer();
Expand Down
10 changes: 6 additions & 4 deletions dev/MenuBar/MenuBarItem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,17 @@ void MenuBarItem::OnApplyTemplate()
{
m_button.set(GetTemplateChildT<winrt::Button>(L"ContentButton", *this));

PopulateContent();
DetachEventHandlers();
AttachEventHandlers();

auto menuBar = SharedHelpers::GetAncestorOfType<winrt::MenuBar>(winrt::VisualTreeHelper::GetParent(*this));
if (menuBar)
{
m_menuBar = winrt::make_weak(menuBar);
// Ask parent MenuBar for its root to enable pass through
winrt::get_self<MenuBar>(menuBar)->RequestPassThroughElement(*this);
}

PopulateContent();
DetachEventHandlers();
AttachEventHandlers();
}

void MenuBarItem::PopulateContent()
Expand Down
55 changes: 55 additions & 0 deletions dev/MenuBar/MenuBar_InteractionTests/MenuBarTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,49 @@ public void MenuBarHeightTest()
}
}

[TestMethod]
public void HoveringBehaviorTest()
{
// Overlay pass through element is only available from IFlyoutBase3 forward
// On OS versions below RS5 test is unreliable/not working.
// Tracked by https://github.com/Microsoft/microsoft-ui-xaml/issues/115
if (PlatformConfiguration.IsDevice(DeviceType.Phone)
|| !ApiInformation.IsTypePresent("Windows.UI.Xaml.Controls.Primitives.IFlyoutBase3")
|| PlatformConfiguration.IsOSVersionLessThan(OSVersion.Redstone4))
{
Log.Comment("Skipping tests on phone, because menubar is not supported.");
return;
}
using (var setup = new TestSetupHelper("MenuBar Tests"))
{
var menuBar = FindElement.ById("SizedMenuBar");
var addButton = FindElement.ByName("AddItemsToEmptyMenuBar");

addButton.Click();
addButton.Click();
addButton.Click();

var help0 = FindElement.ByName<Button>("Help0");
var help1 = FindElement.ByName<Button>("Help1");

// This behavior seems to a bit unreliable, so repeat
InputHelper.LeftClick(help0);
TestEnvironment.VerifyAreEqualWithRetry(20,
() => FindCore.ByName("Add0", shouldWait: false) != null, // The item should be in the tree
() => true);

// Check if hovering over the next button actually will show the correct item
VerifyElement.NotFound("Add1", FindBy.Name);
InputHelper.MoveMouse(help1, 0, 0);
InputHelper.MoveMouse(help1, 1, 1);
InputHelper.MoveMouse(help1, 5, 5);

UIObject add1Element = null;
ElementCache.Clear();
var element = GetElement(ref add1Element, "Add1");
Verify.IsNotNull(add1Element);
}
}

[TestMethod]
public void TabTest()
Expand All @@ -331,5 +374,17 @@ public void TabTest()
Verify.AreEqual(true, fileButton.HasKeyboardFocus);
}
}


private T GetElement<T>(ref T element, string elementName) where T : UIObject
{
if (element == null)
{
Log.Comment("Find the " + elementName);
element = FindElement.ByNameOrId<T>(elementName);
Verify.IsNotNull(element);
}
return element;
}
}
}
8 changes: 7 additions & 1 deletion dev/MenuBar/MenuBar_TestUI/MenuBarPage.xaml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
<Button AutomationProperties.Name="FirstButton" Content="Element to test that focus moves correctly"/>

<TextBlock Text="Sample controls" Style="{ThemeResource StandardGroupHeader}"/>
<TextBlock Text="Sample menu bar"/>
<muxc:MenuBar x:Name="menuBar">
<muxc:MenuBarItem x:Name="FileItem" AutomationProperties.Name="FileItem" Title="File">
<MenuFlyoutItem x:Name="NewItem" AutomationProperties.Name="NewItem" Text="New" Click="NewItem_Click"/>
Expand Down Expand Up @@ -55,10 +56,14 @@
<MenuFlyoutItem Text="Font..."/>
</muxc:MenuBarItem>
</muxc:MenuBar>


<TextBlock Text="Custom sized menu bar" Margin="0,6,0,0"/>
<muxc:MenuBar x:Name="SizedMenuBar" AutomationProperties.Name="SizedMenuBar" Height="24">
<muxc:MenuBarItem x:Name="SizedMenuBarItem" AutomationProperties.Name="SizedMenuBarItem" Title="Size"/>
</muxc:MenuBar>

<TextBlock Text="Initially empty menubar" Margin="0,6,0,0"/>
<muxc:MenuBar x:Name="EmptyMenuBar"></muxc:MenuBar>
</StackPanel>
<TextBlock Grid.Row="1" Text="Options" Style="{ThemeResource StandardGroupHeader}"/>
<StackPanel Margin="0,20,0,20" VerticalAlignment="Top" Grid.Row="2" Orientation="Horizontal">
Expand All @@ -72,6 +77,7 @@
<Button x:Name="RemoveMenuBarItemButton" AutomationProperties.Name="RemoveMenuBarItemButton" Click="RemoveMenuBarItem_Click" Content="Remove Menu Bar Item"/>
<Button x:Name="AddFlyoutItemButton" AutomationProperties.Name="AddFlyoutItemButton" Click="AddFlyoutItem_Click" Content="Add Flyout Item"/>
<Button x:Name="RemoveFlyoutItemButton" AutomationProperties.Name="RemoveFlyoutItemButton" Click="RemoveFlyoutItem_Click" Content="Remove Flyout Item"/>
<Button x:Name="AddItemsToEmptyMenuBar" AutomationProperties.Name="AddItemsToEmptyMenuBar" Click="AddMenuBarToEmptyMenuBarItem_Click" Content="Add a menu item to the empty menu bar"/>
</StackPanel>
<StackPanel Grid.Row="3">
<TextBlock Text="Test output" Style="{ThemeResource StandardGroupHeader}"/>
Expand Down
17 changes: 17 additions & 0 deletions dev/MenuBar/MenuBar_TestUI/MenuBarPage.xaml.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using Windows.Foundation.Metadata;
using Windows.UI.Xaml;
using Windows.UI.Xaml.Automation;
using Windows.UI.Xaml.Controls;
using Windows.UI.Xaml.Input;

Expand Down Expand Up @@ -91,5 +92,21 @@ private void TestFrameCheckbox_Unchecked(object sender, RoutedEventArgs e)
var testFrame = Window.Current.Content as TestFrame;
testFrame.ChangeBarVisibility(Visibility.Collapsed);
}

private void AddMenuBarToEmptyMenuBarItem_Click(object sender, RoutedEventArgs e)
{
int eCount = EmptyMenuBar.Items.Count;
MenuBarItem mainMenuBarHelp = new MenuBarItem();
mainMenuBarHelp.Title = "Help" + eCount;
mainMenuBarHelp.SetValue(AutomationProperties.NameProperty, "Help" + eCount);
MenuFlyoutItem newFlyout = new MenuFlyoutItem() { Text = "Add" + eCount };
// UIA Name for interaction test
newFlyout.SetValue(AutomationProperties.NameProperty, "Add" + eCount);
mainMenuBarHelp.Items.Add(newFlyout);

mainMenuBarHelp.Items.Add(new MenuFlyoutItem() { Text = "Remove" + eCount });
EmptyMenuBar.Items.Add(mainMenuBarHelp);
}

}
}

0 comments on commit dd67c1d

Please sign in to comment.