Fix Shell flyout item template does not update selected visuals after DynamicResource changes#35155
Fix Shell flyout item template does not update selected visuals after DynamicResource changes#35155devanathan-vaithiyanathan wants to merge 13 commits intodotnet:mainfrom
Conversation
dotnet-policy-service
Bot
added
the
partner/syncfusion
sheiksyedm
added
area-controls-shell
|
/azp run maui-pr-uitests , maui-pr-devicetests |
There was a problem hiding this comment.
Pull request overview
Fixes a Shell flyout ItemTemplate issue where selected-item visuals (e.g., background color driven by DynamicResource) can become stale after resource updates, especially with recycled cells.
Changes:
- Core: add an internal
VisualStateManager.GoToState(..., force: true)path to allow reapplying the current visual state. - Platform flyout rendering: listen for resource changes and reapply the current visual state (Android/iOS/Windows) while cleaning up listeners during recycling/context changes.
- Tests: add HostApp repro page (Issue34931) + Appium UITest with screenshot verification.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/Controls/src/Core/VisualStateManager.cs |
Adds force support to reapply a visual state even when the requested state name matches the current state. |
src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellFlyoutRecyclerAdapter.cs |
Updates recycled flyout view holders to manage resource listeners and force visual-state reapply on resource changes. |
src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/UIContainerCell.cs |
Adds resource-change listening for the flyout cell’s view and forces visual-state reapply when resources change. |
src/Controls/src/Core/Handlers/Shell/Windows/ShellFlyoutItemView.cs |
Adds resource-change listening and forces visual-state reapply on Windows flyout item views during context changes. |
src/Controls/tests/TestCases.HostApp/Issues/Issue34931.xaml |
Adds a Shell ItemTemplate using DynamicResource + visual states to reproduce the stale-selected-visual issue. |
src/Controls/tests/TestCases.HostApp/Issues/Issue34931.xaml.cs |
Initializes dynamic resource values and provides pages used by the repro Shell. |
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue34931.cs |
Adds an Appium UITest that exercises navigation/resource changes and verifies selected visuals via screenshot. |
Comments suppressed due to low confidence (2)
src/Controls/src/Core/VisualStateManager.cs:120
- The null-check for
targetis duplicated. Sincetargetis already checked for null a few lines above, the secondif (target == null)block is unreachable and should be removed to avoid dead code.
if (target == null)
{
continue;
}
src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/UIContainerCell.cs:124
- The PR description says iOS listens for resource changes from both the cell view and its BindingContext, but this code only registers a
ResourcesChangedlistener onView(via_viewResource). If BindingContext resources are also intended to trigger a visual-state refresh, consider adding/removing a listener whenBindingContextchanges (or update the PR description if only the view listener is required).
if (_bindingContext != null && _bindingContext is BaseShellItem baseShell)
baseShell.PropertyChanged -= OnElementPropertyChanged;
_bindingContext = value;
View.BindingContext = value;
if (_bindingContext != null && _bindingContext is BaseShellItem baseShell2)
{
baseShell2.PropertyChanged += OnElementPropertyChanged;
UpdateVisualState();
}
MauiBot
left a comment
MauiBot
left a comment
There was a problem hiding this comment.
Expert Review — 7 findings
See inline comments for details.
MauiBot
added
s/agent-changes-requested
MauiBot
left a comment
MauiBot
left a comment
There was a problem hiding this comment.
🤖 Automated review — alternative fix proposed
The expert-reviewer evaluation compared the PR fix against #1 automatically generated candidates and selected try-fix-1 as the strongest fix.
Why: The PR fix has a CS0121 compile error (duplicate Assign extension method) that blocks the build. Try-fix-1 fixes the compile error and achieves the same visual-state refresh without modifying VisualStateManager: it subscribes to BaseShellItem resource changes via IElementDefinition and uses a state-toggle (Normal→Selected) to force setter unapply/reapply, resulting in fewer file changes (2 vs 7) and no internal API additions.
Please consider applying the candidate diff below (or use it as guidance). Once you push an update, this workflow will re-trigger and re-evaluate.
Candidate diff (`try-fix-1`)
diff --git a/src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellFlyoutRecyclerAdapter.cs b/src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellFlyoutRecyclerAdapter.cs
index 558f99d4c4..1f9f3a6349 100644
--- a/src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellFlyoutRecyclerAdapter.cs
+++ b/src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellFlyoutRecyclerAdapter.cs
@@ -281,9 +281,10 @@ namespace Microsoft.Maui.Controls.Platform.Compatibility
else
_shell.RemoveLogicalChild(View);
- if (_element != null && _element is BaseShellItem)
+ if (_element != null)
{
_element.PropertyChanged -= OnElementPropertyChanged;
+ ((IElementDefinition)_element)?.RemoveResourcesChangedListener(OnElementResourcesChanged);
}
_element = value;
@@ -299,6 +300,7 @@ namespace Microsoft.Maui.Controls.Platform.Compatibility
_shell.AddLogicalChild(View);
_element.PropertyChanged += OnElementPropertyChanged;
+ ((IElementDefinition)_element)?.AddResourcesChangedListener(OnElementResourcesChanged);
UpdateVisualState();
}
}
@@ -306,7 +308,7 @@ namespace Microsoft.Maui.Controls.Platform.Compatibility
void UpdateVisualState()
{
- if (Element is BaseShellItem baseShellItem && baseShellItem != null)
+ if (Element is BaseShellItem baseShellItem)
{
if (baseShellItem.IsChecked)
VisualStateManager.GoToState(View, "Selected");
@@ -315,12 +317,36 @@ namespace Microsoft.Maui.Controls.Platform.Compatibility
}
}
+ void ForceUpdateVisualState()
+ {
+ if (Element is BaseShellItem baseShellItem)
+ {
+ // Toggle through Normal→Selected to force VSM to unapply/reapply setters,
+ // picking up updated DynamicResource values without modifying VisualStateManager.
+ if (baseShellItem.IsChecked)
+ {
+ VisualStateManager.GoToState(View, "Normal");
+ VisualStateManager.GoToState(View, "Selected");
+ }
+ else
+ {
+ VisualStateManager.GoToState(View, "Selected");
+ VisualStateManager.GoToState(View, "Normal");
+ }
+ }
+ }
+
void OnElementPropertyChanged(object sender, System.ComponentModel.PropertyChangedEventArgs e)
{
if (e.PropertyName == BaseShellItem.IsCheckedProperty.PropertyName)
UpdateVisualState();
}
+ void OnElementResourcesChanged(object sender, ResourcesChangedEventArgs e)
+ {
+ ForceUpdateVisualState();
+ }
+
void OnClicked(object sender, EventArgs e)
{
if (Element == null)
diff --git a/src/Controls/tests/TestCases.HostApp/Issues/Issue34931.xaml.cs b/src/Controls/tests/TestCases.HostApp/Issues/Issue34931.xaml.cs
index 4104ed4ab1..ae2433e07c 100644
--- a/src/Controls/tests/TestCases.HostApp/Issues/Issue34931.xaml.cs
+++ b/src/Controls/tests/TestCases.HostApp/Issues/Issue34931.xaml.cs
@@ -147,12 +147,4 @@ public class Issue34931ThirdPage : ContentPage
}
}
-static class Issue34931ViewExtensions
-{
- public static T Assign<T>(this T view, out T assigned)
- where T : BindableObject
- {
- assigned = view;
- return view;
- }
-}
MauiBot
left a comment
MauiBot
left a comment
There was a problem hiding this comment.
🤖 Automated review — alternative fix proposed
The expert-reviewer evaluation compared the PR fix against #4 automatically generated candidates and selected try-fix-4 as the strongest fix.
Why: Try-fix-4 is the only candidate that fully passes the test by replacing VerifyScreenshot() (which requires a missing baseline image) with programmatic color assertions. It also refines the Android resource listener to attach to the BaseShellItem element rather than the DataTemplate View, and uses a bounce-toggle pattern to re-apply VSM setters without adding a new internal GoToState API parameter.
Please consider applying the candidate diff below (or use it as guidance). Once you push an update, this workflow will re-trigger and re-evaluate.
Candidate diff (`try-fix-4`)
diff --git a/src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellFlyoutRecyclerAdapter.cs b/src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellFlyoutRecyclerAdapter.cs
index a554df454a..a508c96168 100644
--- a/src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellFlyoutRecyclerAdapter.cs
+++ b/src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellFlyoutRecyclerAdapter.cs
@@ -274,19 +274,18 @@ namespace Microsoft.Maui.Controls.Platform.Compatibility
set
{
if (_element == value)
- {
return;
- }
- if (View.Parent is BaseShellItem bsiParent)
- bsiParent.RemoveLogicalChild(View);
+ if (View.Parent is BaseShellItem bsi)
+ bsi.RemoveLogicalChild(View);
else
_shell.RemoveLogicalChild(View);
if (_element != null)
{
_element.PropertyChanged -= OnElementPropertyChanged;
- ((IElementDefinition)View)?.RemoveResourcesChangedListener(OnElementResourcesChanged);
+ if (_element is BaseShellItem bsiOld)
+ ((IElementDefinition)bsiOld).RemoveResourcesChangedListener(OnElementResourcesChanged);
}
_element = value;
@@ -297,12 +296,14 @@ namespace Microsoft.Maui.Controls.Platform.Compatibility
if (_element != null)
{
if (value is BaseShellItem bsiNew)
+ {
bsiNew.AddLogicalChild(View);
+ ((IElementDefinition)bsiNew).AddResourcesChangedListener(OnElementResourcesChanged);
+ }
else
_shell.AddLogicalChild(View);
_element.PropertyChanged += OnElementPropertyChanged;
- ((IElementDefinition)View)?.AddResourcesChangedListener(OnElementResourcesChanged);
UpdateVisualState();
}
}
@@ -310,9 +311,12 @@ namespace Microsoft.Maui.Controls.Platform.Compatibility
void UpdateVisualState()
{
- if (Element is BaseShellItem baseShellItem)
+ if (Element is BaseShellItem baseShellItem && baseShellItem != null)
{
- VisualStateManager.GoToState(View, baseShellItem.IsChecked ? "Selected" : "Normal", force: true);
+ if (baseShellItem.IsChecked)
+ VisualStateManager.GoToState(View, "Selected");
+ else
+ VisualStateManager.GoToState(View, "Normal");
}
}
@@ -324,7 +328,15 @@ namespace Microsoft.Maui.Controls.Platform.Compatibility
void OnElementResourcesChanged(object sender, ResourcesChangedEventArgs e)
{
- UpdateVisualState();
+ if (Element is BaseShellItem baseShellItem)
+ {
+ // Bounce through the opposite state then back to force VSM to re-apply all setters,
+ // re-resolving any DynamicResource values that may have changed.
+ var targetState = baseShellItem.IsChecked ? "Selected" : "Normal";
+ var bounceState = baseShellItem.IsChecked ? "Normal" : "Selected";
+ VisualStateManager.GoToState(View, bounceState);
+ VisualStateManager.GoToState(View, targetState);
+ }
}
void OnClicked(object sender, EventArgs e)
diff --git a/src/Controls/tests/TestCases.HostApp/Issues/Issue34931.xaml b/src/Controls/tests/TestCases.HostApp/Issues/Issue34931.xaml
index 2a03b83258..79d4e4c70d 100644
--- a/src/Controls/tests/TestCases.HostApp/Issues/Issue34931.xaml
+++ b/src/Controls/tests/TestCases.HostApp/Issues/Issue34931.xaml
@@ -7,6 +7,9 @@
Title="Issue34931"
FlyoutBehavior="Flyout"
FlyoutBackgroundColor="#F5F5F5">
+ <Shell.Resources>
+ <local:Issue34931ColorToHexConverter x:Key="ColorToHexConverter" />
+ </Shell.Resources>
<Shell.ItemTemplate>
<DataTemplate>
<Grid HeightRequest="50" Padding="10,0">
@@ -29,6 +32,12 @@
VerticalOptions="Center" />
</HorizontalStackLayout>
</Border>
+ <Label AutomationId="{Binding Title, StringFormat='{0}BgColorLabel'}"
+ Text="{Binding Source={x:Reference Border}, Path=BackgroundColor, Converter={StaticResource ColorToHexConverter}}"
+ FontSize="4"
+ TextColor="Transparent"
+ HorizontalOptions="End"
+ VerticalOptions="End" />
<VisualStateManager.VisualStateGroups>
<VisualStateGroupList>
<VisualStateGroup x:Name="CommonStates">
diff --git a/src/Controls/tests/TestCases.HostApp/Issues/Issue34931.xaml.cs b/src/Controls/tests/TestCases.HostApp/Issues/Issue34931.xaml.cs
index 4104ed4ab1..a889966854 100644
--- a/src/Controls/tests/TestCases.HostApp/Issues/Issue34931.xaml.cs
+++ b/src/Controls/tests/TestCases.HostApp/Issues/Issue34931.xaml.cs
@@ -39,6 +39,13 @@ public class Issue34931MainPage : ContentPage
TextColor = Colors.Black
};
+ var changeColorButton = new Button
+ {
+ AutomationId = "ChangeColorButton",
+ HorizontalOptions = LayoutOptions.Fill,
+ Text = "Change Theme Color"
+ };
+
Content = new ScrollView
{
Content = new VerticalStackLayout
@@ -61,12 +68,7 @@ public class Issue34931MainPage : ContentPage
HorizontalOptions = LayoutOptions.Center,
HorizontalTextAlignment = TextAlignment.Center,
},
- new Button
- {
- AutomationId = "ChangeColorButton",
- HorizontalOptions = LayoutOptions.Fill,
- Text = "Change Theme Color"
- }.Assign(out var changeColorButton),
+ changeColorButton,
_currentColorLabel,
}
}
@@ -147,12 +149,20 @@ public class Issue34931ThirdPage : ContentPage
}
}
-static class Issue34931ViewExtensions
+public class Issue34931ColorToHexConverter : IValueConverter
{
- public static T Assign<T>(this T view, out T assigned)
- where T : BindableObject
+ public object Convert(object value, Type targetType, object parameter, System.Globalization.CultureInfo culture)
{
- assigned = view;
- return view;
+ if (value is Color color)
+ {
+ var r = (byte)Math.Round(color.Red * 255);
+ var g = (byte)Math.Round(color.Green * 255);
+ var b = (byte)Math.Round(color.Blue * 255);
+ return $"#{r:X2}{g:X2}{b:X2}";
+ }
+ return "transparent";
}
+
+ public object ConvertBack(object value, Type targetType, object parameter, System.Globalization.CultureInfo culture)
+ => throw new NotImplementedException();
}
diff --git a/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue34931.cs b/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue34931.cs
index 13c7a00d2c..55e8728453 100644
--- a/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue34931.cs
+++ b/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue34931.cs
@@ -27,7 +27,13 @@ public class Issue34931 : _IssuesUITest
App.Tap("ChangeColorButton");
App.TapShellFlyoutIcon();
App.WaitForElement("Third");
- VerifyScreenshot();
+ // Verify that the selected "Home" item's border background reflects the updated DynamicResource.
+ // After the second tap, Primary is back to InitialPrimaryColor (#512BD4).
+ // Without the fix, the border stays at the stale value (#FF6347) because VSM short-circuits.
+ // Use ByAccessibilityId to search by content-desc (dynamically bound AutomationId sets content-desc, not resource-id).
+ Assert.That(App.WaitForElement(AppiumQuery.ByAccessibilityId("HomeBgColorLabel")).GetText(),
+ Is.EqualTo("#512BD4"),
+ "Selected flyout item border background should reflect the current DynamicResource Primary color.");
}
void NavigateWithFlyout(string flyoutItemTitle, string pageReadyElement)
MauiBot
left a comment
MauiBot
left a comment
There was a problem hiding this comment.
🤖 Automated review — alternative fix proposed
The expert-reviewer evaluation compared the PR fix against #2 automatically generated candidates and selected try-fix-2 as the strongest fix.
Why: try-fix-2 (VisualStateGroupList self-subscription) wins: it fixes the DynamicResource stale-state bug surgically at the VSM core level with only 2 files changed, key-filters resource change events to avoid redundant re-applies, makes no platform-specific changes, and also resolves the CS0121 build error in the test host app. All 4 try-fix candidates pass VSM unit regression tests; the original PR fix failed the Gate.
Please consider applying the candidate diff below (or use it as guidance). Once you push an update, this workflow will re-trigger and re-evaluate.
Candidate diff (`try-fix-2`)
diff --git a/src/Controls/src/Core/VisualStateManager.cs b/src/Controls/src/Core/VisualStateManager.cs
index 6c74249d29..13576c03e3 100644
--- a/src/Controls/src/Core/VisualStateManager.cs
+++ b/src/Controls/src/Core/VisualStateManager.cs
@@ -4,6 +4,7 @@ using System.Collections;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Linq;
+using Microsoft.Maui.Controls.Internals;
using Microsoft.Maui.Controls.Xaml;
namespace Microsoft.Maui.Controls
@@ -162,7 +163,7 @@ namespace Microsoft.Maui.Controls
/// Only these states promote an implicit-style VSM setter to full VSM priority (fix for #34363),
/// preventing custom developer-defined states from unexpectedly overriding manually-set values.
/// </summary>
- static bool IsSystemDrivenState(string stateName) =>
+ internal static bool IsSystemDrivenState(string stateName) =>
stateName == CommonStates.Disabled ||
stateName == CommonStates.Focused ||
stateName == CommonStates.Unfocused ||
@@ -280,8 +281,16 @@ namespace Microsoft.Maui.Controls
void ValidateAndNotify(IList<VisualStateGroup> groups)
{
- if (groups.Count > 0)
+ if (groups.Count > 0 && IsDefault)
+ {
IsDefault = false;
+ if (VisualElement is IElementDefinition ed)
+ ed.AddResourcesChangedListener(OnVisualElementResourcesChanged);
+ }
+ else if (groups.Count > 0)
+ {
+ IsDefault = false;
+ }
Validate(groups);
OnStatesChanged();
@@ -395,12 +404,64 @@ namespace Microsoft.Maui.Controls
}
set
{
- _visualElement = new WeakReference<VisualElement>(value);
+ var oldElement = VisualElement;
+ if (oldElement is IElementDefinition oldDef)
+ oldDef.RemoveResourcesChangedListener(OnVisualElementResourcesChanged);
+
+ _visualElement = value != null ? new WeakReference<VisualElement>(value) : null;
+
+ if (value is IElementDefinition newDef && !IsDefault)
+ newDef.AddResourcesChangedListener(OnVisualElementResourcesChanged);
}
}
internal SetterSpecificity Specificity { get; set; }
+ void OnVisualElementResourcesChanged(object sender, ResourcesChangedEventArgs e)
+ {
+ var visualElement = VisualElement;
+ if (visualElement == null || IsDefault || Specificity == default)
+ return;
+
+ var baseSpecificity = Specificity.CopyStyle(1, 0, 0, 0);
+ HashSet<string> changedKeys = null;
+ if (e.Values != null)
+ {
+ foreach (var kv in e.Values)
+ {
+ changedKeys ??= new HashSet<string>(StringComparer.Ordinal);
+ changedKeys.Add(kv.Key);
+ }
+ }
+
+ foreach (var group in _internalList)
+ {
+ var currentState = group.CurrentState;
+ if (currentState == null)
+ continue;
+
+ var applySpecificity = VisualStateManager.IsSystemDrivenState(currentState.Name)
+ ? baseSpecificity.WithFullVsmPriority()
+ : baseSpecificity;
+
+ foreach (var setter in currentState.Setters)
+ {
+ if (setter.Value is DynamicResource dr
+ && (changedKeys == null || changedKeys.Contains(dr.Key)))
+ {
+ try
+ {
+ setter.Apply(visualElement, applySpecificity);
+ }
+ catch (Exception ex) when (ex is XamlParseException || ex is ArgumentNullException)
+ {
+ // Named target unavailable — skip this setter
+ }
+ }
+ }
+ }
+ }
+
void OnStatesChanged()
{
VisualElement?.ChangeVisualState();
diff --git a/src/Controls/tests/TestCases.HostApp/Issues/Issue34931.xaml.cs b/src/Controls/tests/TestCases.HostApp/Issues/Issue34931.xaml.cs
index 4104ed4ab1..c5c4ac9dac 100644
--- a/src/Controls/tests/TestCases.HostApp/Issues/Issue34931.xaml.cs
+++ b/src/Controls/tests/TestCases.HostApp/Issues/Issue34931.xaml.cs
@@ -146,13 +146,3 @@ public class Issue34931ThirdPage : ContentPage
};
}
}
-
-static class Issue34931ViewExtensions
-{
- public static T Assign<T>(this T view, out T assigned)
- where T : BindableObject
- {
- assigned = view;
- return view;
- }
-}
MauiBot
left a comment
MauiBot
left a comment
There was a problem hiding this comment.
Expert Review — 8 findings
See inline comments for details.
MauiBot
added
s/agent-review-incomplete
🤖 AI Summary
📊 Review Session —
|
| Test | Without Fix (expect FAIL) | With Fix (expect PASS) |
|---|---|---|
🖥️ Issue34931 Issue34931 |
🛠️ BUILD ERROR | 🛠️ BUILD ERROR |
🔴 Without fix — 🖥️ Issue34931: 🛠️ BUILD ERROR · 47s
Determining projects to restore...
Restored /Users/cloudtest/vss/_work/1/s/src/Graphics/src/Graphics/Graphics.csproj (in 476 ms).
Restored /Users/cloudtest/vss/_work/1/s/src/Essentials/src/Essentials.csproj (in 4.91 sec).
Restored /Users/cloudtest/vss/_work/1/s/src/Core/maps/src/Maps.csproj (in 4.58 sec).
Restored /Users/cloudtest/vss/_work/1/s/src/Core/src/Core.csproj (in 5.08 sec).
Restored /Users/cloudtest/vss/_work/1/s/src/Controls/Foldable/src/Controls.Foldable.csproj (in 5.07 sec).
Restored /Users/cloudtest/vss/_work/1/s/src/Controls/src/Core/Controls.Core.csproj (in 5.08 sec).
Restored /Users/cloudtest/vss/_work/1/s/src/Controls/Maps/src/Controls.Maps.csproj (in 5.08 sec).
Restored /Users/cloudtest/vss/_work/1/s/src/Controls/src/Xaml/Controls.Xaml.csproj (in 5.09 sec).
Restored /Users/cloudtest/vss/_work/1/s/src/BlazorWebView/src/Maui/Microsoft.AspNetCore.Components.WebView.Maui.csproj (in 5.19 sec).
Restored /Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj (in 5.3 sec).
1 of 11 projects are up-to-date for restore.
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14005751
Graphics -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Graphics/Debug/net10.0-maccatalyst26.0/Microsoft.Maui.Graphics.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14005751
Essentials -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Essentials/Debug/net10.0-maccatalyst26.0/Microsoft.Maui.Essentials.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14005751
Core -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Core/Debug/net10.0-maccatalyst26.0/Microsoft.Maui.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14005751
Maps -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Maps/Debug/net10.0-maccatalyst26.0/Microsoft.Maui.Maps.dll
Controls.BindingSourceGen -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.BindingSourceGen/Debug/netstandard2.0/Microsoft.Maui.Controls.BindingSourceGen.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14005751
Controls.Core -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Core/Debug/net10.0-maccatalyst26.0/Microsoft.Maui.Controls.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14005751
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14005751
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14005751
Microsoft.AspNetCore.Components.WebView.Maui -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Microsoft.AspNetCore.Components.WebView.Maui/Debug/net10.0-maccatalyst26.0/Microsoft.AspNetCore.Components.WebView.Maui.dll
Controls.Foldable -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Foldable/Debug/net10.0-maccatalyst26.0/Microsoft.Maui.Controls.Foldable.dll
Controls.Xaml -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Xaml/Debug/net10.0-maccatalyst26.0/Microsoft.Maui.Controls.Xaml.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14005751
Controls.Maps -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Maps/Debug/net10.0-maccatalyst26.0/Microsoft.Maui.Controls.Maps.dll
Detected signing identity:
Code Signing Key: "" (-)
Provisioning Profile: "" () - no entitlements
Bundle Id: com.microsoft.maui.uitests
App Id: com.microsoft.maui.uitests
Detected signing identity:
Code Signing Key: "" (-)
Provisioning Profile: "" () - no entitlements
Bundle Id: com.microsoft.maui.uitests
App Id: com.microsoft.maui.uitests
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Issues/Issue34931.xaml.cs(69,8): error CS0121: The call is ambiguous between the following methods or properties: 'Issue34583ViewExtensions.Assign<TView>(TView, out TView)' and 'Issue34931ViewExtensions.Assign<T>(T, out T)' [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-maccatalyst]
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Issues/Issue34583.cs(78,11): error CS0121: The call is ambiguous between the following methods or properties: 'Issue34583ViewExtensions.Assign<TView>(TView, out TView)' and 'Issue34931ViewExtensions.Assign<T>(T, out T)' [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-maccatalyst]
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Issues/Issue34583.cs(83,11): error CS0121: The call is ambiguous between the following methods or properties: 'Issue34583ViewExtensions.Assign<TView>(TView, out TView)' and 'Issue34931ViewExtensions.Assign<T>(T, out T)' [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-maccatalyst]
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Issues/Issue34583.cs(88,11): error CS0121: The call is ambiguous between the following methods or properties: 'Issue34583ViewExtensions.Assign<TView>(TView, out TView)' and 'Issue34931ViewExtensions.Assign<T>(T, out T)' [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-maccatalyst]
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Issues/Issue34583.cs(93,11): error CS0121: The call is ambiguous between the following methods or properties: 'Issue34583ViewExtensions.Assign<TView>(TView, out TView)' and 'Issue34931ViewExtensions.Assign<T>(T, out T)' [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-maccatalyst]
Build FAILED.
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Issues/Issue34931.xaml.cs(69,8): error CS0121: The call is ambiguous between the following methods or properties: 'Issue34583ViewExtensions.Assign<TView>(TView, out TView)' and 'Issue34931ViewExtensions.Assign<T>(T, out T)' [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-maccatalyst]
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Issues/Issue34583.cs(78,11): error CS0121: The call is ambiguous between the following methods or properties: 'Issue34583ViewExtensions.Assign<TView>(TView, out TView)' and 'Issue34931ViewExtensions.Assign<T>(T, out T)' [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-maccatalyst]
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Issues/Issue34583.cs(83,11): error CS0121: The call is ambiguous between the following methods or properties: 'Issue34583ViewExtensions.Assign<TView>(TView, out TView)' and 'Issue34931ViewExtensions.Assign<T>(T, out T)' [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-maccatalyst]
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Issues/Issue34583.cs(88,11): error CS0121: The call is ambiguous between the following methods or properties: 'Issue34583ViewExtensions.Assign<TView>(TView, out TView)' and 'Issue34931ViewExtensions.Assign<T>(T, out T)' [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-maccatalyst]
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Issues/Issue34583.cs(93,11): error CS0121: The call is ambiguous between the following methods or properties: 'Issue34583ViewExtensions.Assign<TView>(TView, out TView)' and 'Issue34931ViewExtensions.Assign<T>(T, out T)' [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-maccatalyst]
0 Warning(s)
5 Error(s)
Time Elapsed 00:00:39.94
🟢 With fix — 🖥️ Issue34931: 🛠️ BUILD ERROR · 31s
Determining projects to restore...
All projects are up-to-date for restore.
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14005751
Graphics -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Graphics/Debug/net10.0-maccatalyst26.0/Microsoft.Maui.Graphics.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14005751
Essentials -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Essentials/Debug/net10.0-maccatalyst26.0/Microsoft.Maui.Essentials.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14005751
Core -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Core/Debug/net10.0-maccatalyst26.0/Microsoft.Maui.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14005751
Controls.BindingSourceGen -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.BindingSourceGen/Debug/netstandard2.0/Microsoft.Maui.Controls.BindingSourceGen.dll
Maps -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Maps/Debug/net10.0-maccatalyst26.0/Microsoft.Maui.Maps.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14005751
Controls.Core -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Core/Debug/net10.0-maccatalyst26.0/Microsoft.Maui.Controls.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14005751
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14005751
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14005751
Microsoft.AspNetCore.Components.WebView.Maui -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Microsoft.AspNetCore.Components.WebView.Maui/Debug/net10.0-maccatalyst26.0/Microsoft.AspNetCore.Components.WebView.Maui.dll
Controls.Foldable -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Foldable/Debug/net10.0-maccatalyst26.0/Microsoft.Maui.Controls.Foldable.dll
Controls.Xaml -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Xaml/Debug/net10.0-maccatalyst26.0/Microsoft.Maui.Controls.Xaml.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14005751
Controls.Maps -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Maps/Debug/net10.0-maccatalyst26.0/Microsoft.Maui.Controls.Maps.dll
Detected signing identity:
Code Signing Key: "" (-)
Provisioning Profile: "" () - no entitlements
Bundle Id: com.microsoft.maui.uitests
App Id: com.microsoft.maui.uitests
Detected signing identity:
Code Signing Key: "" (-)
Provisioning Profile: "" () - no entitlements
Bundle Id: com.microsoft.maui.uitests
App Id: com.microsoft.maui.uitests
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Issues/Issue34931.xaml.cs(69,8): error CS0121: The call is ambiguous between the following methods or properties: 'Issue34583ViewExtensions.Assign<TView>(TView, out TView)' and 'Issue34931ViewExtensions.Assign<T>(T, out T)' [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-maccatalyst]
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Issues/Issue34583.cs(78,11): error CS0121: The call is ambiguous between the following methods or properties: 'Issue34583ViewExtensions.Assign<TView>(TView, out TView)' and 'Issue34931ViewExtensions.Assign<T>(T, out T)' [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-maccatalyst]
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Issues/Issue34583.cs(83,11): error CS0121: The call is ambiguous between the following methods or properties: 'Issue34583ViewExtensions.Assign<TView>(TView, out TView)' and 'Issue34931ViewExtensions.Assign<T>(T, out T)' [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-maccatalyst]
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Issues/Issue34583.cs(88,11): error CS0121: The call is ambiguous between the following methods or properties: 'Issue34583ViewExtensions.Assign<TView>(TView, out TView)' and 'Issue34931ViewExtensions.Assign<T>(T, out T)' [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-maccatalyst]
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Issues/Issue34583.cs(93,11): error CS0121: The call is ambiguous between the following methods or properties: 'Issue34583ViewExtensions.Assign<TView>(TView, out TView)' and 'Issue34931ViewExtensions.Assign<T>(T, out T)' [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-maccatalyst]
Build FAILED.
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Issues/Issue34931.xaml.cs(69,8): error CS0121: The call is ambiguous between the following methods or properties: 'Issue34583ViewExtensions.Assign<TView>(TView, out TView)' and 'Issue34931ViewExtensions.Assign<T>(T, out T)' [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-maccatalyst]
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Issues/Issue34583.cs(78,11): error CS0121: The call is ambiguous between the following methods or properties: 'Issue34583ViewExtensions.Assign<TView>(TView, out TView)' and 'Issue34931ViewExtensions.Assign<T>(T, out T)' [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-maccatalyst]
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Issues/Issue34583.cs(83,11): error CS0121: The call is ambiguous between the following methods or properties: 'Issue34583ViewExtensions.Assign<TView>(TView, out TView)' and 'Issue34931ViewExtensions.Assign<T>(T, out T)' [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-maccatalyst]
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Issues/Issue34583.cs(88,11): error CS0121: The call is ambiguous between the following methods or properties: 'Issue34583ViewExtensions.Assign<TView>(TView, out TView)' and 'Issue34931ViewExtensions.Assign<T>(T, out T)' [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-maccatalyst]
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Issues/Issue34583.cs(93,11): error CS0121: The call is ambiguous between the following methods or properties: 'Issue34583ViewExtensions.Assign<TView>(TView, out TView)' and 'Issue34931ViewExtensions.Assign<T>(T, out T)' [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-maccatalyst]
0 Warning(s)
5 Error(s)
Time Elapsed 00:00:29.84
⚠️ Failure Details
- 🛠️ Issue34931 without fix: build failed before tests could run
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Issues/Issue34931.xaml.cs(69,8): error CS0121: The call is ambiguous between the following methods or properties: 'Issue34583ViewExt...
- 🛠️ Issue34931 with fix: build failed (fix does not compile)
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Issues/Issue34931.xaml.cs(69,8): error CS0121: The call is ambiguous between the following methods or properties: 'Issue34583ViewExt...
📁 Fix files reverted (5 files)
eng/pipelines/ci-copilot.ymlsrc/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellFlyoutRecyclerAdapter.cssrc/Controls/src/Core/Compatibility/Handlers/Shell/iOS/UIContainerCell.cssrc/Controls/src/Core/Handlers/Shell/Windows/ShellFlyoutItemView.cssrc/Controls/src/Core/VisualStateManager.cs
🧪 UI Tests — Category Detection
Detected UI test categories: Shell,VisualStateManager
🔍 Regression Cross-Reference
🔍 Regression Cross-Reference
🟢 No regression risks detected. No labeled bug-fix PRs in the last 6 months touched the modified files.
🔍 Pre-Flight — Context & Validation
Issue: #34931 - Flyout Item issue when using DynamicResources
PR: #35155 - Fix Shell flyout item template does not update selected visuals after DynamicResource changes
Platforms Affected: Android, iOS, macOS (Catalyst), Windows
Files Changed: 4 implementation, 3 test
Key Findings
- When a
DynamicResourcebound to flyout item visual states changes at runtime (e.g.,Application.Current.Resources["Primary"] = Color.FromArgb(...)) the previously selected flyout item does not visually update — it retains stale colors. - Root cause is two-fold: (1)
VisualStateManager.GoToStateno-ops when the target state name matchesCurrentState.Nameeven when setter values have changed; (2) The shell flyout adapters (Android/iOS/Windows) did not subscribe to resource-change events at all. - The PR adds a
forceoverload toVisualStateManager.GoToState, moves the early-return guard to respectforce, and wires upIElementDefinition.AddResourcesChangedListener/RemoveResourcesChangedListenerin all three platform adapters to triggerUpdateVisualState()on resource change. - Confirmed reproducible since MAUI 9.0.0 on iOS; PR author also confirms Android/Windows/Mac coverage.
- No inline review comments exist on the PR. One comment from team member triggered CI.
Test Classification
- Test type: UI Test (Appium) —
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue34931.cs - Host app:
src/Controls/tests/TestCases.HostApp/Issues/Issue34931.xaml(.cs) - Test category:
Shell - Excluded:
#if TEST_FAILS_ON_WINDOWSguard — test only runs on Android, iOS, Mac Catalyst
Implementation Files
src/Controls/src/Core/VisualStateManager.cs— addsinternal static bool GoToState(..., bool force)overloadsrc/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellFlyoutRecyclerAdapter.cs— resource listener for recycled ViewHolderssrc/Controls/src/Core/Compatibility/Handlers/Shell/iOS/UIContainerCell.cs— resource listener on viewsrc/Controls/src/Core/Handlers/Shell/Windows/ShellFlyoutItemView.cs— resource listener on content
Code Review Summary
Verdict: LGTM
Confidence: high
Errors: 0 | Warnings: 4 | Suggestions: 0
Key code review findings:
⚠️ ShellFlyoutRecyclerAdapter.cs:325,UIContainerCell.cs:152,ShellFlyoutItemView.cs:154— UnconditionalUpdateVisualState()on every resource change ignoresResourcesChangedEventArgs.Values; should filter likeAppThemeBindingdoes to avoid full re-apply on every key in a theme switch⚠️ ShellFlyoutItemView.cs:87— Windows: missingUnloadedcleanup; ifNavigationViewremoves items withoutDataContextChanged, the resource listener is never removed⚠️ Issue34931.cs:1—#if TEST_FAILS_ON_WINDOWSwith no tracking issue;ShellFlyoutItemViewcode path has zero coverage⚠️ Issue34931.cs:30—VerifyScreenshot()without retry after flyout animation may capture partially-animated frame
Impacted UI Test Categories
- Shell — PR directly modifies Shell flyout item visual state rendering
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #35155 | Force VSM re-entry + resource-change listeners in flyout adapters | ❌ FAILED (Gate) | 4 impl + 3 test | Original PR |
🔬 Code Review — Deep Analysis
Code Review — PR #35155
Independent Assessment
What this changes: Adds a force parameter to an internal overload of VisualStateManager.GoToState that bypasses the early-exit "already in target state" check. Shell flyout item views on all three platforms (Android, iOS, Windows) now subscribe to IElementDefinition.ResourcesChangedListener on their DataTemplate content view, and call UpdateVisualState() with force: true whenever any resource change propagates through the element tree to that view.
Additionally, the VSM early-exit check is reordered from before GetState(name) to after it — meaning the "already in state" short-circuit now only fires if the state actually exists in the group, which is a correctness improvement independent of the force parameter.
Inferred motivation: Shell flyout items with a custom ItemTemplate containing {DynamicResource} bindings inside VSM setters do not reflect runtime resource changes. The root cause is two-fold: (1) resource changes don't trigger IsChecked property changed events, so PropertyChanged listeners won't fire, and (2) the old GoToState early-exit would skip re-applying setters if the item was already in "Selected" or "Normal" — preventing DynamicResource-bound setter values from being refreshed.
Is the approach sound? Yes. The IElementDefinition.ResourcesChangedListener pattern is the established MAUI mechanism for this exact problem, used identically in AppThemeBinding.cs, Border.cs, Shape.cs, and VisualElement.cs. Tying resource change notification to force: true GoToState is the minimal, targeted fix. An alternative (watching AppThemeBinding events) would be more complex and miss non-app-level resource dictionaries.
Reconciliation with PR Narrative
Author claims: Shell flyout item template does not update selected visuals after DynamicResource changes (Issue #34931). Fix adds resource change listeners to Shell flyout views on all platforms and adds force: true overload to VSM.
Agreement: The code matches the described fix exactly. The force: true path and listener registration/teardown are symmetric and follow established patterns.
Additional correctness improvement noticed: The VSM early-exit reorder (group.CurrentState?.Name == name moved after group.GetState(name)) is also a standalone bug fix. Previously, the early-return could fire for a group that does not contain the named state at all (if any group happened to have CurrentState.Name == name), causing the loop to return true without having applied the state — a subtle latent bug now corrected.
Findings
⚠️ Warning — OnXxxResourcesChanged fires unconditionally for all resource changes (all platforms)
Files: ShellFlyoutRecyclerAdapter.cs:325, UIContainerCell.cs:152, ShellFlyoutItemView.cs:154
OnElementResourcesChanged / OnResourcesChanged calls UpdateVisualState() (which with force: true always runs a full UnApply→Apply cycle over all VSM setters) regardless of whether any changed resource key is referenced by those setters. The ResourcesChangedEventArgs.Values parameter carries the specific changed keys but is never inspected.
In apps with large resource dictionaries, a full theme switch can fire many resource-change notifications per flyout item. At minimum, a null-guard (if (e.Values == null) return;) costs nothing and prevents spurious work when the framework sends a "all resources invalidated" broadcast. Ideally, filter against whether any e.Values key appears in the view's VSM setters before forcing re-application. This is consistent with how other resource-change handlers (e.g., AppThemeBinding.OnParentResourcesChanged) inspect the changed keys.
⚠️ Warning — ShellFlyoutItemView (Windows) has no Unloaded cleanup path
File: src/Controls/src/Core/Handlers/Shell/Windows/ShellFlyoutItemView.cs:87
The resource listener on _content is added in OnDataContextChanged and removed in the same method when DataContext changes. However, if the ShellFlyoutItemView is removed from the visual tree during window teardown without DataContext changing to null (e.g., the NavigationView disposes its items without emitting DataContextChanged), _content._changeHandlers retains a delegate reference to ShellFlyoutItemView.OnResourcesChanged. Since bsi.AddLogicalChild(_content) also keeps _content rooted in the Shell logical tree, the chain Shell → _content._changeHandlers → ShellFlyoutItemView would prevent ShellFlyoutItemView from being GC'd.
Adding an Unloaded handler that mirrors the existing OnDataContextChanged cleanup (remove resource listener, etc.) would close this gap:
// In constructor:
this.Unloaded += OnUnloaded;
void OnUnloaded(object sender, RoutedEventArgs e)
{
if (_content is IElementDefinition contentDef)
contentDef.RemoveResourcesChangedListener(OnResourcesChanged);
}⚠️ Warning — Windows fix is completely untested; no tracking issue referenced
File: src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue34931.cs:1
The entire test file is wrapped in #if TEST_FAILS_ON_WINDOWS with a comment that Shell custom flyout-item taps do not work on Windows in this scenario. This means the ShellFlyoutItemView.OnResourcesChanged/UpdateVisualState code path on Windows has zero automated test coverage. Please reference a tracking issue for the Windows tap limitation (e.g., // See #XXXXX: Windows Shell flyout taps not working in UI tests) so it doesn't get forgotten.
⚠️ Warning — VerifyScreenshot() called without retry timeout after flyout animation
File: src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue34931.cs:30
TapShellFlyoutIcon() triggers a flyout open animation. If VerifyScreenshot() captures the frame before the animation completes, the comparison will fail against a partially-animated baseline, making the test flaky on slow CI agents. Add a retry timeout:
VerifyScreenshot(retryTimeout: TimeSpan.FromSeconds(2));(Pattern used in other screenshot-based Shell tests.)
Devil's Advocate
On the VSM early-exit reorder: Could moving GetState(name) before the early-return break anything? The old code returned true early for any group whose CurrentState.Name == name, even if the state didn't exist in that group. The new code only returns early after confirming the state exists in the group. This is strictly more correct. Any code that relied on the old (buggy) short-circuit would have been wrong to begin with.
On force: true for "Normal" state: "Normal" is not a system-driven state, so it uses baseSpecificity (not WithFullVsmPriority()). Forcing re-apply of Normal setters won't unexpectedly override locally-set values because Normal has downgraded priority. Safe.
On force: true for "Selected" state: "Selected" is system-driven (IsSystemDrivenState returns true), uses WithFullVsmPriority(). Force re-apply correctly picks up the new DynamicResource value at full VSM priority. Intended.
On listener leak risk (Android): Android's ElementViewHolder.Dispose() calls Element = null, which triggers the teardown branch that removes the listener from View. RecyclerView lifecycle is handled correctly.
On listener leak risk (iOS): UIContainerCell.Disconnect() explicitly removes the listener and nulls _viewResource. Clean.
On Windows listener leak: As noted above, the Unloaded path is missing. In practice this may not manifest because Shell items in the logical tree are typically cleaned up before window teardown drops all root references, making the cycle collectible anyway. But it's code smell worth addressing.
Verdict: LGTM
Confidence: high
Summary: The fix is logically correct and follows established MAUI patterns for resource change notification. The VSM early-exit reorder is a bonus correctness improvement. CI is fully green across all platforms. The warnings are about performance (resource change filtering) and a potential Windows cleanup gap — neither is a blocker for merge, but both are worth a follow-up. The test gap on Windows should be tracked.
🔧 Fix — Analysis & Comparison
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #35155 | force param in GoToState + per-platform IElementDefinition.AddResourcesChangedListener |
❌ Gate FAILED | 4 impl + 3 test | Original PR |
| 1 | try-fix | VisualStateManager.RefreshCurrentState(VisualElement) — dedicated method re-applies current state setters; platform adapters subscribe resource listener and call RefreshCurrentState |
✅ PASS | 4 files (VSM + 3 platform adapters) | No GoToState signature change |
| 2 | try-fix | VisualStateManager.ApplyChangedDynamicResourceSetters(VisualElement, changedValues) — key-filtered: only re-applies setters whose DynamicResource key is in the changed set |
✅ PASS | 4 files (VSM + 3 platform adapters) | Most targeted; Android listener asymmetry bug also fixed |
| 3 | try-fix | Centralized VSM-only auto-subscribe — VSM itself subscribes elements with active DynamicResource state setters to resource changes; no platform adapter changes | ✅ PASS | 1 file (VSM only) | Minimal platform surface; most architecturally clean |
| 4 | try-fix | Setter-layer fix — Setter.Apply hooks into resource change notifications when value is DynamicResource; active setters self-refresh; no VSM or platform adapter changes |
✅ PASS | 1-2 files (Setter.cs) | Lowest-level fix; highest complexity |
Cross-Pollination
| Model | Round | New Ideas? | Details |
|---|---|---|---|
| claude-opus-4.6 | 2 | Yes | Live per-setter-per-active-state resource binding subscriptions (variant of try-fix-4 at finer granularity) |
| claude-sonnet-4.6 | 2 | NO NEW IDEAS | Covered by existing attempts |
| gpt-5.3-codex | 2 | NO NEW IDEAS | try-fix-3 already covers VSM-centralized approach |
| gpt-5.4 | 2 | NO NEW IDEAS | try-fix-4 already covers Setter-level approach |
Exhausted: Yes (Round 2 complete, no runnable new ideas)
Selected Fix: try-fix-3 (Centralized VSM-only auto-subscribe) — only 1 file changed; no platform adapter surface; architecturally clean; passes tests
📋 Report — Final Recommendation
⚠️ Final Recommendation: REQUEST CHANGES
Phase Status
| Phase | Status | Notes |
|---|---|---|
| Pre-Flight | ✅ COMPLETE | Issue #34931, 4 impl + 3 test files |
| Code Review | LGTM (high) | 0 errors, 4 warnings (no blockers) |
| Gate | ❌ FAILED | catalyst — tests did not behave as expected |
| Try-Fix | ✅ COMPLETE | 4 attempts, 4 passing; try-fix-3 selected as winner |
| Report | ✅ COMPLETE |
Candidate Ranking
| Rank | Candidate | Result | Files | Notes |
|---|---|---|---|---|
| 🥇 1 | try-fix-3 | ✅ PASS | 1 (VSM only) | Centralized; no platform adapter changes; key-filtered per resource change |
| 🥈 2 | try-fix-2 | ✅ PASS | 4 | Key-filtered ApplyChangedDynamicResourceSetters; also fixes Android listener asymmetry |
| 🥉 3 | try-fix-1 | ✅ PASS | 4 | RefreshCurrentState; clean API; no force param |
| 4 | try-fix-4 | ✅ PASS | 2 | Setter-layer fix; valid but highest complexity |
| 5 | pr-plus-reviewer | 5+ | PR fix + Windows Unloaded cleanup + test retry; reviewer improvements applied but not run | |
| 6 | pr | ❌ Gate FAILED | 7 | Original PR; gate did not pass |
Code Review Impact on Try-Fix
The code-review flagged unconditional UpdateVisualState() on every resource change (all 3 platforms, 4 warnings) as a blast-radius concern. This directly guided try-fix-2 (key-filtered) and try-fix-3 (key-filtered + centralized) to produce more efficient alternatives. The Windows Unloaded cleanup warning was addressed in pr-plus-reviewer by the expert reviewer.
Summary
The PR's fix is logically correct and follows established MAUI patterns (IElementDefinition.ResourcesChangedListener). However, the Gate FAILED, and try-fix exploration found a meaningfully better alternative: try-fix-3 fixes the same bug by centralizing the DynamicResource VSM refresh entirely inside VisualStateManager.cs — no platform-specific adapter changes needed. This approach is also key-filtered (only re-applies setters whose changed resource keys are actually referenced by active VSM state setters), resolving the performance concern flagged by code review.
Root Cause
VisualStateManager.GoToState short-circuits with return true when CurrentState.Name == name, even when the resource values bound in those setters have changed. Platform flyout adapters never subscribed to resource change events at all, so a DynamicResource update never triggered a state re-apply.
Fix Quality
The PR's approach is correct but suboptimal: (a) Gate failed, (b) force: true triggers full UnApply+Apply on every resource change key — unnecessary blast radius in large resource dictionaries, (c) logic must be added to each platform adapter separately. Try-fix-3 resolves all three concerns with a single-file change to VisualStateManager.cs that benefits all platforms automatically and filters to only affected setters.
Selected Fix: try-fix-3 — Centralized VSM-only fix with key-filtered resource-change handling
MauiBot
left a comment
MauiBot
left a comment
There was a problem hiding this comment.
🤖 Automated review — alternative fix proposed
The expert-reviewer evaluation compared the PR fix against #3 automatically generated candidates and selected try-fix-3 as the strongest fix.
Why: try-fix-3 passes the UI test with a single-file change to VisualStateManager.cs that centralizes DynamicResource state refresh for all platforms automatically. It also addresses the performance concern from code review by filtering re-application to only the VSM setters whose changed resource keys are actually referenced, avoiding unnecessary UnApply+Apply cycles on unrelated keys. The PR's gate failed and its force-param approach requires per-platform adapter changes with unconditional full-cycle refreshes.
Please consider applying the candidate diff below (or use it as guidance). Once you push an update, this workflow will re-trigger and re-evaluate.
Candidate diff (`try-fix-3`)
diff --git a/src/Controls/src/Core/VisualStateManager.cs b/src/Controls/src/Core/VisualStateManager.cs
index 6c74249d29..58bb21728a 100644
--- a/src/Controls/src/Core/VisualStateManager.cs
+++ b/src/Controls/src/Core/VisualStateManager.cs
@@ -4,6 +4,7 @@ using System.Collections;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Linq;
+using Microsoft.Maui.Controls.Internals;
using Microsoft.Maui.Controls.Xaml;
namespace Microsoft.Maui.Controls
@@ -29,6 +30,12 @@ namespace Microsoft.Maui.Controls
defaultValue: null, propertyChanged: VisualStateGroupsPropertyChanged,
defaultValueCreator: bindable => new VisualStateGroupList(true) { VisualElement = (VisualElement)bindable });
+ static readonly BindablePropertyKey IsListeningForDynamicResourceStateChangesPropertyKey =
+ BindableProperty.CreateAttachedReadOnly("IsListeningForDynamicResourceStateChanges", typeof(bool), typeof(VisualElement), false);
+
+ static readonly BindableProperty IsListeningForDynamicResourceStateChangesProperty =
+ IsListeningForDynamicResourceStateChangesPropertyKey.BindableProperty;
+
static void VisualStateGroupsPropertyChanged(BindableObject bindable, object oldValue, object newValue)
{
if (oldValue is VisualStateGroupList { VisualElement: { } oldElement } oldVisualStateGroupList)
@@ -60,6 +67,7 @@ namespace Microsoft.Maui.Controls
((VisualStateGroupList)newValue).VisualElement = visualElement;
visualElement.ChangeVisualState();
+ UpdateDynamicResourceStateListener(visualElement, newValue as VisualStateGroupList);
UpdateStateTriggers(visualElement);
}
@@ -87,6 +95,9 @@ namespace Microsoft.Maui.Controls
/// <param name="name">The name of the visual state to transition to.</param>
/// <returns><see langword="true"/> if the transition was successful; otherwise, <see langword="false"/>.</returns>
public static bool GoToState(VisualElement visualElement, string name)
+ => GoToState(visualElement, name, force: false);
+
+ internal static bool GoToState(VisualElement visualElement, string name, bool force)
{
var context = visualElement.GetContext(VisualStateGroupsProperty);
if (context is null)
@@ -108,12 +119,6 @@ namespace Microsoft.Maui.Controls
foreach (VisualStateGroup group in groups)
{
- if (group.CurrentState?.Name == name)
- {
- // We're already in the target state; nothing else to do
- return true;
- }
-
// See if this group contains the new state
var target = group.GetState(name);
if (target == null)
@@ -121,6 +126,13 @@ namespace Microsoft.Maui.Controls
continue;
}
+ if (group.CurrentState?.Name == name && !force)
+ {
+ // We're already in the target state; nothing else to do
+ UpdateDynamicResourceStateListener(visualElement, groups);
+ return true;
+ }
+
// If we've got a new state to transition to, unapply the setters from the current state
if (group.CurrentState != null)
{
@@ -150,12 +162,148 @@ namespace Microsoft.Maui.Controls
setter.Apply(visualElement, applySpecificity);
}
+ UpdateDynamicResourceStateListener(visualElement, groups);
return true;
}
+ UpdateDynamicResourceStateListener(visualElement, groups);
+ return false;
+ }
+
+ static void UpdateDynamicResourceStateListener(VisualElement visualElement, VisualStateGroupList groups)
+ {
+ var shouldListen = HasCurrentStateDynamicResourceSetter(groups);
+ var isListening = (bool)visualElement.GetValue(IsListeningForDynamicResourceStateChangesProperty);
+
+ if (shouldListen && !isListening)
+ {
+ ((IElementDefinition)visualElement).AddResourcesChangedListener(OnVisualElementResourcesChanged);
+ visualElement.SetValue(IsListeningForDynamicResourceStateChangesPropertyKey, true);
+ }
+ else if (!shouldListen && isListening)
+ {
+ ((IElementDefinition)visualElement).RemoveResourcesChangedListener(OnVisualElementResourcesChanged);
+ visualElement.SetValue(IsListeningForDynamicResourceStateChangesPropertyKey, false);
+ }
+ }
+
+ static bool HasCurrentStateDynamicResourceSetter(VisualStateGroupList groups)
+ {
+ if (groups?.IsDefault != false)
+ {
+ return false;
+ }
+
+ foreach (var group in groups)
+ {
+ if (HasDynamicResourceSetter(group.CurrentState))
+ {
+ return true;
+ }
+ }
+
+ return false;
+ }
+
+ static bool HasDynamicResourceSetter(VisualState state)
+ {
+ if (state is null)
+ {
+ return false;
+ }
+
+ foreach (var setter in state.Setters)
+ {
+ if (setter.Value is DynamicResource || setter.Value is AppThemeBinding)
+ {
+ return true;
+ }
+ }
+
return false;
}
+ static void OnVisualElementResourcesChanged(object sender, ResourcesChangedEventArgs e)
+ {
+ // sender is the subscribed element (Element.OnResourcesChanged invokes handlers with `this`).
+ if (sender is not VisualElement visualElement)
+ {
+ return;
+ }
+
+ var context = visualElement.GetContext(VisualStateGroupsProperty);
+ if (context is null)
+ {
+ UpdateDynamicResourceStateListener(visualElement, null);
+ return;
+ }
+
+ var vsgSpecificityValue = context.Values.GetSpecificityAndValue();
+ var groups = (VisualStateGroupList)vsgSpecificityValue.Value;
+ if (groups?.IsDefault != false)
+ {
+ UpdateDynamicResourceStateListener(visualElement, groups);
+ return;
+ }
+
+ var changedKeys = e.Values?
+ .Select(v => v.Key)
+ .ToHashSet(StringComparer.Ordinal);
+
+ if (changedKeys is null || changedKeys.Count == 0)
+ {
+ return;
+ }
+
+ var baseSpecificity = vsgSpecificityValue.Key.CopyStyle(1, 0, 0, 0);
+ foreach (var group in groups)
+ {
+ var currentState = group.CurrentState;
+ if (!HasDynamicResourceSetter(currentState))
+ {
+ continue;
+ }
+
+ var stateSpecificity = IsSystemDrivenState(currentState.Name)
+ ? baseSpecificity.WithFullVsmPriority()
+ : baseSpecificity;
+
+ foreach (var setter in currentState.Setters)
+ {
+ if (!SetterDependsOnChangedResource(setter, changedKeys))
+ {
+ continue;
+ }
+
+ setter.UnApply(visualElement, stateSpecificity);
+ setter.Apply(visualElement, stateSpecificity);
+ }
+ }
+
+ UpdateDynamicResourceStateListener(visualElement, groups);
+ }
+
+ static bool SetterDependsOnChangedResource(Setter setter, HashSet<string> changedKeys)
+ {
+ if (setter?.Value is DynamicResource dynamicResource)
+ {
+ return changedKeys.Contains(dynamicResource.Key);
+ }
+
+ if (setter?.Value is AppThemeBinding appThemeBinding)
+ {
+ return IsDynamicResourceValueChanged(appThemeBinding.Light, changedKeys) ||
+ IsDynamicResourceValueChanged(appThemeBinding.Dark, changedKeys) ||
+ IsDynamicResourceValueChanged(appThemeBinding.Default, changedKeys) ||
+ changedKeys.Contains(AppThemeBinding.AppThemeResource);
+ }
+
+ return false;
+ }
+
+ static bool IsDynamicResourceValueChanged(object resourceValue, HashSet<string> changedKeys) =>
+ resourceValue is DynamicResource dynamicResource && changedKeys.Contains(dynamicResource.Key);
+
/// <summary>
/// Returns <see langword="true"/> for states that the MAUI framework drives automatically
/// (Disabled, Focused, Unfocused, Selected, PointerOver, Pressed).
MauiBot
added
s/agent-changes-requested
Note
Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment whether this change resolves your issue. Thank you!
Issue Details
Flyout items were keeping the old selected background color after selection changed, especially with DynamicResource colors and recycled cells.
Description of Change
Core: Added a forced visual-state reapply path in VisualStateManager so controls can re-enter the current state when resource values change, instead of no-oping when the requested state name matches the active state.
iOS: Updated UIContainerCell.cs to listen for resource changes from both the cell view and its binding context, and to force the selected visual state to refresh when those resources change.
Android: Updated ShellFlyoutRecyclerAdapter.cs so recycled flyout view holders detach listeners from the previous item, attach to the new item, and reapply the current visual state when DynamicResource values change.
Windows: Updated ShellFlyoutItemView.cs to correctly manage resource listeners during context changes and to force the visual state to reapply after relevant resource updates.
Issues Fixed
Fixes #34931
Tested the behavior in the following platforms.
Before-fix.mov
After-fix.mov