Skip to content

Commit

Permalink
Add RadioMenuFlyoutSubItemStyle (#4865)
Browse files Browse the repository at this point in the history 
* Partly working, that's always nice.

* whoa it works now

* Rename attached property, cleanup, fix the UI to match other items.

* Make sure icons also still work.

* Some cleanup.

* Added test, PR comments.

* Disabled failing tests, fixed leaking menu items

* Update tag in idl

* Update RadioMenuFlyoutItem.idl

Add new APIs to MUX_PUBLIC_V2 attribute

Co-authored-by: Ranjesh <[email protected]>
  • Loading branch information
@teaP @ranjeshj
teaP and ranjeshj authored Apr 30, 2021
1 parent 05e22b0 commit 2adf00c
Show file tree
Hide file tree
Showing 9 changed files with 376 additions and 49 deletions.
24 changes: 24 additions & 0 deletions dev/Generated/RadioMenuFlyoutItem.properties.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ namespace winrt::Microsoft::UI::Xaml::Controls

#include "RadioMenuFlyoutItem.g.cpp"

GlobalDependencyProperty RadioMenuFlyoutItemProperties::s_AreCheckStatesEnabledProperty{ nullptr };
GlobalDependencyProperty RadioMenuFlyoutItemProperties::s_GroupNameProperty{ nullptr };
GlobalDependencyProperty RadioMenuFlyoutItemProperties::s_IsCheckedProperty{ nullptr };

Expand All @@ -23,6 +24,17 @@ RadioMenuFlyoutItemProperties::RadioMenuFlyoutItemProperties()

void RadioMenuFlyoutItemProperties::EnsureProperties()
{
if (!s_AreCheckStatesEnabledProperty)
{
s_AreCheckStatesEnabledProperty =
InitializeDependencyProperty(
L"AreCheckStatesEnabled",
winrt::name_of<bool>(),
winrt::name_of<winrt::RadioMenuFlyoutItem>(),
true /* isAttached */,
ValueHelper<bool>::BoxValueIfNecessary(false),
&RadioMenuFlyoutItem::OnAreCheckStatesEnabledPropertyChanged);
}
if (!s_GroupNameProperty)
{
s_GroupNameProperty =
Expand All @@ -49,6 +61,7 @@ void RadioMenuFlyoutItemProperties::EnsureProperties()

void RadioMenuFlyoutItemProperties::ClearProperties()
{
s_AreCheckStatesEnabledProperty = nullptr;
s_GroupNameProperty = nullptr;
s_IsCheckedProperty = nullptr;
}
Expand All @@ -69,6 +82,17 @@ void RadioMenuFlyoutItemProperties::OnIsCheckedPropertyChanged(
winrt::get_self<RadioMenuFlyoutItem>(owner)->OnPropertyChanged(args);
}


void RadioMenuFlyoutItemProperties::SetAreCheckStatesEnabled(winrt::MenuFlyoutSubItem const& target, bool value)
{
target.SetValue(s_AreCheckStatesEnabledProperty, ValueHelper<bool>::BoxValueIfNecessary(value));
}

bool RadioMenuFlyoutItemProperties::GetAreCheckStatesEnabled(winrt::MenuFlyoutSubItem const& target)
{
return ValueHelper<bool>::CastOrUnbox(target.GetValue(s_AreCheckStatesEnabledProperty));
}

void RadioMenuFlyoutItemProperties::GroupName(winrt::hstring const& value)
{
[[gsl::suppress(con)]]
Expand Down
5 changes: 5 additions & 0 deletions dev/Generated/RadioMenuFlyoutItem.properties.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,20 @@ class RadioMenuFlyoutItemProperties
public:
RadioMenuFlyoutItemProperties();

static void SetAreCheckStatesEnabled(winrt::MenuFlyoutSubItem const& target, bool value);
static bool GetAreCheckStatesEnabled(winrt::MenuFlyoutSubItem const& target);

void GroupName(winrt::hstring const& value);
winrt::hstring GroupName();

void IsChecked(bool value);
bool IsChecked();

static winrt::DependencyProperty AreCheckStatesEnabledProperty() { return s_AreCheckStatesEnabledProperty; }
static winrt::DependencyProperty GroupNameProperty() { return s_GroupNameProperty; }
static winrt::DependencyProperty IsCheckedProperty() { return s_IsCheckedProperty; }

static GlobalDependencyProperty s_AreCheckStatesEnabledProperty;
static GlobalDependencyProperty s_GroupNameProperty;
static GlobalDependencyProperty s_IsCheckedProperty;

Expand Down
72 changes: 57 additions & 15 deletions dev/RadioMenuFlyoutItem/InteractionTests/RadioMenuFlyoutItemTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,46 +56,88 @@ public void TestCleanup()
};

[TestMethod]
[TestProperty("Ignore", "True")] // Disabled as per tracking issue #3125 and internal issue 19603059
public void BasicTest()
{
if (PlatformConfiguration.IsOSVersionLessThan(OSVersion.Redstone3))
{
Log.Warning("Test is disabled on RS2");
return;
}

using (var setup = new TestSetupHelper("RadioMenuFlyoutItem Tests"))
{
Log.Comment("Verify initial states");
VerifySelectedItems("Orange", "Compact");
VerifySelectedItems("Orange", "Compact", "Name");

InvokeItem("Yellow");
VerifySelectedItems("Yellow", "Compact");
InvokeItem("FlyoutButton", "YellowItem");
VerifySelectedItems("Yellow", "Compact", "Name");

InvokeItem("Expanded");
VerifySelectedItems("Yellow", "Expanded");
InvokeItem("FlyoutButton", "ExpandedItem");
VerifySelectedItems("Yellow", "Expanded", "Name");

Log.Comment("Verify you can't uncheck an item");
InvokeItem("Yellow");
VerifySelectedItems("Yellow", "Expanded");
InvokeItem("FlyoutButton", "YellowItem");
VerifySelectedItems("Yellow", "Expanded", "Name");
}
}

public void InvokeItem(string item)
[TestMethod]
public void SubMenuTest()
{
Log.Comment("Open flyout");
Button flyoutButton = FindElement.ByName<Button>("FlyoutButton");
if (PlatformConfiguration.IsOSVersionLessThan(OSVersion.Redstone3))
{
Log.Warning("Test is disabled on RS2");
return;
}

using (var setup = new TestSetupHelper("RadioMenuFlyoutItem Tests"))
{
InvokeSubItem("SubMenuFlyoutButton", "RadioSubMenu", "ArtistNameItem");
VerifySelectedItems("Orange", "Compact", "ArtistName");

InvokeItem("SubMenuFlyoutButton", "DateItem");
VerifySelectedItems("Orange", "Compact", "Date");
}
}

public void InvokeItem(string flyoutButtonName, string itemName)
{
Log.Comment("Open flyout by clicking " + flyoutButtonName);
Button flyoutButton = FindElement.ByName<Button>(flyoutButtonName);
flyoutButton.Invoke();
Wait.ForIdle();

Log.Comment("Invoke item: " + itemName);
MenuItem menuItem = FindElement.ByName<MenuItem>(itemName);
menuItem.Click();
Wait.ForIdle();
}

public void InvokeSubItem(string flyoutButtonName, string menuName, string itemName)
{
Log.Comment("Open flyout by clicking " + flyoutButtonName);
Button flyoutButton = FindElement.ByName<Button>(flyoutButtonName);
flyoutButton.Invoke();
Wait.ForIdle();

Log.Comment("Invoke item: " + item);
MenuItem menuItem = FindElement.ByName<MenuItem>(item + "Item");
Log.Comment("Open Menu: " + menuName);
MenuItem menuItem = FindElement.ByName<MenuItem>(menuName);
menuItem.Click();
Wait.ForIdle();

Log.Comment("Invoke item: " + itemName);
menuItem = FindElement.ByName<MenuItem>(itemName);
menuItem.Click();
Wait.ForIdle();
}

public void VerifySelectedItems(string item1, string item2)
public void VerifySelectedItems(string item1, string item2, string item3)
{
foreach (string item in Items)
{
TextBlock itemState = FindElement.ByName<TextBlock>(item + "State");

if (item == item1 || item == item2)
if (item == item1 || item == item2 || item == item3)
{
Verify.AreEqual(itemState.DocumentText, "Checked", "Verify " + item + " is checked");
}
Expand Down
68 changes: 54 additions & 14 deletions dev/RadioMenuFlyoutItem/RadioMenuFlyoutItem.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,32 @@
#include "RuntimeProfiler.h"
#include "ResourceAccessor.h"

thread_local std::unique_ptr<std::map<winrt::hstring, winrt::weak_ref<RadioMenuFlyoutItem>>> RadioMenuFlyoutItem::s_selectionMap;

RadioMenuFlyoutItem::RadioMenuFlyoutItem()
{
__RP_Marker_ClassById(RuntimeProfiler::ProfId_RadioMenuFlyoutItem);

m_InternalIsCheckedChangedRevoker = RegisterPropertyChanged(*this, winrt::ToggleMenuFlyoutItem::IsCheckedProperty(), { this, &RadioMenuFlyoutItem::OnInternalIsCheckedChanged });

if (!s_selectionMap)
{
// Ensure that this object exists
s_selectionMap = std::make_unique<std::map<winrt::hstring, winrt::weak_ref<RadioMenuFlyoutItem>>>();
}

SetDefaultStyleKey(this);
}

RadioMenuFlyoutItem::~RadioMenuFlyoutItem()
{
// If this is the checked item, remove it from the lookup.
if (IsChecked())
{
SharedHelpers::EraseIfExists(*s_selectionMap, GroupName());
}
}

void RadioMenuFlyoutItem::OnPropertyChanged(const winrt::DependencyPropertyChangedEventArgs& args)
{
winrt::IDependencyProperty property = args.Property();
Expand All @@ -27,7 +44,7 @@ void RadioMenuFlyoutItem::OnPropertyChanged(const winrt::DependencyPropertyChang
m_isSafeUncheck = true;
InternalIsChecked(IsChecked());
m_isSafeUncheck = false;
UpdateSiblings();
UpdateCheckedItemInGroup();
}
}
}
Expand All @@ -50,31 +67,54 @@ void RadioMenuFlyoutItem::OnInternalIsCheckedChanged(const winrt::DependencyObje
else if (!IsChecked())
{
IsChecked(true);
UpdateSiblings();
UpdateCheckedItemInGroup();
}
}

void RadioMenuFlyoutItem::UpdateSiblings()
void RadioMenuFlyoutItem::UpdateCheckedItemInGroup()
{
if (IsChecked())
{
// Since this item is checked, uncheck all siblings
if (auto parent = winrt::VisualTreeHelper::GetParent(*this))
const auto groupName = GroupName();

if (const auto previousCheckedItemWeak = (*s_selectionMap)[groupName])
{
const int childrenCount = winrt::VisualTreeHelper::GetChildrenCount(parent);
for (int i = 0; i < childrenCount; i++)
if (auto previousCheckedItem = previousCheckedItemWeak.get())
{
auto child = winrt::VisualTreeHelper::GetChild(parent, i);
if (auto radioItem = child.try_as<winrt::RadioMenuFlyoutItem>())
// Uncheck the previously checked item.
previousCheckedItem->IsChecked(false);
}
}
(*s_selectionMap)[groupName] = this->get_weak();
}
}

void RadioMenuFlyoutItem::OnAreCheckStatesEnabledPropertyChanged(const winrt::DependencyObject& sender, const winrt::DependencyPropertyChangedEventArgs& args)
{
if (unbox_value<bool>(args.NewValue()))
{
if (auto const& subMenu = sender.try_as<winrt::MenuFlyoutSubItem>())
{
// Every time the submenu is loaded, see if it contains a checked RadioMenuFlyoutItem or not.
subMenu.Loaded(
{
[subMenuWeak = winrt::make_weak(subMenu)](winrt::IInspectable const& sender, auto const&)
{
if (winrt::get_self<RadioMenuFlyoutItem>(radioItem) != this
&& radioItem.GroupName() == GroupName())
if (auto subMenu = subMenuWeak.get())
{
radioItem.IsChecked(false);
bool isAnyItemChecked = false;
for (auto const& item : subMenu.Items())
{
if (auto const& radioItem = item.try_as<winrt::RadioMenuFlyoutItem>())
{
isAnyItemChecked = isAnyItemChecked || radioItem.IsChecked();
}
}

winrt::VisualStateManager::GoToState(subMenu, isAnyItemChecked ? L"Checked" : L"Unchecked", false);
}
}
}
});
}

}
}
7 changes: 6 additions & 1 deletion dev/RadioMenuFlyoutItem/RadioMenuFlyoutItem.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,18 +45,23 @@ class RadioMenuFlyoutItem :

public:
RadioMenuFlyoutItem();
~RadioMenuFlyoutItem();

// IsChecked property is ambiguous with ToggleMenuFlyoutItem, lift up RadioMenuFlyoutItem::IsChecked to disambiguate.
using RadioMenuFlyoutItemProperties::IsChecked;

void OnPropertyChanged(const winrt::DependencyPropertyChangedEventArgs& args);

static void OnAreCheckStatesEnabledPropertyChanged(const winrt::DependencyObject& sender, const winrt::DependencyPropertyChangedEventArgs& args);

private:
void OnInternalIsCheckedChanged(const winrt::DependencyObject& sender, const winrt::DependencyProperty& args);

void UpdateSiblings();
void UpdateCheckedItemInGroup();

bool m_isSafeUncheck{ false };

PropertyChanged_revoker m_InternalIsCheckedChangedRevoker{};

static thread_local std::unique_ptr<std::map<winrt::hstring, winrt::weak_ref<RadioMenuFlyoutItem>>> s_selectionMap;
};
11 changes: 10 additions & 1 deletion dev/RadioMenuFlyoutItem/RadioMenuFlyoutItem.idl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,15 @@ unsealed runtimeclass RadioMenuFlyoutItem : Windows.UI.Xaml.Controls.MenuFlyoutI

static Windows.UI.Xaml.DependencyProperty IsCheckedProperty{ get; };
static Windows.UI.Xaml.DependencyProperty GroupNameProperty{ get; };

[MUX_PUBLIC_V2]
{
[MUX_DEFAULT_VALUE("false")]
[MUX_PROPERTY_CHANGED_CALLBACK_METHODNAME("OnAreCheckStatesEnabledPropertyChanged")]
static Windows.UI.Xaml.DependencyProperty AreCheckStatesEnabledProperty{ get; };
static void SetAreCheckStatesEnabled(Windows.UI.Xaml.Controls.MenuFlyoutSubItem object, Boolean value);
static Boolean GetAreCheckStatesEnabled(Windows.UI.Xaml.Controls.MenuFlyoutSubItem object);
}
}

}
}
Loading

0 comments on commit 2adf00c

Please sign in to comment.