-
Notifications
You must be signed in to change notification settings - Fork 706
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Proposal: Foreground for ComboBox.PlaceholderText should be same as TextBox.PlaceholderText #2774
Comments
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 |
I will create a PR tomorrow for the placeholder foreground. |
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:
What we are saying here and in the other visual states of Focused and FocusedPressed here and here is that if the 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):
These lightweight styling issues above should presumably be tracked in a separate issue about the XAML resource lookup algorithm. Your thoughts, @StephenLPeters? |
@kikisaints FYI |
@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? |
@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 <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 |
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
Scope
Important Notes
Open Questions
The text was updated successfully, but these errors were encountered: