TabView: Enable separate lightweight styling of the Add button and the TabViewItem's Close button

[Felix-Dev]

Description

This PR improves the TabView lightweight styling story to enable developers to independently style the appearance of the TabViewItem's Close button and the TabView's Add button. Below is a list of all added, changed and now unused theme resources as part of this PR:

Added theme resources

• TabViewItemHeaderCloseButtonBackground
• TabViewItemHeaderCloseButtonBackgroundPressed
• TabViewItemHeaderCloseButtonBackgroundPointerOver
• TabViewItemHeaderPressedCloseButtonBackground
• TabViewItemHeaderPointerOverCloseButtonBackground
• TabViewItemHeaderSelectedCloseButtonBackground
• TabViewItemHeaderDisabledCloseButtonBackground
• TabViewItemHeaderCloseButtonForeground
• TabViewItemHeaderCloseButtonForegroundPressed
• TabViewItemHeaderCloseButtonForegroundPointerOver
• TabViewItemHeaderPressedCloseButtonForeground
• TabViewItemHeaderPointerOverCloseButtonForeground
• TabViewItemHeaderSelectedCloseButtonForeground
• TabViewItemHeaderDisabledCloseButtonForeground
• TabViewItemHeaderBackgroundDisabled

Reason

Create individual theme resources for the TabViewItem's Close button.

You might notice there are no TabViewItemHeader**SelectedPointerOver**CloseButtonForeground and TabViewItemHeader**SelectedPressed**CloseButtonForeground theme resources for the TabViewItem's Close button listed here. The reason for this is that there are currently no separate TabViewItemHeaderBackgroundSelectedPointerOver and TabViewItemHeaderBackgroundSelectedPressed theme resources as can be seen here:

microsoft-ui-xaml/dev/TabView/TabView.xaml

Line 438 in 78fa7f6

<VisualState x:Name="PointerOverSelected">

And here:

microsoft-ui-xaml/dev/TabView/TabView.xaml

Line 451 in 78fa7f6

<VisualState x:Name="PressedSelected">

As such, developers currently cannot use lightweight styling to distinguish the TabViewItem's background while in selected state (the active tab) from its rest mode, its hover mode or its pressed mode. As such, there is also no need to create such theme resources for the Close button right now. If we decide to add the respective TabViewItemHeaderBackgroundSelectedPointerOver/Pressed theme resources, we should then also add the corresponding theme resources for the Close button.

Changed theme resources

The following theme resources had their values changed as part of this PR:

• TabViewButtonBackgroundPressed
• TabViewButtonBackgroundPointerOver
• TabViewButtonForegroundPressed
• TabViewButtonForegroundPointerOver

Reason

Prior to this PR, these theme resources were only used for the TabViewItem's Close button, as can be seen here:

microsoft-ui-xaml/dev/TabView/TabView.xaml

Line 260 in 78fa7f6

<VisualState x:Name="PointerOver">

And here:

microsoft-ui-xaml/dev/TabView/TabView.xaml

Line 271 in 78fa7f6

<VisualState x:Name="Pressed">

The TabView's Add button did not use these theme resources even though it is using the other TabViewButton* theme resources. Instead, it used the following theme resources for its Pressed/PointerOver visual states:

microsoft-ui-xaml/dev/TabView/TabView.xaml

Line 335 in 78fa7f6

<VisualState x:Name="PointerOver">

microsoft-ui-xaml/dev/TabView/TabView.xaml

Line 347 in 78fa7f6

<VisualState x:Name="Pressed">

As part of this PR, the TabView's Add button now uses these TabViewButtonBackgroundPressed/PointerOver and TabViewButtonForegroundPressed/PointerOver theme resources as well. Thus, to keep the TabView's Add button visual states unchanged in this PR, these four theme resources above have been modified to match the previous theme resources used by the Add button. This is fine because the TabViewItem's Close button which was previously using these resources exclusively now no longer consumes them (see "Added theme resources" section above).

Theme resources no longer in use

The following theme resources are no longer in use after this PR. To prevent a breaking change, for now these theme resources have been retained with a note detailing their status:

• TabViewButtonBackgroundActiveTab
• TabViewButtonForegroundActiveTab

Reason

The different TabViewButton* theme resources are now solely used for the TabView buttons located outside of the TabViewItems, such as the TabView's Add button. As such, there is no concept of an "active tab" for these buttons any longer, thus those theme resources are no longer needed.

