-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Fixed ComboBox style and template #10818
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 1 out of 8 changed files in this pull request and generated no comments.
Files not reviewed (7)
- src/Microsoft.DotNet.Wpf/src/Themes/PresentationFramework.Fluent/Resources/Theme/Dark.xaml: Language not supported
- src/Microsoft.DotNet.Wpf/src/Themes/PresentationFramework.Fluent/Resources/Theme/HC.xaml: Language not supported
- src/Microsoft.DotNet.Wpf/src/Themes/PresentationFramework.Fluent/Resources/Theme/Light.xaml: Language not supported
- src/Microsoft.DotNet.Wpf/src/Themes/PresentationFramework.Fluent/Resources/Variables.xaml: Language not supported
- src/Microsoft.DotNet.Wpf/src/Themes/PresentationFramework.Fluent/Themes/Fluent.Dark.xaml: Language not supported
- src/Microsoft.DotNet.Wpf/src/Themes/PresentationFramework.Fluent/Themes/Fluent.HC.xaml: Language not supported
- src/Microsoft.DotNet.Wpf/src/Themes/PresentationFramework.Fluent/Themes/Fluent.Light.xaml: Language not supported
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #10818 +/- ##
===================================================
- Coverage 13.72443% 13.59071% -0.13373%
===================================================
Files 3316 3316
Lines 664822 664822
Branches 74651 74651
===================================================
- Hits 91243 90354 -889
- Misses 570879 571870 +991
+ Partials 2700 2598 -102
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
src/Microsoft.DotNet.Wpf/src/Themes/PresentationFramework.Fluent/Resources/Theme/Dark.xaml
Show resolved
Hide resolved
src/Microsoft.DotNet.Wpf/src/Themes/PresentationFramework.Fluent/Styles/ComboBox.xaml
Show resolved
Hide resolved
@@ -43,7 +43,7 @@ | |||
<Thickness x:Key="TimePickerHostPadding">0,1,0,2</Thickness> | |||
<Thickness x:Key="DatePickerHostPadding">0,1,0,2</Thickness> | |||
<Thickness x:Key="DatePickerHostMonthPadding">9,0,0,1</Thickness> | |||
<Thickness x:Key="ComboBoxEditableTextPadding">10,0,30,0</Thickness> | |||
<!-- <Thickness x:Key="ComboBoxEditableTextPadding">10,0,30,0</Thickness> --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why has it been commented ? Why not remove this ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No reason at all. Will remove this.
</Grid> | ||
<ControlTemplate.Triggers> | ||
<Trigger Property="IsMouseOver" Value="True"> | ||
<!-- Disabling this trigger as this will be handled by the ComboBox style. --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How is this trigger disabled ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will update that, I meant to write, "disabling this setter"
src/Microsoft.DotNet.Wpf/src/Themes/PresentationFramework.Fluent/Styles/ComboBox.xaml
Show resolved
Hide resolved
src/Microsoft.DotNet.Wpf/src/Themes/PresentationFramework.Fluent/Styles/ComboBox.xaml
Show resolved
Hide resolved
src/Microsoft.DotNet.Wpf/src/Themes/PresentationFramework.Fluent/Styles/ComboBox.xaml
Show resolved
Hide resolved
src/Microsoft.DotNet.Wpf/src/Themes/PresentationFramework.Fluent/Styles/ComboBox.xaml
Show resolved
Hide resolved
src/Microsoft.DotNet.Wpf/src/Themes/PresentationFramework.Fluent/Styles/ComboBox.xaml
Show resolved
Hide resolved
src/Microsoft.DotNet.Wpf/src/Themes/PresentationFramework.Fluent/Styles/ComboBox.xaml
Show resolved
Hide resolved
<DoubleAnimation | ||
Storyboard.TargetName="DropDownBorder" | ||
Storyboard.TargetProperty="(Border.RenderTransform).(TranslateTransform.Y)" | ||
From="-90" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May be good to have figma links / recordings of the animations to better understand this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have any such source. Moreover, the Fluent 2 Design Figma style that is available publicly only mentions the category of animations based on type and timing but they don't mention which type of animation is to be used where.
@godlytalias, if you can point me to any such links it would be a great help.
Fixes #10470 , #10656
Description
In this PR I have restyled the ComboBox style and template to make it more similar to WinUI's style, however the styles are slightly different to make it easier in WPF. Here are the list of few major changes in the PR :
Bug Fixes covered in this PR :
This is not an exhaustive list.
Customer Impact
Correct style for developers.
Regression
No.
Testing
Local app testing.
Risk
Minimal.
Microsoft Reviewers: Open in CodeFlow