Skip to content

Fix ApplyToDerivedTypes not working for implicit styles (#9648)#32711

Merged
jfversluis merged 2 commits intomainfrom
dev/stdelc/fix9648
Nov 20, 2025
Merged

Fix ApplyToDerivedTypes not working for implicit styles (#9648)#32711
jfversluis merged 2 commits intomainfrom
dev/stdelc/fix9648

Conversation

@StephaneDelcroix
Copy link
Copy Markdown
Contributor

@StephaneDelcroix StephaneDelcroix commented Nov 18, 2025

This fixes issue #9648 where ApplyToDerivedTypes was not working correctly for implicit styles. Only the most specific style was being applied, and base class styles were being ignored.

Changes:

  • Fixed MergedStyle.OnImplicitStyleChanged() to collect and merge all applicable implicit styles from the type hierarchy
  • Added unit tests (Maui9648) that verify the fix works correctly
  • Added performance benchmarks to measure impact

Test Results:

  • New tests: 6/6 PASS (all inflators: Runtime, XamlC, SourceGen)
  • Existing tests: 7,159/7,159 PASS (0 regressions)

Performance Impact (measured with BenchmarkDotNet):

  • Single implicit style: 1.68x baseline (minimal overhead, optimized)
  • Two implicit styles: 3.35x baseline (+11.5 μs per element)
  • Three implicit styles: 5.33x baseline (+21 μs per element)
  • Overhead is one-time cost at element creation, not per-render
  • Real-world impact: Imperceptible for typical usage (< 1-2ms per page)

Fixes #9648

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

Issues Fixed

Fixes #9648

This fixes issue #9648 where ApplyToDerivedTypes was not working correctly
for implicit styles. Only the most specific style was being applied, and base
class styles were being ignored.

Changes:
- Fixed MergedStyle.OnImplicitStyleChanged() to collect and merge all
  applicable implicit styles from the type hierarchy
- Added unit tests (Maui9648) that verify the fix works correctly
- Added performance benchmarks to measure impact

Test Results:
- New tests: 6/6 PASS (all inflators: Runtime, XamlC, SourceGen)
- Existing tests: 7,159/7,159 PASS (0 regressions)

Performance Impact (measured with BenchmarkDotNet):
- Single implicit style: 1.68x baseline (minimal overhead, optimized)
- Two implicit styles: 3.35x baseline (+11.5 μs per element)
- Three implicit styles: 5.33x baseline (+21 μs per element)
- Overhead is one-time cost at element creation, not per-render
- Real-world impact: Imperceptible for typical usage (< 1-2ms per page)

Fixes #9648
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes issue #9648 where ApplyToDerivedTypes was not working correctly for implicit styles. Previously, only the most specific implicit style was applied to elements, ignoring styles from base types in the hierarchy. The fix modifies MergedStyle.OnImplicitStyleChanged() to collect all applicable implicit styles from the type hierarchy and merge them using style chaining.

Key Changes:

  • Modified the implicit style resolution algorithm to collect all applicable styles instead of returning early after finding the first one
  • Implemented style merging by creating a BasedOn chain from base to derived types
  • Added optimization for single-style case to avoid overhead
  • Added comprehensive unit tests and performance benchmarks

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
src/Controls/src/Core/MergedStyle.cs Core fix - collects and merges multiple implicit styles using BasedOn chaining
src/Controls/tests/Xaml.UnitTests/Issues/Maui9648.xaml Test XAML with three-level style hierarchy (InputView → Entry → CustomEntry)
src/Controls/tests/Xaml.UnitTests/Issues/Maui9648.xaml.cs Unit tests verifying style inheritance works correctly
src/Controls/tests/Xaml.Benchmarks/ImplicitStylesBenchmark.cs Performance benchmarks measuring overhead of style merging

Comment on lines +197 to +214
// Copy behaviors if any exist
if (currentStyle.Behaviors.Count > 0)
{
foreach (var behavior in currentStyle.Behaviors)
{
wrapperStyle.Behaviors.Add(behavior);
}
}

// Copy triggers if any exist
if (currentStyle.Triggers.Count > 0)
{
foreach (var trigger in currentStyle.Triggers)
{
wrapperStyle.Triggers.Add(trigger);
}
}

Copy link

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Behaviors and triggers should not be copied into the wrapper style. Since the wrapper style has BasedOn = mergedStyle (line 186), behaviors and triggers from base styles will already be applied through the BasedOn chain when the style is applied (see Style.ApplyCore at line 166 which applies the BasedOn style first). Copying them here will cause behaviors and triggers to be attached multiple times to the same element, leading to duplicate behavior execution and potential state management issues. Remove the behavior and trigger copying logic (lines 197-213).

Suggested change
// Copy behaviors if any exist
if (currentStyle.Behaviors.Count > 0)
{
foreach (var behavior in currentStyle.Behaviors)
{
wrapperStyle.Behaviors.Add(behavior);
}
}
// Copy triggers if any exist
if (currentStyle.Triggers.Count > 0)
{
foreach (var trigger in currentStyle.Triggers)
{
wrapperStyle.Triggers.Add(trigger);
}
}

Copilot uses AI. Check for mistakes.
Comment thread src/Controls/src/Core/MergedStyle.cs Outdated
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@StephaneDelcroix
Copy link
Copy Markdown
Contributor Author

/azp run MAUI-UITests-public

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@jfversluis jfversluis merged commit 0af93ce into main Nov 20, 2025
161 of 163 checks passed
@jfversluis jfversluis deleted the dev/stdelc/fix9648 branch November 20, 2025 13:39
StephaneDelcroix added a commit that referenced this pull request Dec 18, 2025
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
@github-actions github-actions Bot locked and limited conversation to collaborators Dec 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ApplyToDerivedTypes not working for implicit styles

3 participants