Improved style inheritance

[kubaflo]

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

Corrects style inheritance in Style.ApplyCore by unapplying the base style before reapplying it.

Issues Fixed

Fixes #31280

[Copilot]

Pull Request Overview

This PR fixes a style inheritance issue where derived styles were not properly inheriting from their base styles when properties were overridden. The fix ensures that when a style is applied, the base style is first unapplied and then reapplied, allowing proper inheritance behavior.

• Corrects style inheritance by unapplying base styles before reapplying them in Style.ApplyCore
• Adds comprehensive unit tests to verify style inheritance works correctly across multiple levels
• Validates that overridden properties in derived styles are properly applied while maintaining base style properties

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
src/Controls/src/Core/Style.cs	Core fix adding base style unapplication before reapplication in ApplyCore method
src/Controls/tests/Xaml.UnitTests/Issues/Issue31280.xaml	Test XAML defining hierarchical styles with property overrides to validate inheritance
src/Controls/tests/Xaml.UnitTests/Issues/Issue31280.xaml.cs	Unit test class verifying that style inheritance works correctly for both compiled and non-compiled XAML

[dotnet-policy-service]

Hey there @@kubaflo! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

[github-actions]

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 31317

Or

• Run remotely in PowerShell:

iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 31317"

[StephaneDelcroix]

this looks good, but Style changes have a tendency to create regressions, I'd recommend targeting net11.0

also, xaml unittests no longer use nunit but xunit

[StephaneDelcroix]

Multi-Model Consensus Review (5 models: claude-opus-4.6 ×2, claude-sonnet-4.6, gemini-3-pro-preview, gpt-5.3-codex)

CI Status: ✅ All checks passing

Consensus Findings

No consensus bugs found (5/5 models clean).

One model raised a potential re-entrancy concern via RemoveDynamicResource → OnBasedOnResourceChanged but this was not confirmed by any other model and does not reach consensus threshold.

Test Coverage

✅ Includes XAML test (Issue31280.xaml) and unit test (Issue31280.xaml.cs) exercising multi-level style inheritance with property overrides.

Verdict: ✅ Approve

Minimal, correct fix with proper safety guard. No regressions identified.

[StephaneDelcroix]

rename "Issue31280" to "Maui31280". IssueXXXXX used to track issues in the Xamarin.Forms repository, MauiXXXXX on maui

[kubaflo]

/azp run maui-pr

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[MauiBot]

🤖 AI Summary

📊 Expand Full Review — 7b440a8 · Update Maui31280.xaml.cs

---

🔍 Pre-Flight — Context & Validation

---

Issue: #31280 - Styles based on a style that is based on another style that uses AppThemeBinding do not inherit properties correctly.
PR: #31317 - Improved style inheritance
Platforms Affected: iOS, Android, Windows, macOS
Files Changed: 1 implementation, 2 test

Key Findings

• The PR changes src/Controls/src/Core/Style.cs to unapply a base style before reapplying it inside Style.ApplyCore, targeting inherited-style behavior when AppThemeBinding is involved.
• The linked issue is verified on all platforms and describes a three-level style inheritance case where the third style incorrectly resolves to the original base color instead of the overridden inherited color.
• The PR adds a XAML unit test under src/Controls/tests/Xaml.UnitTests/Issues/ to validate the inheritance chain and expected TextColor values.
• Prior human review called out repository conventions: XAML unit tests now use xUnit and MAUI issue tests should use the MauiXXXXX naming pattern rather than IssueXXXXX.
• No prior PRAgent phase output was found in PR comments; this run starts from pre-flight.

Edge Cases / Discussion Notes

• The issue reproduces only when the base style uses AppThemeBinding; simple literal values do not exhibit the problem.
• The reported workaround is to avoid AppThemeBinding in the base style.
• Because this PR uses XAML unit tests rather than HostApp UI tests, gate verification will need to use the XAML unit test project rather than the UI-test verification path.

Fix Candidates

#	Source	Approach	Test Result	Files Changed	Notes
PR	PR #31317	Unapply base style before reapplying it during Style.ApplyCore, plus add a regression XAML unit test covering multi-level style inheritance with AppThemeBinding.	⏳ PENDING (Gate)	src/Controls/src/Core/Style.cs, src/Controls/tests/Xaml.UnitTests/Issues/Maui31280.xaml, src/Controls/tests/Xaml.UnitTests/Issues/Maui31280.xaml.cs	Original PR

---

🚦 Gate — Test Verification

---

Gate Result: ❌ FAILED

Platform: android (host context); verification executed with platform-agnostic XAML unit tests because this PR adds no HostApp/UI test coverage.
Mode: Exact PR Validation
Test Project: src/Controls/tests/Xaml.UnitTests/Controls.Xaml.UnitTests.csproj
Test Filter: StyleInheritanceShouldWork

