Skip to content

Commit 2adf00c

Browse files
teaPranjeshj
teaP
and
ranjeshj
authored
Add RadioMenuFlyoutSubItemStyle (microsoft#4865)
* 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 <28935693+ranjeshj@users.noreply.github.com>
1 parent 05e22b0 commit 2adf00c

9 files changed

+376
-49
lines changed

‎dev/Generated/RadioMenuFlyoutItem.properties.cpp

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ namespace winrt::Microsoft::UI::Xaml::Controls
1313

1414
#include "RadioMenuFlyoutItem.g.cpp"
1515

16+
GlobalDependencyProperty RadioMenuFlyoutItemProperties::s_AreCheckStatesEnabledProperty{ nullptr };
1617
GlobalDependencyProperty RadioMenuFlyoutItemProperties::s_GroupNameProperty{ nullptr };
1718
GlobalDependencyProperty RadioMenuFlyoutItemProperties::s_IsCheckedProperty{ nullptr };
1819

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

2425
void RadioMenuFlyoutItemProperties::EnsureProperties()
2526
{
27+
if (!s_AreCheckStatesEnabledProperty)
28+
{
29+
s_AreCheckStatesEnabledProperty =
30+
InitializeDependencyProperty(
31+
L"AreCheckStatesEnabled",
32+
winrt::name_of<bool>(),
33+
winrt::name_of<winrt::RadioMenuFlyoutItem>(),
34+
true /* isAttached */,
35+
ValueHelper<bool>::BoxValueIfNecessary(false),
36+
&RadioMenuFlyoutItem::OnAreCheckStatesEnabledPropertyChanged);
37+
}
2638
if (!s_GroupNameProperty)
2739
{
2840
s_GroupNameProperty =
@@ -49,6 +61,7 @@ void RadioMenuFlyoutItemProperties::EnsureProperties()
4961

5062
void RadioMenuFlyoutItemProperties::ClearProperties()
5163
{
64+
s_AreCheckStatesEnabledProperty = nullptr;
5265
s_GroupNameProperty = nullptr;
5366
s_IsCheckedProperty = nullptr;
5467
}
@@ -69,6 +82,17 @@ void RadioMenuFlyoutItemProperties::OnIsCheckedPropertyChanged(
6982
winrt::get_self<RadioMenuFlyoutItem>(owner)->OnPropertyChanged(args);
7083
}
7184

85+
86+
void RadioMenuFlyoutItemProperties::SetAreCheckStatesEnabled(winrt::MenuFlyoutSubItem const& target, bool value)
87+
{
88+
target.SetValue(s_AreCheckStatesEnabledProperty, ValueHelper<bool>::BoxValueIfNecessary(value));
89+
}
90+
91+
bool RadioMenuFlyoutItemProperties::GetAreCheckStatesEnabled(winrt::MenuFlyoutSubItem const& target)
92+
{
93+
return ValueHelper<bool>::CastOrUnbox(target.GetValue(s_AreCheckStatesEnabledProperty));
94+
}
95+
7296
void RadioMenuFlyoutItemProperties::GroupName(winrt::hstring const& value)
7397
{
7498
[[gsl::suppress(con)]]

‎dev/Generated/RadioMenuFlyoutItem.properties.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,20 @@ class RadioMenuFlyoutItemProperties
99
public:
1010
RadioMenuFlyoutItemProperties();
1111

12+
static void SetAreCheckStatesEnabled(winrt::MenuFlyoutSubItem const& target, bool value);
13+
static bool GetAreCheckStatesEnabled(winrt::MenuFlyoutSubItem const& target);
14+
1215
void GroupName(winrt::hstring const& value);
1316
winrt::hstring GroupName();
1417

1518
void IsChecked(bool value);
1619
bool IsChecked();
1720

21+
static winrt::DependencyProperty AreCheckStatesEnabledProperty() { return s_AreCheckStatesEnabledProperty; }
1822
static winrt::DependencyProperty GroupNameProperty() { return s_GroupNameProperty; }
1923
static winrt::DependencyProperty IsCheckedProperty() { return s_IsCheckedProperty; }
2024

25+
static GlobalDependencyProperty s_AreCheckStatesEnabledProperty;
2126
static GlobalDependencyProperty s_GroupNameProperty;
2227
static GlobalDependencyProperty s_IsCheckedProperty;
2328

‎dev/RadioMenuFlyoutItem/InteractionTests/RadioMenuFlyoutItemTests.cs

Lines changed: 57 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -56,46 +56,88 @@ public void TestCleanup()
5656
};
5757

5858
[TestMethod]
59-
[TestProperty("Ignore", "True")] // Disabled as per tracking issue #3125 and internal issue 19603059
6059
public void BasicTest()
6160
{
61+
if (PlatformConfiguration.IsOSVersionLessThan(OSVersion.Redstone3))
62+
{
63+
Log.Warning("Test is disabled on RS2");
64+
return;
65+
}
66+
6267
using (var setup = new TestSetupHelper("RadioMenuFlyoutItem Tests"))
6368
{
6469
Log.Comment("Verify initial states");
65-
VerifySelectedItems("Orange", "Compact");
70+
VerifySelectedItems("Orange", "Compact", "Name");
6671

67-
InvokeItem("Yellow");
68-
VerifySelectedItems("Yellow", "Compact");
72+
InvokeItem("FlyoutButton", "YellowItem");
73+
VerifySelectedItems("Yellow", "Compact", "Name");
6974

70-
InvokeItem("Expanded");
71-
VerifySelectedItems("Yellow", "Expanded");
75+
InvokeItem("FlyoutButton", "ExpandedItem");
76+
VerifySelectedItems("Yellow", "Expanded", "Name");
7277

7378
Log.Comment("Verify you can't uncheck an item");
74-
InvokeItem("Yellow");
75-
VerifySelectedItems("Yellow", "Expanded");
79+
InvokeItem("FlyoutButton", "YellowItem");
80+
VerifySelectedItems("Yellow", "Expanded", "Name");
7681
}
7782
}
7883

79-
public void InvokeItem(string item)
84+
[TestMethod]
85+
public void SubMenuTest()
8086
{
81-
Log.Comment("Open flyout");
82-
Button flyoutButton = FindElement.ByName<Button>("FlyoutButton");
87+
if (PlatformConfiguration.IsOSVersionLessThan(OSVersion.Redstone3))
88+
{
89+
Log.Warning("Test is disabled on RS2");
90+
return;
91+
}
92+
93+
using (var setup = new TestSetupHelper("RadioMenuFlyoutItem Tests"))
94+
{
95+
InvokeSubItem("SubMenuFlyoutButton", "RadioSubMenu", "ArtistNameItem");
96+
VerifySelectedItems("Orange", "Compact", "ArtistName");
97+
98+
InvokeItem("SubMenuFlyoutButton", "DateItem");
99+
VerifySelectedItems("Orange", "Compact", "Date");
100+
}
101+
}
102+
103+
public void InvokeItem(string flyoutButtonName, string itemName)
104+
{
105+
Log.Comment("Open flyout by clicking " + flyoutButtonName);
106+
Button flyoutButton = FindElement.ByName<Button>(flyoutButtonName);
107+
flyoutButton.Invoke();
108+
Wait.ForIdle();
109+
110+
Log.Comment("Invoke item: " + itemName);
111+
MenuItem menuItem = FindElement.ByName<MenuItem>(itemName);
112+
menuItem.Click();
113+
Wait.ForIdle();
114+
}
115+
116+
public void InvokeSubItem(string flyoutButtonName, string menuName, string itemName)
117+
{
118+
Log.Comment("Open flyout by clicking " + flyoutButtonName);
119+
Button flyoutButton = FindElement.ByName<Button>(flyoutButtonName);
83120
flyoutButton.Invoke();
84121
Wait.ForIdle();
85122

86-
Log.Comment("Invoke item: " + item);
87-
MenuItem menuItem = FindElement.ByName<MenuItem>(item + "Item");
123+
Log.Comment("Open Menu: " + menuName);
124+
MenuItem menuItem = FindElement.ByName<MenuItem>(menuName);
125+
menuItem.Click();
126+
Wait.ForIdle();
127+
128+
Log.Comment("Invoke item: " + itemName);
129+
menuItem = FindElement.ByName<MenuItem>(itemName);
88130
menuItem.Click();
89131
Wait.ForIdle();
90132
}
91133

92-
public void VerifySelectedItems(string item1, string item2)
134+
public void VerifySelectedItems(string item1, string item2, string item3)
93135
{
94136
foreach (string item in Items)
95137
{
96138
TextBlock itemState = FindElement.ByName<TextBlock>(item + "State");
97139

98-
if (item == item1 || item == item2)
140+
if (item == item1 || item == item2 || item == item3)
99141
{
100142
Verify.AreEqual(itemState.DocumentText, "Checked", "Verify " + item + " is checked");
101143
}

‎dev/RadioMenuFlyoutItem/RadioMenuFlyoutItem.cpp

Lines changed: 54 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,32 @@
77
#include "RuntimeProfiler.h"
88
#include "ResourceAccessor.h"
99

10+
thread_local std::unique_ptr<std::map<winrt::hstring, winrt::weak_ref<RadioMenuFlyoutItem>>> RadioMenuFlyoutItem::s_selectionMap;
11+
1012
RadioMenuFlyoutItem::RadioMenuFlyoutItem()
1113
{
1214
__RP_Marker_ClassById(RuntimeProfiler::ProfId_RadioMenuFlyoutItem);
1315

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

18+
if (!s_selectionMap)
19+
{
20+
// Ensure that this object exists
21+
s_selectionMap = std::make_unique<std::map<winrt::hstring, winrt::weak_ref<RadioMenuFlyoutItem>>>();
22+
}
23+
1624
SetDefaultStyleKey(this);
1725
}
1826

27+
RadioMenuFlyoutItem::~RadioMenuFlyoutItem()
28+
{
29+
// If this is the checked item, remove it from the lookup.
30+
if (IsChecked())
31+
{
32+
SharedHelpers::EraseIfExists(*s_selectionMap, GroupName());
33+
}
34+
}
35+
1936
void RadioMenuFlyoutItem::OnPropertyChanged(const winrt::DependencyPropertyChangedEventArgs& args)
2037
{
2138
winrt::IDependencyProperty property = args.Property();
@@ -27,7 +44,7 @@ void RadioMenuFlyoutItem::OnPropertyChanged(const winrt::DependencyPropertyChang
2744
m_isSafeUncheck = true;
2845
InternalIsChecked(IsChecked());
2946
m_isSafeUncheck = false;
30-
UpdateSiblings();
47+
UpdateCheckedItemInGroup();
3148
}
3249
}
3350
}
@@ -50,31 +67,54 @@ void RadioMenuFlyoutItem::OnInternalIsCheckedChanged(const winrt::DependencyObje
5067
else if (!IsChecked())
5168
{
5269
IsChecked(true);
53-
UpdateSiblings();
70+
UpdateCheckedItemInGroup();
5471
}
5572
}
5673

57-
void RadioMenuFlyoutItem::UpdateSiblings()
74+
void RadioMenuFlyoutItem::UpdateCheckedItemInGroup()
5875
{
5976
if (IsChecked())
6077
{
61-
// Since this item is checked, uncheck all siblings
62-
if (auto parent = winrt::VisualTreeHelper::GetParent(*this))
78+
const auto groupName = GroupName();
79+
80+
if (const auto previousCheckedItemWeak = (*s_selectionMap)[groupName])
6381
{
64-
const int childrenCount = winrt::VisualTreeHelper::GetChildrenCount(parent);
65-
for (int i = 0; i < childrenCount; i++)
82+
if (auto previousCheckedItem = previousCheckedItemWeak.get())
6683
{
67-
auto child = winrt::VisualTreeHelper::GetChild(parent, i);
68-
if (auto radioItem = child.try_as<winrt::RadioMenuFlyoutItem>())
84+
// Uncheck the previously checked item.
85+
previousCheckedItem->IsChecked(false);
86+
}
87+
}
88+
(*s_selectionMap)[groupName] = this->get_weak();
89+
}
90+
}
91+
92+
void RadioMenuFlyoutItem::OnAreCheckStatesEnabledPropertyChanged(const winrt::DependencyObject& sender, const winrt::DependencyPropertyChangedEventArgs& args)
93+
{
94+
if (unbox_value<bool>(args.NewValue()))
95+
{
96+
if (auto const& subMenu = sender.try_as<winrt::MenuFlyoutSubItem>())
97+
{
98+
// Every time the submenu is loaded, see if it contains a checked RadioMenuFlyoutItem or not.
99+
subMenu.Loaded(
100+
{
101+
[subMenuWeak = winrt::make_weak(subMenu)](winrt::IInspectable const& sender, auto const&)
69102
{
70-
if (winrt::get_self<RadioMenuFlyoutItem>(radioItem) != this
71-
&& radioItem.GroupName() == GroupName())
103+
if (auto subMenu = subMenuWeak.get())
72104
{
73-
radioItem.IsChecked(false);
105+
bool isAnyItemChecked = false;
106+
for (auto const& item : subMenu.Items())
107+
{
108+
if (auto const& radioItem = item.try_as<winrt::RadioMenuFlyoutItem>())
109+
{
110+
isAnyItemChecked = isAnyItemChecked || radioItem.IsChecked();
111+
}
112+
}
113+
114+
winrt::VisualStateManager::GoToState(subMenu, isAnyItemChecked ? L"Checked" : L"Unchecked", false);
74115
}
75116
}
76-
}
117+
});
77118
}
78-
79119
}
80120
}

‎dev/RadioMenuFlyoutItem/RadioMenuFlyoutItem.h

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,18 +45,23 @@ class RadioMenuFlyoutItem :
4545

4646
public:
4747
RadioMenuFlyoutItem();
48+
~RadioMenuFlyoutItem();
4849

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

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

55+
static void OnAreCheckStatesEnabledPropertyChanged(const winrt::DependencyObject& sender, const winrt::DependencyPropertyChangedEventArgs& args);
56+
5457
private:
5558
void OnInternalIsCheckedChanged(const winrt::DependencyObject& sender, const winrt::DependencyProperty& args);
5659

57-
void UpdateSiblings();
60+
void UpdateCheckedItemInGroup();
5861

5962
bool m_isSafeUncheck{ false };
6063

6164
PropertyChanged_revoker m_InternalIsCheckedChangedRevoker{};
65+
66+
static thread_local std::unique_ptr<std::map<winrt::hstring, winrt::weak_ref<RadioMenuFlyoutItem>>> s_selectionMap;
6267
};

‎dev/RadioMenuFlyoutItem/RadioMenuFlyoutItem.idl

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,15 @@ unsealed runtimeclass RadioMenuFlyoutItem : Windows.UI.Xaml.Controls.MenuFlyoutI
1414

1515
static Windows.UI.Xaml.DependencyProperty IsCheckedProperty{ get; };
1616
static Windows.UI.Xaml.DependencyProperty GroupNameProperty{ get; };
17+
18+
[MUX_PUBLIC_V2]
19+
{
20+
[MUX_DEFAULT_VALUE("false")]
21+
[MUX_PROPERTY_CHANGED_CALLBACK_METHODNAME("OnAreCheckStatesEnabledPropertyChanged")]
22+
static Windows.UI.Xaml.DependencyProperty AreCheckStatesEnabledProperty{ get; };
23+
static void SetAreCheckStatesEnabled(Windows.UI.Xaml.Controls.MenuFlyoutSubItem object, Boolean value);
24+
static Boolean GetAreCheckStatesEnabled(Windows.UI.Xaml.Controls.MenuFlyoutSubItem object);
25+
}
1726
}
1827

19-
}
28+
}

0 commit comments

Comments
 (0)