Skip to content
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

ScrollViewer Improvement #1672

Merged
merged 9 commits into from Mar 18, 2020
Merged

ScrollViewer Improvement #1672

merged 9 commits into from Mar 18, 2020

Conversation

@AmRo045
Copy link
Contributor

AmRo045 commented Feb 26, 2020

IsAutoHideEnabled and CornerRectangleVisibility properties:
Preview

ShowSeparators property:
ShowSeparators

… ScrollViewerAssist
@Keboo Keboo added the enhancement label Feb 29, 2020
<Trigger Property="wpf:ScrollViewerAssist.CornerRectangleVisibility" Value="Collapsed">
<Setter TargetName="PART_Corner" Property="Visibility" Value="Collapsed"/>
</Trigger>

<Trigger Property="wpf:ScrollViewerAssist.CornerRectangleVisibility" Value="Hidden">
<Setter TargetName="PART_Corner" Property="Visibility" Value="Hidden"/>
</Trigger>
Comment on lines 58 to 64

This comment has been minimized.

Copy link
@Keboo

Keboo Mar 2, 2020

Member

Could these be moved into a TemplateBinding on the PART_Corner?

This comment has been minimized.

Copy link
@AmRo045

AmRo045 Mar 2, 2020

Author Contributor

Yes it's better option for setting CornerRectangleVisibility.

<ScrollBar
x:Name="PART_VerticalScrollBar"
AutomationProperties.AutomationId="VerticalScrollBar"
Cursor="Arrow"

This comment has been minimized.

Copy link
@Keboo

Keboo Mar 2, 2020

Member

Is it necessary to set the Cursor on the Scroll bar?

This comment has been minimized.

Copy link
@AmRo045

AmRo045 Mar 2, 2020

Author Contributor

I think we don't need set Cursor for scrollbar. So i'll remove this property.

<Condition Property="wpf:ScrollViewerAssist.IsAutoHideEnabled" Value="True" />
<Condition Property="IsMouseOver" Value="True" />
Comment on lines 68 to 69

This comment has been minimized.

Copy link
@Keboo

Keboo Mar 2, 2020

Member

I am thinking we should probably add one more condition and separate out the PART_VerticalScrollBar and PART_HorizontalScrollBar into their own MultiTriggers.

<Condition Property="ComputedVerticalScrollBarVisibility" Value="Visible" />

That should eliminate the need for the Triggers below that are checking for Disabled and Hidden. It also allows for individually controlling which scrollbars get hidden/shown.

This comment has been minimized.

Copy link
@AmRo045

AmRo045 Mar 2, 2020

Author Contributor

Thank you for the recommendations

@Keboo

This comment has been minimized.

Copy link
Member

Keboo commented Mar 17, 2020

@AmRo045 I made one change to move some of the behavior out of the attached property and into triggers on the template. I believe this is a little cleaner since it wont leave event handlers around that might need to get cleaned up. Thoughts?

@AmRo045

This comment has been minimized.

Copy link
Contributor Author

AmRo045 commented Mar 17, 2020

@Keboo That's true. Your changes make the code more clean and understandable. Thank you

@AmRo045 AmRo045 requested a review from Keboo Mar 17, 2020
@Keboo
Keboo approved these changes Mar 18, 2020
@Keboo Keboo added this to the 3.1.0 milestone Mar 18, 2020
@Keboo Keboo merged commit d8e8676 into MaterialDesignInXAML:master Mar 18, 2020
1 check passed
1 check passed
MaterialDesignInXAML.MaterialDesignInXamlToolkit Build #20200318.1 succeeded
Details
@AmRo045 AmRo045 deleted the AmRo045:scrollviewer branch Mar 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.