Microsoft-ui-xaml: Proposal: Foreground for ComboBox.PlaceholderText should be same as TextBox.PlaceholderText

[harvinders]

Proposal: Foreground for ComboBox.PlaceholderText should be same as TextBox.PlaceholderText

Summary

The foreground colour for ComboBox's PlaceholderText should be same as TextBox's PlaceholderText, as it would allow it to be differentiated from the selected value in the combobox.

Rationale

• Help align PlaceholderText foreground between controls
• Help differentiate the PlaceholderText from the selected value

Scope

| Capability | Priority |
| :---------- | :------- |
| Match the foreground colour to TextBox's PlaceholderText foreground | Must |
| Allow lightweight style | Should |

Important Notes

Open Questions

[Felix-Dev]

Makes sense to me. Below you will find a table showcasing the current placeholder foreground color of a ComboBox in uneditable mode while in rest state, focused state and focused pressed state (there is no separate FocusedPointerOver visual state). The proposed solution updates the placeholder foreground color to match that of the TextBox and keep the placeholder foreground consistent between the visual states outlined above to separate it from non-placeholder text.

| Theme | Current | Proposed |
| -------|---------| -----------|
| Light | ||
| Dark | | |

The above proposed solution would add a new ComboBoxPlaceHolderForegroundFocused theme resource alongside the already existing ComboBoxPlaceHolderForeground and ComboBoxPlaceHolderForegroundFocusedPressed theme resources.

When the TextBox is in editable mode, only the placeholder foreground color needs to be updated as above. In the other Focused* visual states it already matches that of the TextBox's placeholder foreground:

[StephenLPeters]

Should be pretty simple given that we have lifted the combo box template to WinUI 2 already. I agree the placeholder text foreground color should match textbox's. FYI @chigy

[Felix-Dev]

I will create a PR tomorrow for the placeholder foreground.

[Felix-Dev]

About lightweight styling the placeholder foreground: This doesn't appear to work as intended and expected for now. Observe for example the following XAML:

<ComboBox PlaceholderText="Some placeholder">
    <ComboBox.Resources>
        <SolidColorBrush x:Key="ComboBoxPlaceHolderForeground" Color="Green" />
        <SolidColorBrush x:Key="ComboBoxPlaceHolderForegroundFocused" Color="Orange" />
        <SolidColorBrush x:Key="ComboBoxPlaceHolderForegroundFocusedPressed" Color="Red" />
    </ComboBox.Resources>
</ComboBox>

The expected result would be this:

but the actual result is this:

These issues are located here, for example: https://github.com/microsoft/microsoft-ui-xaml/blob/6994eb90c8c7656f694d01253cfa643f29a0df9f/dev/ComboBox/ComboBox_themeresources.xaml#L620

What we are saying here and in the other visual states of Focused and FocusedPressed here and here is that if the ComboBox.PlaceholderForeground API is set, it will take precendence over the specified theme resources. However, if the PlaceholderForeground API is not set, as in my example XAML above, these theme resources should be used. And in fact they are (TargetNullValue works correctly). Unfortunately for us though, the theme resources values applied here are the values defined in the ComboBox_themeresources.xml resource dictionary! The overriden resource values in my example XAML above are being ignored (probably not seen by the theme resource lookup algorithm in use here). @StephenLPeters I'm not sure how related this XAML resource lookup issue is with the issue we were seeing in https://github.com/microsoft/microsoft-ui-xaml/pull/2674#issuecomment-646236865 but I'm still pinging you here as a FYI at least.

This issue also effects the TextBox and its placeholder theme resources like TextControlPlaceholderForeground and TextControlPlaceholderForegroundFocused and as such is also an issue for the ComboBox when it is in an editable state (ComboBox.IsEditable = true): https://github.com/microsoft/microsoft-ui-xaml/blob/6994eb90c8c7656f694d01253cfa643f29a0df9f/dev/CommonStyles/TextBox_themeresources.xaml#L231

These lightweight styling issues above should presumably be tracked in a separate issue about the XAML resource lookup algorithm. Your thoughts, @StephenLPeters?

[chigy]

@kikisaints FYI

[StephenLPeters]

@Felix-Dev making sure I understand, this issue is still present after your PR correct? If you put the theme resource in a parent resource dictionary does it work? what about the app.xaml resources? I wonder if the lookup initiated by the binding functions differently than a normal resource lookup, similar to how resource lookups from code behind don't match xaml resource lookups..

@MikeHillberg or @jevansaks maybe you already know about any intricacies to lookups from binding?

[Felix-Dev]

@StephenLPeters Yes, the issue would still be present even with my PR merged. In other words, using lightweight styling to set the Placeholder foreground for the TextBox/ComboBox/... would still not work (see my example XAML markup in the post above).

It also does not matter if I override these resources on the control level, the page level or the app level. Same effect - my overriden placeholder foreground theme resources will be ignored.

And yes, "the lookup initiated by the binding functions" seems like a good candidate for being the culprit here. Observe that this only happens for the Placeholder which is the only one using Binding in ComboBox visual states. If I change the following XAML code in the visual state from

<ObjectAnimationUsingKeyFrames Storyboard.TargetName="PlaceholderTextBlock" Storyboard.TargetProperty="Foreground">
    <DiscreteObjectKeyFrame KeyTime="0" Value="{Binding PlaceholderForeground, RelativeSource={RelativeSource TemplatedParent}, TargetNullValue={ThemeResource ComboBoxPlaceHolderForegroundFocused}}" />
</ObjectAnimationUsingKeyFrames>

to:

<ObjectAnimationUsingKeyFrames Storyboard.TargetName="PlaceholderTextBlock" Storyboard.TargetProperty="Foreground">
    <DiscreteObjectKeyFrame KeyTime="0" Value="{ThemeResource ComboBoxPlaceHolderForegroundFocused}" />
</ObjectAnimationUsingKeyFrames>

my theme resource overrides for ComboBoxPlaceHolderForegroundFocused will be applied just as expected.