Additional Info

Additional Info 1

If you checked issue #2655 which this PR is aiming to close, you will notice that I spoke of creating TabView Add button specific theme resources like TabViewAddButtonBackground there. However, as you can see, there is no mention of them in this PR. The reason is the following: With the TabView.TabStripHeader and TabView.TabStripFooter APIs, additional content can be added to the TabView strip as can be seen in this image:

Now, we can use these two APIs to also add additional buttons to the TabStrip. A great example of where that is done for a TabView is Edge UWP:

Notice how all the TabView buttons use the same style (if we ignore the slight difference in button widths in the two left-most buttons compared to the rest). This is something we could also provide to developers in a future PR. My thinking here is this: WinUI currently uses a private TabViewButtonStyle to set the design of the TabView's Add button:

microsoft-ui-xaml/dev/TabView/TabView.xaml

Line 300 in 78fa7f6

<Style x:Name="TabViewButtonStyle" TargetType="Button">

We can make that style "public" by moving it into the TabView_themeresources.xml files and let developers use it for their custom TabStrip buttons added in the TabStripHeader and the TabStripFooter. See, this is the current default design we get by adding buttons without a style:

<muxc:TabView x:Name="TabView" AddTabButtonClick="TabView_AddTabButtonClick">
    <muxc:TabView.TabItems>
        <muxc:TabViewItem Header="Tab 1">
            <muxc:TabViewItem.IconSource>
                <muxc:SymbolIconSource Symbol="Document" />
            </muxc:TabViewItem.IconSource>
        </muxc:TabViewItem>
    </muxc:TabView.TabItems>
    <muxc:TabView.TabStripHeader>
        <Button>
            <Button.Content>
                <SymbolIcon Symbol="Home" />
            </Button.Content>
        </Button>
    </muxc:TabView.TabStripHeader>
    <muxc:TabView.TabStripFooter>
        <Button>
            <Button.Content>
                <SymbolIcon Symbol="Setting" />
            </Button.Content>
        </Button>
    </muxc:TabView.TabStripFooter>
</muxc:TabView>

Result:

Wouldn't it be nice to expose the TabViewButtonStyle here so that developers could use it like this?

<muxc:TabView.TabStripFooter>
    <Button Style="{StaticResource TabViewButtonStyle}">
        <Button.Content>
            <SymbolIcon Symbol="Setting" />
        </Button.Content>
    </Button>
</muxc:TabView.TabStripFooter>

That way, we can easily enable developers to create a unified TabView button design, as seen in Edge UWP.

I think by now you are seeing why I did not want to create TabViewAddButton* specific theme resources to be used in a new TabViewAddButtonStyle. As I believe this is something worth exploring, I decided to let the TabView Add button keep using the existing resources, even though they are not specifically named after the Add button. As the TabViewItem's Close button still got its dedicated theme resources as part of this PR, this PR still fixes issue #2655 as intended.

Additional Info 2

With this PR the theme resources TabViewItemHeaderSelectedCloseButtonForeground and TabViewItemHeaderSelectedCloseButtonBackground where added. As their naming indicates, they control the foreground and background appearance of the TabViewItem's Close button when the tab is selected (the active tab). If we want to be really precise with our lightweight styling offering here, we should also add the corresponding

• TabViewItemHeaderSelectedCloseButtonForegroundPointerOver
• TabViewItemHeaderSelectedCloseButtonForegroundPressed
• TabViewItemHeaderSelectedCloseButtonBackgroundPointerOver
• TabViewItemHeaderSelectedCloseButtonBackgroundPressed

theme resources. These would control the appearance of the Close button in a selected tab when the Close button is in the hover/pressed state. However, adding these theme resources will require additional logic to be added to TabView.cpp so I did not yet touch these.

Conclusion

Let me know what you think here 😀

Motivation and Context

Closes #2655.

How Has This Been Tested?

Tested manually (see below)

Screenshots:

Take a look at the following XAML and resulting look:

<muxc:TabView>
    <muxc:TabView.Resources>
        <SolidColorBrush x:Key="TabViewItemHeaderCloseButtonForeground" Color="Red" />
        <SolidColorBrush x:Key="TabViewButtonForeground" Color="YellowGreen" />
    </muxc:TabView.Resources>
