DP inheritance shouldn't match DP names

Youssef1313 commented 1 month ago

Current behavior

Currently, our implementation of DP inheritance works by matching the DP name and having the Inherits option. This doesn't match WinUI

Expected behavior

In WinUI, it either has to be the exact same DP, or otherwise, it matches against hardcoded set of DPs.

Looking more carefully at what WinUI does, there is InheritedProperties::GetCorrespondingInheritedProperty with the following code:

        case KnownPropertyIndex::RichTextBlock_Foreground:
        case KnownPropertyIndex::TextBlock_Foreground:
        case KnownPropertyIndex::Control_Foreground:
        case KnownPropertyIndex::TextElement_Foreground:
        case KnownPropertyIndex::IconElement_Foreground:
            if (pdo->GetTypeIndex() == KnownTypeIndex::RichTextBlock) { propertyIndex = KnownPropertyIndex::RichTextBlock_Foreground; }
            else if (pdo->GetTypeIndex() == KnownTypeIndex::TextBlock)      { propertyIndex = KnownPropertyIndex::TextBlock_Foreground; }
            else if (pdo->OfTypeByIndex<KnownTypeIndex::Control>())        { propertyIndex = KnownPropertyIndex::Control_Foreground; }
            else if (pdo->OfTypeByIndex<KnownTypeIndex::TextElement>())    { propertyIndex = KnownPropertyIndex::TextElement_Foreground; }
            else if (pdo->OfTypeByIndex<KnownTypeIndex::IconElement>())    { propertyIndex = KnownPropertyIndex::IconElement_Foreground; }
            break;

        // To make AppBarButtons work correctly in Windows Blue, IconElement.Foreground must be able to
        // inherit directly from ContentPresenter.Foreground. This is captured separately from the other
        // Foreground-property cases above, in order to avoid introducing regressions into Win8 apps:
        // Since there was no general linkage between ContentPresenter.Foreground and other controls'
        // Foreground properties previously, we don't want to introduce one now.
        case KnownPropertyIndex::ContentPresenter_Foreground:
            if (pdo->OfTypeByIndex<KnownTypeIndex::IconElement>())    { propertyIndex = KnownPropertyIndex::IconElement_Foreground; }
            break;

It looks like ContentPresenter.Foreground may not be inheriting the Foreground from other classes? etc

How to reproduce it (as minimally and precisely as possible)

No response

Workaround

No response

Works on UWP/WinUI

None

Environment

No response

NuGet package version(s)

No response

Affected platforms

No response

IDE

No response

IDE version

No response

Relevant plugins

No response

Anything else we need to know?

No response

Youssef1313 commented 1 month ago

ContentPresenter's code in WinUI actually pulls such properties from the parent, with one gotcha. If the parent is a PopupRoot, the logical parent is used. Otherwise, the visual parent is used

Youssef1313 commented 2 weeks ago

The assumption here isn't always true. It's possibly true for all SparseGroup DPs though. See findings in https://github.com/unoplatform/uno/pull/17766#issuecomment-2294936199

unoplatform / uno