Fix implicit styles with BasedOn chains breaking in 10.0.20 #33218
+334
−60
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This fixes issue #33203 where implicit styles using BasedOn with keyed styles stopped working in 10.0.20. The regression was introduced by PR #32711 which fixed #9648 (ApplyToDerivedTypes not working for implicit styles). The problem was that when merging multiple applicable implicit styles, the code created wrapper styles that replaced the original BasedOn chain, losing the connection to keyed styles that defined the actual property values. The fix changes the approach: instead of creating wrapper styles that break BasedOn chains, we now store the list of applicable implicit styles and apply them sequentially during Apply(). This preserves each style's BasedOn chain intact while still supporting the ApplyToDerivedTypes functionality from #9648. Fixes #33203
StephaneDelcroix
added
t/bug
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a regression in .NET MAUI 10.0.20 where implicit styles using BasedOn chains with keyed styles stopped working. The issue was introduced by PR #32711 which fixed ApplyToDerivedTypes functionality.
Key Changes
- Changed merging strategy: Instead of creating wrapper styles that broke
BasedOnchains, the fix now stores multiple applicable implicit styles in a list and applies them sequentially - Preserved BasedOn chains: Each style's
BasedOnchain remains intact, ensuring property values from keyed styles are properly resolved - Maintained ApplyToDerivedTypes support: The fix continues to support multiple implicit styles from the type hierarchy while fixing the regression
Comment on lines
111
to
127
| // Apply implicit styles from most base to most derived | ||
| // This ensures derived styles override base styles for conflicting properties | ||
| if (_applicableImplicitStyles != null && _applicableImplicitStyles.Count > 0) | ||
| { | ||
| // Apply in reverse order (most base first, most derived last) | ||
| // so derived styles override base styles | ||
| for (int i = _applicableImplicitStyles.Count - 1; i >= 0; i--) | ||
| { | ||
| //NOTE specificity could be more fine grained (using distance) | ||
| ((IStyle)_applicableImplicitStyles[i]).Apply(bindable, new SetterSpecificity(SetterSpecificity.StyleImplicit, 0, 0, 0)); | ||
| } | ||
| } | ||
| else | ||
| { | ||
| //NOTE specificity could be more fine grained (using distance) | ||
| ImplicitStyle?.Apply(bindable, new SetterSpecificity(SetterSpecificity.StyleImplicit, 0, 0, 0)); | ||
| } |
There was a problem hiding this comment.
The logic for applying and unapplying multiple implicit styles is duplicated across multiple methods (Apply() at lines 113-127, UnApply() at lines 145-155, and SetStyle() at lines 254-264 and 275-286). Consider extracting this logic into private helper methods like ApplyImplicitStyles(BindableObject target) and UnApplyImplicitStyles(BindableObject target) to eliminate duplication and ensure consistency. This would improve maintainability and reduce the risk of introducing bugs when updating the style application logic in the future.
Comment on lines
+202
to
+204
| // Set ImplicitStyle to the most derived style for compatibility | ||
| // (some code may check if ImplicitStyle is set) | ||
| ImplicitStyle = applicableStyles[0]; |
There was a problem hiding this comment.
Setting ImplicitStyle to the most derived style (applicableStyles[0]) when multiple applicable styles exist may lead to unexpected behavior or confusion. Code that checks if ImplicitStyle is set may not realize there are multiple styles being applied. Consider either:
- Setting ImplicitStyle to null when multiple styles are present to make it clear that the _applicableImplicitStyles list should be used instead
- Adding a property or method to indicate when multiple implicit styles are active
This would make the multi-style state more explicit and prevent potential misuse of the ImplicitStyle property.
Suggested change
| // Set ImplicitStyle to the most derived style for compatibility | |
| // (some code may check if ImplicitStyle is set) | |
| ImplicitStyle = applicableStyles[0]; | |
| // When multiple implicit styles are active, leave ImplicitStyle null | |
| // so callers know to consult _applicableImplicitStyles instead. | |
| ImplicitStyle = null; |
Tests verify that: 1. Implicit styles with BasedOn to keyed styles work correctly 2. Derived implicit styles properly override base styles' properties
jfversluis
previously approved these changes
Dec 19, 2025
When a base style uses AppThemeBinding for a property and a derived style overrides that property with a static value, the AppThemeBinding would re-apply when theme changes or resources propagate, overwriting the derived style's value. Changes: - AppThemeBinding.Apply(fromTarget): Skip re-apply when value is set from target - AppThemeBinding.ApplyCore(): Check if current value was set by this binding before re-applying (using IsValueFromThisBinding helper) - MergedStyle: Apply graduated specificities for multiple implicit styles Added unit test Maui33203AppTheme to verify derived style setters override base style's AppThemeBinding in both Light and Dark modes.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 if this change resolves your issue. Thank you!
Description of Change
This fixes issue #33203 where implicit styles using
BasedOnwith keyed styles stopped working in 10.0.20.Root Cause
The regression was introduced by PR #32711 which fixed #9648 (
ApplyToDerivedTypesnot working for implicit styles). That fix created wrapper styles to merge multiple applicable implicit styles, but in doing so it replaced the originalBasedOnchain with a new one, losing the connection to keyed styles that defined the actual property values.The Fix
Instead of creating wrapper styles that break
BasedOnchains, we now store the list of applicable implicit styles and apply them sequentially duringApply(). This preserves each style'sBasedOnchain intact while still supporting theApplyToDerivedTypesfunctionality from #9648.Test Results
ApplyToDerivedTypesfix) still passBasedOn) now works correctlyIssues Fixed
Fixes #33203