</muxc:TabView>

[marcelwgn]

I think it would be good to also add a sample in the testapp, there already is a TabView where a few resources are changed.

Another thing I noticed is that we added the resources for "TabViewItemHeader" + [Pressed,PointerOver,Selected] + "CloseButtonBackground". For completeness, I think we should also add a "TabViewItemHeaderDisabledCloseButtonBackground" resource.

Thank you for the detailed explanation of your changes!

[Felix-Dev]

@chingucoding

Another thing I noticed is that we added the resources for "TabViewItemHeader" + [Pressed,PointerOver,Selected] + "CloseButtonBackground". For completeness, I think we should also add a "TabViewItemHeaderDisabledCloseButtonBackground" resource.

Currently, when a TabViewItem is disabled, the background and foreground appearance of its Close button can be set via the following theme resources:

<muxc:TabView>
    <muxc:TabView.Resources>
        <SolidColorBrush x:Key="TabViewItemHeaderCloseButtonForegroundDisabled" Color="Yellow" />
        <SolidColorBrush x:Key="TabViewItemHeaderCloseButtonBackgroundDisabled" Color="Black" />
    </muxc:TabView.Resources>
</muxc:TabView>

Result:

We could decide to remove these theme resources and add your suggested theme resources

• TabViewItemHeaderDisabledCloseButtonBackground
• TabViewItemHeaderDisabledCloseButtonForeground

instead.

Since developers cannot directly set the Close button to disabled (TabViewItem.IsClosable=false causes the Close button to not be shown) but they can directly set the TabViewItem to disabled, I am currently in favor of your suggestion as that sounds more natural to me from a developer's perspective and how they would interact with the TabViewItem and its Close button (programmatically) here.

Speaking of these resources, another currently missing theme resource we should add as part of this PR is this one:

• TabViewItemHeaderBackgroundDisabled

Reason: We currently have the "TabViewItemHeaderBackground" + ["", "PointerOver", "Pressed", "Selected"] theme resources and the matching TabViewItemHeaderForeground* theme resources, as well as a TabViewItemHeaderForegroundDisabled theme resource.

It would also give developers the opportunity to highlight disabled tabs more easily in case additional visual separator from enabled tabs is need. See for example this customized TabView:

It appears to me a unique background for a disabled TabViewItem would come in handy here. Using lightweight styling, the same TabView could be designed like this then:

Looks already better to me.

I think it would be good to also add a sample in the testapp, there already is a TabView where a few resources are changed.

Perhaps a new TabView test page could be added dedicated entirely to visual customization of the TabView control (covering present and future theme resources, as well as possible future public component styles like TabViewButtonStyle, orTabViewScrollButtonStyle)? I have the feeling that adding multiple different styling customization tests to the existing TabView test page could easily add up to an overwhelming (visual) complexity then.

[marcelwgn]

[...] We could decide to remove these theme resources and add your suggested theme resources

TabViewItemHeaderDisabledCloseButtonBackground
TabViewItemHeaderDisabledCloseButtonForeground
instead.

Since developers cannot directly set the Close button to disabled (TabViewItem.IsClosable=false causes the Close button to not be shown) but they can directly set the TabViewItem to disabled, I am currently in favor of your suggestion as that sounds more natural to me from a developer's perspective and how they would interact with the TabViewItem and its Close button (programmatically) here.

Yes, it's a bit confusing that essentially TabViewItemHeaderCloseButtonForegroundDisabled is what TabViewItemHeaderDisabledCloseButtonForeground would be. I think we should be consistent here and use TabViewItemHeaderDisabledCloseButtonForeground and TabViewItemHeaderDisabledCloseButtonBackground.

Speaking of these resources, another currently missing theme resource we should add as part of this PR is this one:

TabViewItemHeaderBackgroundDisabled
Reason: We currently have the "TabViewItemHeaderBackground" + ["", "PointerOver", "Pressed", "Selected"] theme resources and the matching TabViewItemHeaderForeground* theme resources, as well as a TabViewItemHeaderForegroundDisabled theme resource.

Agree with you, since we have the other resources it feels a bit strange to not have that resource too. Good idea!

Perhaps a new TabView test page could be added dedicated entirely to visual customization of the TabView control (covering present and future theme resources, as well as possible future public component styles like TabViewButtonStyle, orTabViewScrollButtonStyle)? I have the feeling that adding multiple different styling customization tests to the existing TabView test page could easily add up to an overwhelming (visual) complexity then.