• Tests FAIL without fix: ✅
• Tests PASS with fix: ❌

Evidence

• The underlying regression is real: when the Style.ApplyCore change is removed, the regression reproduces and the deepest style resolves TextColor to green instead of red.
• However, the PR as submitted does not currently pass the new test project build because src/Controls/tests/Xaml.UnitTests/Issues/Maui31280.xaml.cs still imports NUnit.Framework in an xUnit test project.
• Exact PR-state validation fails with compile errors before test execution:
  • error CS0246: The type or namespace name 'NUnit' could not be found
  • error CS0246: The type or namespace name 'CollectionAttribute' could not be found
• As a result, the PR fix is not merge-ready in its current form even though the Style.cs logic change itself appears to address the reported behavior once the test file is corrected.

---

🔧 Fix — Analysis & Comparison

---

Fix Candidates

#	Source	Approach	Test Result	Files Changed	Notes
1	try-fix (claude-opus-4.6)	Detect style re-application in IStyle.Apply and call UnApplyCore before reapplying so stale AppThemeBinding state is cleared by the style itself.	✅ PASS*	2 files	Passed once the test file import was corrected to xUnit.
2	try-fix (claude-sonnet-4.6)	Replace the static OnBasedOnResourceChanged callback with a style-owned callback that captures the specific Style instance and reapplies the correct owner in ApplyCore.	✅ PASS*	2 files	Passed once the test file import was corrected to xUnit.
3	try-fix (gpt-5.3-codex)	Keep the callback static but route OnBasedOnResourceChanged to the matching style node in the BasedOn chain before unapply/reapply.	✅ PASS*	1 file	Passed once the test file import was corrected to xUnit.
4	try-fix (gemini-3-pro-preview)	Validate resource identity in OnBasedOnResourceChanged so nested style updates are ignored unless they match the root style's BaseResourceKey.	✅ PASS*	2 files	Passed once the test file import was corrected to xUnit.
5	try-fix (claude-opus-4.6 round 2)	Invert ApplyCore ordering so derived setters run before BasedOn, attempting to let derived values win via specificity/ordering instead of cleanup.	❌ FAIL	2 files	Fundamentally incompatible with persistent AppThemeBinding listeners.
6	try-fix (claude-sonnet-4.6 round 2)	Flatten the BasedOn chain into a winner map, unapply overridden base setters, and add a re-entrancy guard for the callback.	✅ PASS*	2 files	Works, but substantially more complex than the PR fix.
PR	PR #31317	Unapply the base style before reapplying it inside Style.ApplyCore.	❌ FAIL (Gate)	3 files	Exact PR state fails to compile because Maui31280.xaml.cs uses NUnit.Framework in an xUnit project.

* All passing try-fix attempts also corrected the test file import from using NUnit.Framework; to using Xunit;, which is required for the regression test project to compile.

Cross-Pollination

Model	Round	New Ideas?	Details
claude-opus-4.6	1	Yes	Self-cleaning re-application in IStyle.Apply
claude-sonnet-4.6	1	Yes	Make _basedOnResourceProperty callback style-owned instead of static/top-style-driven
gpt-5.3-codex	1	Yes	Resolve callback to matching owner within the BasedOn chain
gemini-3-pro-preview	1	Yes	Validate resource identity to filter callback cross-talk
claude-opus-4.6	2	Yes	Invert ApplyCore order so base runs after derived at lower priority
claude-sonnet-4.6	2	Yes	Flatten the BasedOn chain into one effective setter map
gpt-5.3-codex	2	No	NO NEW IDEAS
gemini-3-pro-preview	2	No	NO NEW IDEAS
claude-opus-4.6	3	Yes	Add per-property style-precedence tokens to specificity/value provenance
claude-sonnet-4.6	3	Yes	Apply AppThemeBinding through binding-engine precedence instead of value assignment
gpt-5.3-codex	3	Yes	Preserve lowered specificity inside AppThemeBinding callback updates
gemini-3-pro-preview	3	Yes	Remove stale bindings in Setter.Apply before setting plain values

Exhausted: Partial — exploration found additional theoretical directions, but the exact PR state already fails gate due to a concrete test compile issue, so further alternative execution was not necessary to reach a recommendation.
Selected Fix: Keep the PR's Style.ApplyCore logic and also correct src/Controls/tests/Xaml.UnitTests/Issues/Maui31280.xaml.cs to use xUnit (using Xunit;). That is the simplest merge-ready path.

---

📋 Report — Final Recommendation

---

⚠️ Final Recommendation: REQUEST CHANGES

Phase Status

