-
-
Notifications
You must be signed in to change notification settings - Fork 876
Fix SplitButton various issues #1498
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
@@ -72,22 +72,22 @@ | |||
<ControlTemplate TargetType="{x:Type controls:SplitButton}"> | |||
<Border | |||
x:Name="ContentBorder" | |||
Grid.Row="0" |
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.
This isn't needed, there's no Grid, so I removed it as well.
@@ -115,10 +115,13 @@ | |||
Grid.Column="1" | |||
VerticalAlignment="Center" | |||
Content="{TemplateBinding Content}" | |||
ContentTemplate="{TemplateBinding ContentTemplate}" |
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 also added this so ContentTemplate can be used.
@@ -42,7 +42,6 @@ | |||
<ControlTemplate TargetType="{x:Type ToggleButton}"> | |||
<Border | |||
x:Name="ContentBorder" | |||
Margin="0,-1,-1,-1" |
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.
This caused the toggle button to be off-center, there's no need for this, so just remove it.
@@ -134,7 +136,7 @@ | |||
Focusable="False" | |||
Foreground="{TemplateBinding Foreground}" | |||
IsChecked="{TemplateBinding IsDropDownOpen}" | |||
Style="{StaticResource DefaultSplitButtonToggleButtonStyle}"> | |||
Style="{DynamicResource DefaultSplitButtonToggleButtonStyle}"> |
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.
So we can override the style of the toggle button without having to redeclare the entire template.
Update SplitButton.xaml
private void AttachToggleButtonClick() | ||
{ | ||
if (SplitButtonToggleButton != null) | ||
{ | ||
SplitButtonToggleButton.Click -= OnSplitButtonToggleButtonOnClick; |
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.
This had to be changed to PreviewMouseLeftButtonUp for two reasons.
- If the Click event is used, there's hardly any visible Pressed state as it instantly presses.
- The SplitButton's context menu would immediately open on mouse down, this isn't the same in WinUI, where a mouse up is required.
@@ -85,29 +94,12 @@ public SplitButton() | |||
}; | |||
} | |||
|
|||
protected virtual void AttachTemplateResources() |
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.
This is unused, so remove.
Pull request type
Please check the type of change your PR introduces:
What is the current behavior?
Currently, a SplitButton has a mouseover/pressed color for the entire element. This isn't what WinUI does, it only changes this for the part hovered over (i.e., content and the toggle button).
New:


(only toggle has mouseover brush).
(only content has mouseover brush).
Old:

(entire button has mouseover brush).