I am not much of a fan in introducing another test page as we need a way to navigate to that and the test UIs are already hard to use as they are (yes I know I also created a new test page for a TabView PR). However having a complete page dedicated to that seems the best way forward here.

[Felix-Dev]

Adding the theme resource TabViewItemHeaderBackgroundDisabled will potentially "break" existing TabView designs (that is, the look of disabled TabViewItems in a TabView with a customized TabViewItem background would change when developers will upgrade to the WinUI version containing this new theme resource). It will not cause a breaking change where the app can no longer be compiled/run successfully.

Reason: Currently, as can be seen above, disabled TabViewItems use the background specified by the TabViewItemHeaderBackground theme resource. In order to not "break" the UI of existing disabled TabViewItems, the new theme resource would essentially have to reference this theme resource by default, like so:

<StaticResource x:Key="TabViewItemHeaderBackgroundDisabled"   ResourceKey="TabViewItemHeaderBackground" />

The idea here is that to keep the existing styling behavior intact, overriding the TabViewItemHeaderBackground theme resource will also propagate to the TabViewItemHeaderBackgroundDisabled theme resource, thus a new theme resource can be introduced without "breaking" existing ligweight-styled UI. (Of course, the new theme resource can still be modified directly as usual to assign it a unique color independently of the TabViewItemHeaderBackground theme resource.)

