TabViewItem should consume Foreground/Background properties

[Felix-Dev]

Describe the bug
The TabViewtem currently ignores the Foreground and Background properties. While, for example, overriding theme resources such as TabViewItemHeaderBackground can be used to set the background color of tabs, developers rightfully expect to be able to use the aforementioned properties as well.

See for example the following XAML:

<muxc:TabView>
    <muxc:TabView.TabItems>
        <muxc:TabViewItem Header="Tab 1">
            <muxc:TabViewItem.IconSource>
                <muxc:SymbolIconSource Symbol="Home" />
            </muxc:TabViewItem.IconSource>
        </muxc:TabViewItem>
        <muxc:TabViewItem Header="Tab 2" Foreground="Orange" Background="Green">
            <muxc:TabViewItem.IconSource>
                <muxc:SymbolIconSource Symbol="Emoji" />
            </muxc:TabViewItem.IconSource>
        </muxc:TabViewItem>
    </muxc:TabView.TabItems>
</muxc:TabView>

The expected look would be:

but the actual look is this (neither Foreground nor Background value is respected):

Version Info

NuGet package version:
Microsoft.UI.Xaml2.5.0-prerelease.200609001

Open Questions
I based the Foreground property affecting both the icon and the Tab header on the fact that they are "content" to be set by the user and the precedence set by the MenuFlyoutItem and the NavigationViewItem:

<muxc:TabViewItem.ContextFlyout>
    <MenuFlyout>
        <MenuFlyoutItem Text="123" Foreground="Red" Icon="Accept"/>
    </MenuFlyout>   
</muxc:TabViewItem.ContextFlyout>

with the corresponding image:

And the NavigationViewItem XAML as follows:

<muxc:NavigationViewItem Content="Menu Item 1" Icon="Home" Foreground="Red">
    <muxc:NavigationViewItem.MenuItems>
        <muxc:NavigationViewItem Content="Menu Item 1.1" Icon="Emoji"/>
    </muxc:NavigationViewItem.MenuItems>
</muxc:NavigationViewItem>

with the resulting look:

I think it is worth thinking about creating a consistent foreground styling behavior here. The specific issue with the TabViewItem compared to the above two controls is that it has an explicit TabViewItemIconForeground theme resource to enable independent icon styling from the rest of the TabViewItem content (like the header). I would like to see some consistency here between all three controls so that either

• setting Foreground affects both Icon and Content/Header/Text in TabViewItem/MenuFlyoutItem/NavigationViewItem or
• we can set the Icon foreground value independently in all three cases (for example via theme resources) and the Foreground property would only affect the Content/Header/Text

Scenario 1 would mean that the specific TabViewItemIconForeground* theme resources would no longer be useful (to prevent a breaking change I would keep them in the resource dictionary with an added comment that they are no longer in use) whereas Scenario 2 should mean that both MenuFlyoutItem and NavigationViewItem will get specific MenuFlyoutItemIconForeground*/NavigationViewItemIconForeground* theme resources and the Foreground API will no longer effect their Icon foreground color.

[StephenLPeters]

I agree, the foreground and background properties should do something. You bring up some good points though, its not clear what exactly the foreground property should do. @stmoy thoughts?

[DHowett]

Terminal is interested in this as well. We have a feature that allows a user to customize her tab colors, but it's implemented by locally overriding the TabViewItemHeader* resources. It's terribly unpleasant.

[Felix-Dev]

Pinging @stmoy for thoughts about the how we should proceed here (given the TabViewItem's unique situation with the Icon and ContentPresenter theme resources).

[mdtauk]

How would inheritance work for this?

[Felix-Dev]

@mdtauk I would use TemplateBinding here.

[stmoy]

Thanks for the ping @Felix-Dev - this slipped past my radar.

Agreed that foreground and background properties should do something as @StephenLPeters mentions. I believe the most straightforward and consistent approach would be to do what MenuFlyout and NavigationView do (Scenario 1, above).

I'm not sure if not using a named brush is a breaking change or not. Sure, you could still set it, but it wouldn't do anything - which is the same as what happens if we remove it altogether, right?

[Felix-Dev]

@stmoy If we remove them altogether, apps which are using them now could end up crashing after updating to a WinUI version where these have been removed, to my understanding. For example, picture someone using them like this (without a theme resource override on an app/page level in scope):

<[SomeClass].Foreground="{ThemeResource TabViewItemIconForeground}"/>

Now, such a usecase arguably looks quite unlikely but if an app does consume those theme resources similar to above then a removal here would mean this app couldn't run any longer (since that referenced resource is now missing) without the developer having to modify their app code. If we keep the resources included, then they won't break apps, but, as you said correctly, they also won't do anything any longer. They will basically be "garbage" at this point...only to be kept because of an arguably small chance someone out there might be using them in such a way that their removal from WinUI would result in an app crash.

[Felix-Dev]

@stmoy What is the status here? Are there still open questions or can we proceed with a PR for this issue (by following the outlined Scenario 1)?

[stmoy]

Thanks for the reminder - let me ping the team. The biggest question is related to "is this a breaking change, and if it is, should we hold off?" - it's a philosophical question so it is taking longer than I'd hope 😅