Phase	Status	Notes
Pre-Flight	✅ COMPLETE	Small Style fix plus new XAML regression test; issue verified on all platforms.
Gate	❌ FAILED	Exact PR state does not build the new XAML test on the android-hosted review environment because Maui31280.xaml.cs imports NUnit.Framework in an xUnit project.
Try-Fix	✅ COMPLETE	6 executed attempts: 5 passing alternatives (all required the xUnit import correction), 1 failing alternative that confirmed cleanup of persistent AppThemeBinding state matters.
Report	✅ COMPLETE

Summary

The Style.ApplyCore logic change appears directionally correct: when I validated the behavior with the test file corrected to xUnit, the regression failed without the Style.cs change and passed with it. However, the PR as submitted is not merge-ready because the newly added test file still uses NUnit.Framework, so the XAML unit test project fails to compile before the regression can run.

Root Cause

There are two separate issues in the current PR state:

1. Behavioral bug being fixed by the PR: multi-level BasedOn style application combined with AppThemeBinding can leave the deepest style inheriting the original base value (Green) instead of the intermediate override (Red).
2. New test wiring bug in the PR itself: src/Controls/tests/Xaml.UnitTests/Issues/Maui31280.xaml.cs imports NUnit.Framework, but Controls.Xaml.UnitTests uses xUnit. That causes compile errors (CS0246 for NUnit, CollectionAttribute, etc.) in exact PR-state validation.

Fix Quality

The implementation in src/Controls/src/Core/Style.cs is small and plausible, and the try-fix phase found multiple alternative passing strategies — but none were clearly better once complexity was considered. The simplest path is to keep the PR’s Style.ApplyCore logic and fix the regression test file to use xUnit (using Xunit;). Until that test import issue is corrected, I would not approve the PR.

---

[kubaflo]

/azp run maui-pr-uitests

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[kubaflo]

Code Review — PR #31317

Independent Assessment

What this changes: In Style.ApplyCore, adds an ((IStyle)basedOn).UnApply(bindable) call before ((IStyle)basedOn).Apply(...) so that when a style inheritance chain is (re-)applied, stale property values from a previous base-style application are cleared before the fresh application. Adds a XAML unit test exercising a 3-level style chain with AppThemeBinding in the root style.

Inferred motivation: When a derived style overrides a property set by a base style that uses AppThemeBinding, the override gets lost during re-application because the base style's old values aren't cleared first.

Reconciliation with PR Narrative

Author claims: "Corrects style inheritance in Style.ApplyCore by unapplying the base style before reapplying it." Fixes #31280.
Agreement: The core fix matches the claim and the approach is sound. Calling UnApply before Apply ensures a clean slate. UnApply on a never-applied style is safe (no-ops through specificities.TryGetValue guard). Reviewer @StephaneDelcroix requested renaming to Maui31280 and migration to xUnit — the renaming was done but the xUnit migration has issues.

Findings

❌ Critical — Wrong using import causes compilation failure

File: Maui31280.xaml.cs:3
The test file imports using NUnit.Framework; but uses xUnit APIs throughout: [Collection("Issue")] (xUnit), [Theory] (ambiguous), and Assert.Equal (xUnit — NUnit uses Assert.AreEqual). This project uses xUnit. Should be using Xunit;. This is almost certainly the cause of the CI build failures.

⚠️ Important — Missing AppThemeBinding test infrastructure setup

File: Maui31280.xaml.cs:14-16 (the Tests class)
The XAML uses {AppThemeBinding Light=Green, Dark=Green} but the test class doesn't set up the required infrastructure. Every other test in this project that uses AppThemeBinding (e.g., Maui23201.xaml.cs, Maui19388.xaml.cs) follows this pattern:

[Collection("Issue")]
public class Tests : IDisposable
{
    public Tests()
    {
        Application.SetCurrentApplication(new MockApplication());
        DispatcherProvider.SetCurrent(new DispatcherProviderStub());
    }

    public void Dispose()
    {
        AppInfo.SetCurrent(null);
        Application.SetCurrentApplication(null);
        DispatcherProvider.SetCurrent(null);
    }
    // ...
}

Without this, the AppThemeBinding may not resolve correctly, meaning the test could pass for the wrong reasons or fail unexpectedly.

⚠️ Minor — Missing newlines at end of files

Files: Maui31280.xaml and Maui31280.xaml.cs
Both files are missing the trailing newline (\ No newline at end of file in diff). All other files in this directory have trailing newlines.

Devil's Advocate

• Is the UnApply-before-Apply safe on first application? Yes — UnApplyCore early-returns via specificities.TryGetValue when the style was never applied, and _targets.Remove is a no-op. Safe.
• Re-entrancy concern? UnApply calls RemoveDynamicResource which could trigger OnBasedOnResourceChanged, potentially causing recursive unapply/apply. However, OnBasedOnResourceChanged looks up this style (the parent) from (bindable as IStyleElement).Style, and the _targets.TryGetValue guard should prevent double-processing. The multi-model review (5 models) found no confirmed bug here.
• Should this target net11.0 instead of main? @StephaneDelcroix flagged this: "Style changes have a tendency to create regressions, I'd recommend targeting net11.0." This is a process/risk decision for the team. The fix is minimal (2 lines), but the Style system is notoriously fragile.