However, currently, the XAML resource system does not support this level of indirection. Overriding the referenced theme resource (TabViewItemHeaderBackground in this case) on either the app level or page level (or even more fine grained) does not update the referencing theme resource (TabViewItemHeaderBackgroundDisabled here). The referencing theme resource will still use the original value it was set to in the resource dictionary it was introduced in (like generic.xaml or WinUI's multiple [ControlName]_themeresources.xaml files). In our example above, this means the TabViewItemHeaderBackgroundDisabled theme resource points to the original value of the TabViewItemHeaderBackground theme resource which is SystemControlTransparentBrush. Here is the matching screenshot:

(In the screen shot above, the TabViewItemHeaderBackground theme resource was changed to a SolidColorBrush with color #FFC71F45. It doesn't matter if it is overriden at the app level or page level.)

The missing support in the XAML resource system for this level of indirection has come up a couple of times over the past days (here and here, for example) and should be tracked in its own issue at this point.

Anyway, to come back to this PR: I still strongly favor introducing a new TabViewItemHeaderBackgroundDisabled theme resource as in the long run it makes sure we will provide developers with a powerful lightweight-styling experience they are coming to expect from WinUI and which plays an important part in customizing the default look of Controls without requiring a re-template. Even if in the short-term it might require developers to include the new TabViewItemHeaderBackgroundDisabled theme resource upon a WinUI upgrade in case they restyled the TabView control and want to restore their current look for disabled TabViewItems.

[marcelwgn]

Having a breaking change is definitely a problem here. However having inconsistent resource names is also not ideal either.
I would say that we introducing that resource is better in the long run, despite introducing a breaking change here.

Regarding the XAML resource indirection: It's quite unfortunate that that doesn't work. It would allow to introduce a new resource without breaking existing apps. Is there already a proposal/issue for that? If not, would you like to create one?

[Felix-Dev]

@chingucoding

Having a breaking change is definitely a problem here

Seems like we have to be a bit more precise here as I apparently managed to confuse @StephenLPeters here before when talking about these "UI breaking" changes as "breaking changes". Adding the TabViewItemHeaderBackgroundDisabled theme resource will not cause apps to crash. "Merely" the UI of a TabView with a customized TabViewItem background and using disabled tabs would end up looking different due to this change and easily correctable by developers. I thus am currently terming these kind of changes as "UI breaking" changes in the hope that this expresses these are less severe than the changes which would actually cause apps to crash after upgrading.

That said, I would prefer updating WinUI's lightweight styling experiences in a way without causing these kinds of possible design interruptions but they seem to be unavoidable as of now. And speaking of the XAML resource indirection: I am not aware of any open issue about it here in this repo and I plan to to create a tracking issue for it in the next 1-2 days.

I would say that we introducing that resource is better in the long run, despite introducing a breaking change here.

I fully agree. At worst, developers simply have to use the newly introduced theme resource here to restore their customized TabViewItem design and in return they will gain greater control over how to design their TabView in all the possible visual state configurations.

[StephenLPeters]

@Felix-Dev Thank you for such a thorough write up and @chingucoding for the indepth discussion. A few things:

Removed theme resources
The following theme resources were deleted as part of this PR. Note: This is a breaking change!

TabViewButtonBackgroundActiveTab
TabViewButtonForegroundActiveTab

Despite the TabView template no longer referencing these resources it is still a breaking change to remove them. For instance if someone retemplated the TabView control their copy of the template would still reference this resouces and if they updated the WinUI version their app would not longer compile if we deleted these. This will not meet the breaking change bar for our WinUI3 release so we will ask that we not make this change in WinUI 2 at this time. Instead, we should leave the resouces in place despite them not being used... :/

The TabViewItemHeaderBackgroundDisabled resource makes sense to me, my vote would be add it. @chigy FYI.

We could decide to remove these theme resources and add your suggested theme resources

TabViewItemHeaderDisabledCloseButtonBackground
TabViewItemHeaderDisabledCloseButtonForeground
instead.

Is the motivation for these just that the current resource name doesn't match the name of the other resources? If so, I think this change would also be a breaking change and would not meet the Winui 3 bar.

[StephenLPeters]

I think it would be good to also add a sample in the testapp, there already is a TabView where a few resources are changed.

Perhaps a new TabView test page could be added dedicated entirely to visual customization of the TabView control (covering present and future theme resources, as well as possible future public component styles like TabViewButtonStyle, orTabViewScrollButtonStyle)? I have the feeling that adding multiple different styling customization tests to the existing TabView test page could easily add up to an overwhelming (visual) complexity then.

I love this idea, a page that made it easy to change all of the light weight style values of a control and in real time see the effect of that change seems super valuable, not just for Tabview, but probably all of our controls. I wonder if this sort of test page could be generated as well? It seems possible, having a way to change the page to high contrast mode would be cool too. I think we have a habit of not treating the light weight styles as APIs even though that's what they are.

[ranjeshj]

@Felix-Dev Thank you for such a thorough write up and @chingucoding for the indepth discussion. A few things:

Removed theme resources
The following theme resources were deleted as part of this PR. Note: This is a breaking change!

TabViewButtonBackgroundActiveTab
TabViewButtonForegroundActiveTab

Despite the TabView template no longer referencing these resources it is still a breaking change to remove them. For instance if someone retemplated the TabView control their copy of the template would still reference this resouces and if they updated the WinUI version their app would not longer compile if we deleted these. This will not meet the breaking change bar for our WinUI3 release so we will ask that we not make this change in WinUI 2 at this time. Instead, we should leave the resouces in place despite them not being used... :/

The TabViewItemHeaderBackgroundDisabled resource makes sense to me, my vote would be add it. @chigy FYI.

We could decide to remove these theme resources and add your suggested theme resources

TabViewItemHeaderDisabledCloseButtonBackground
TabViewItemHeaderDisabledCloseButtonForeground
instead.

Is the motivation for these just that the current resource name doesn't match the name of the other resources? If so, I think this change would also be a breaking change and would not meet the Winui 3 bar.

Agree with Stephen here. We can add new resources, but should avoid changing or removing existing ones which could break the re-templating scenario in WinUI3. We are trying to make the upgrade to WinUI3 as painless as possible.

[Felix-Dev]

@StephenLPeters @ranjeshj

Despite the TabView template no longer referencing these resources it is still a breaking change to remove them. For instance if someone retemplated the TabView control their copy of the template would still reference this resouces and if they updated the WinUI version their app would not longer compile if we deleted these. This will not meet the breaking change bar for our WinUI3 release so we will ask that we not make this change in WinUI 2 at this time. Instead, we should leave the resouces in place despite them not being used... :/

I'm fine with leaving them in place for now. Though I would like to add a comment above them in the resource dictionary then, indicating that they are no longer used. Perhaps something like "Note: These theme resources are deprecated and could be removed in a future WinUI update."?

Is the motivation for these just that the current resource name doesn't match the name of the other resources? If so, I think this change would also be a breaking change and would not meet the Winui 3 bar.

To be clear here: The theme resources

• TabViewItemHeaderCloseButtonForegroundDisabled
• TabViewItemHeaderCloseButtonBackgroundDisabled

currently do not exist outside of this PR! Removing them would not not be a breaking change then. The story behind these two theme resources is the following: Prior to this PR, the appearance of the TabViewItem's Close button when the TabViewItem is disabled is controlled by the following two theme resources:

• TabViewButtonBackgroundDisabled
• TabViewButtonForegroundDisabled

In addition, the TabView's Add button also consumes these two theme resources for its disabled appearance. Since this PR is about creating dedicated theme resources for the TabViewItem's Close button and TabView buttons like the Add button, the Close button has to use a new set of resources here.

Initially, I just replaced these two resources above with the new

• TabViewItemHeaderCloseButtonForegroundDisabled
• TabViewItemHeaderCloseButtonBackroundDisabled

theme resources, while keeping the two theme resources

• TabViewButtonBackgroundDisabled
• TabViewButtonForegroundDisabled

to be used by the TabView's Add button. Hence achieving the purpose of this PR of having dedicated theme resources (in this case for the disabled states of the Close button and the Add button).

However, as @chingucoding and I have discussed above, introducing these two theme resources don't precisely reflect the way the developer can interact with the control. See, these two theme resources are applied when the TabViewItem is disabled. The developer cannot directly set the Close button to disabled. However, the developer can set the TabViewItem to disabled!
As such, I believe a developer will approach the available theme resources like this:
"Hmm, I can set a TabViewItem to disabled and I want to customize the appearance of my TabViewItem in this case...aha, I can see resources like TabViewItemHeaderForegroundDisabled and TabViewItemHeaderDisabledCloseButtonForeground...perfect!"
I don't think it is intuitive that a developer would start looking at a TabViewItemHeaderCloseButtonForegroundDisabled named theme resource immediately when they first and foremost are looking for theme resources to customize the appearance of the TabViewItem and its content when it is disabled.

Theme resources named like TabViewItemHeaderDisabledCloseButtonForeground would also match a theme resource like TabViewItemHeaderSelectedCloseButtonForeground (introduced in this PR) which is used to customize the foreground of the Close button when its owning TabViewItem is selected. Here again, the same perspective applies. A developer can set a TabViewItem to selected, and not the Close button, so the theme resource is named as such to indicate this behavior. Following this logic for the disabled Close button's appearance as well creates consistency in our "logical" naming approach here.

Either way, since all the theme resources in question here do not exist prior to this PR, no breaking change would be caused here one way or the other. The deciding point here is just which theme resource names sounds more "logical" from a developer's stand point. So, to recap, we can decide between either

• TabViewItemHeaderCloseButtonBackgroundDisabled
• TabViewItemHeaderCloseButtonForegroundDisabled

or

• TabViewItemHeaderDisabledCloseButtonBackground
• TabViewItemHeaderDisabledCloseButtonForeground

Both @chingucoding and me prefer the latter set right now.

(That reply went a bit longer than I would have hoped for (and you perhaps expected 😁) but I think this should clearly explain where we are are standing here.)

[StephenLPeters]

Okay, Awesome, I agree with you two, your purposed name seems better. Thank you for the clear explination. @MikeHillberg as the resident naming guru :)

[StephenLPeters]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[Felix-Dev]

Since a few days have passed and no other comments arrived, I guess I can go ahead with adding the

• TabViewItemHeaderBackgroundDisabled
• TabViewItemHeaderDisabledCloseButtonBackground
• TabViewItemHeaderDisabledCloseButtonForeground

theme resources now.

[MikeHillberg]

Sorry for not replying, and thanks for the recap. So TabViewItemHeaderDisabledCloseButtonBackground means the background of the Close button in the TabViewItem's header, when the button is disabled?

[Felix-Dev]

@MikeHillberg
TabViewItemHeaderDisabledCloseButtonBackground will be applied to the background of the Close button in the TabViewItem's header when the TabViewItem is set to disabled (TabViewItem.IsEnabled=false;). This is similar to the TabViewItemHeaderSelectedCloseButtonBackground theme resource which sets the background of the Close button when the TabViewItem is selected.

In case the developer grabs the TabViewItem's Close button by walking the visual tree (there is no API exposed on the TabViewItem to directly set the Close button to disabled) and then sets the button to disabled, the TabViewItemHeaderDisabledCloseButtonBackground won't be applied here (as it's not set set in the Button's visual states but in the TabViewItem's visual states). However, normally, a developer would only set the TabViewItem to disabled and not just set the Close button to disabled by walking the visual tree while keeping the rest of tab enabled. (If they don't want a tab to be closable, they can use the TabViewItem.IsClosable API which will hide the Close button then.)

That said...the implementation of the TabViewItem.IsClosable API could be changed in the future though (at least in theory) to show a disabled Close button instead of hiding it as done currently. In that case, a theme resource named like TabViewItemHeaderCloseButtonBackgroundDisabled would make more sense.

[MikeHillberg]

@Felix-Dev - You said that TabViewItemHeaderDisabledCloseButtonBackground applies when the TabViewItem is disabled, but in the template I see it in the visual state for the Button (but currently in the PR it's named TabViewItemHeaderCloseButtonBackgroundDisabled). Am I reading it right?

[Felix-Dev]

@MikeHillberg Yes, you are reading it correctly. Currently, the PR is using the TabViewItemHeaderCloseButtonBackgroundDisabled theme resource to customize the background color of the TabViewItem's Close button when the button is disabled (the button will also be disabled when the owning TabViewItem is disabled).

In this PR, it was suggested to swap out the proposed TabViewItemHeaderCloseButtonBackgroundDisabled theme resource with a theme resource named TabViewItemHeaderDisabledCloseButtonBackground to provide developers with theme resources consistent with how they interact with the TabViewItem and its components. This TabViewItemHeaderDisabledCloseButtonBackground theme resource would be placed inside the TabViewItem's visual state here

microsoft-ui-xaml/dev/TabView/TabView.xaml

Line 468 in 1bd3129

<VisualState x:Name="Disabled">

with the following code:

<Setter Target="CloseButton.Background" Value="{ThemeResource TabViewItemHeaderDisabledCloseButtonBackground}" />

While this would provide a more consistent set of theme resources for the TabViewItem with regards to the current TabView implementation, moving the CloseButtonBackgroundDisabled theme resource out of the visual states of the button into the visual states of the TabViewItem would turn into an issue if at some point in the future the TabViewItem.IsClosable API would not hide the Close button, but be changed to show a disabled Close button inseatd. In other words, developers could then independently set the Close button to disabled while keeping the owning TabViewItem enabled, which would require the existence of CloseButtonBackgroundDisabled in the visual states of the Close button. With the current TabViewItem API, developers cannot independently set the Close button's enabled state from the owning TabViewItem's enabled state. They can only disable a Close button by disabling the owning TabViewItem. (I am excluding the possibility of developers walking the visual tree of a TabViewItem to obtain an instance of the Close button.)

Hopefully this gave you a good understanding about where we stand right now in this PR with regards to the proposed TabViewItemHeaderDisabledCloseButtonBackground vs TabViewItemHeaderCloseButtonBackgroundDisabled theme resources.

[MikeHillberg]

@Felix-Dev Thanks for the (re-)explanation. Sorry, I got lost in the long thread.

In that future feature of a TabView.IsClosable state that disables rather than hides the Close button, could the TabViewItem's existing CloseButtonCollapsed visual state be updated to set CloseButton.Background to TabViewItemHeaderDisabledCloseButtonBackground? (And then not make any visual state updates inside the CloseButton.)

[Felix-Dev]

@MikeHillberg Yes, we could do that.

In the case where the TabViewItem.IsClosable API behavior is changed to show a disabled Close button, we should keep in mind though that the background of an enabled TabViewItem with IsClosable = false might be different than the background of a disabled TabViewItem (which would also show a disabled Close button). With the backgrounds potentially differing here, we should probably offer developers with two theme resources here to set the background of the Close button when it's in disabled state: One to cover the case of an enabled TabViewItem which cannot be closed, and the other resource for the case where the entire TabViewItem is disabled.

That said, I think we could for now settle on the proposed TabViewItemHeaderDisabledCloseButtonBackground theme resource and if at some point TabViewItem.IsClosable's behavior is changed we could always introduce another theme resource for this new specific case (TabViewItem enabled with Close button disabled).

[MikeHillberg]

@Felix-Dev That makes sense. Thanks again for explaining.

[StephenLPeters]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[StephenLPeters]

@teaP would you like to take a look?

[teaP]

I apologize if I'm out of the loop, are these brushes different?