Verdict: NEEDS_CHANGES

Confidence: high
Summary: The core 2-line fix in Style.cs is correct and well-motivated. However, the test file has a wrong using import (NUnit.Framework instead of Xunit) that likely causes CI build failures, and it's missing the required AppThemeBinding infrastructure setup (MockApplication, DispatcherProvider) that all similar tests in the project use. Fix these two issues and CI should go green.

[kubaflo]

🟡 .NET MAUI Review - Changes Suggested

Expand Full Review - 7b440a8 - Improved style inheritance

---

Independent Assessment

What this changes: In Style.ApplyCore, adds an ((IStyle)basedOn).UnApply(bindable) call before ((IStyle)basedOn).Apply(...) so that when a style inheritance chain is (re-)applied, stale property values from a previous base-style application are cleared before the fresh application. Adds a XAML unit test exercising a 3-level style chain with AppThemeBinding in the root style.

Inferred motivation: When a derived style overrides a property set by a base style that uses AppThemeBinding, the override gets lost during re-application because the base style's old values aren't cleared first.

Reconciliation with PR Narrative

Author claims: "Corrects style inheritance in Style.ApplyCore by unapplying the base style before reapplying it." Fixes #31280.
Agreement: The core fix matches the claim and the approach is sound. Calling UnApply before Apply ensures a clean slate. UnApply on a never-applied style is safe (no-ops through specificities.TryGetValue guard). Reviewer @StephaneDelcroix requested renaming to Maui31280 and migration to xUnit — the renaming was done but the xUnit migration has issues.

Findings

❌ Critical — Wrong using import causes compilation failure

File: Maui31280.xaml.cs:3
The test file imports using NUnit.Framework; but uses xUnit APIs throughout: [Collection("Issue")] (xUnit), [Theory] (ambiguous), and Assert.Equal (xUnit — NUnit uses Assert.AreEqual). This project uses xUnit. Should be using Xunit;. This is almost certainly the cause of the CI build failures.

⚠️ Important — Missing AppThemeBinding test infrastructure setup

File: Maui31280.xaml.cs:14-16 (the Tests class)
The XAML uses {AppThemeBinding Light=Green, Dark=Green} but the test class doesn't set up the required infrastructure. Every other test in this project that uses AppThemeBinding (e.g., Maui23201.xaml.cs, Maui19388.xaml.cs) follows this pattern:

[Collection("Issue")]
public class Tests : IDisposable
{
    public Tests()
    {
        Application.SetCurrentApplication(new MockApplication());
        DispatcherProvider.SetCurrent(new DispatcherProviderStub());
    }

    public void Dispose()
    {
        AppInfo.SetCurrent(null);
        Application.SetCurrentApplication(null);
        DispatcherProvider.SetCurrent(null);
    }
    // ...
}

Without this, the AppThemeBinding may not resolve correctly, meaning the test could pass for the wrong reasons or fail unexpectedly.

⚠️ Minor — Missing newlines at end of files

Files: Maui31280.xaml and Maui31280.xaml.cs
Both files are missing the trailing newline (\ No newline at end of file in diff). All other files in this directory have trailing newlines.

Devil's Advocate

• Is the UnApply-before-Apply safe on first application? Yes — UnApplyCore early-returns via specificities.TryGetValue when the style was never applied, and _targets.Remove is a no-op. Safe.
• Re-entrancy concern? UnApply calls RemoveDynamicResource which could trigger OnBasedOnResourceChanged, potentially causing recursive unapply/apply. However, OnBasedOnResourceChanged looks up this style (the parent) from (bindable as IStyleElement).Style, and the _targets.TryGetValue guard should prevent double-processing. The multi-model review (5 models) found no confirmed bug here.
• Should this target net11.0 instead of main? @StephaneDelcroix flagged this: "Style changes have a tendency to create regressions, I'd recommend targeting net11.0." This is a process/risk decision for the team. The fix is minimal (2 lines), but the Style system is notoriously fragile.

Verdict: NEEDS_CHANGES

Confidence: high
Summary: The core 2-line fix in Style.cs is correct and well-motivated. However, the test file has a wrong using import (NUnit.Framework instead of Xunit) that likely causes CI build failures, and it's missing the required AppThemeBinding infrastructure setup (MockApplication, DispatcherProvider) that all similar tests in the project use. Fix these two issues and CI should go green.

[kubaflo]

/azp run maui-pr-uitests maui-pr-devicetests

[azure-pipelines]

No pipelines are associated with this pull request.