-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Add HamburgerVisibility property #1020
Conversation
@@ -54,6 +54,7 @@ | |||
DisplayMode="@[DisplayMode:Enum:SplitViewDisplayMode.CompactInline]" | |||
CompactPaneLength="@[CompactPaneLength:Slider:48:10-80]" | |||
HamburgerHeight="@[HamburgerHeight:Slider:48:10-80]" | |||
HamburgerVisibility="@[HamburgerVisibility:Enum:Visibility.Visible]" |
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 would call it HamburgerButtonVisibility to avoid misunderstandings
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.
@deltakosh That was my first idea as well. But then I saw that all other properties of the hamburger button only have the "Hamburger" prefix (HamburgerHeight, HamburgerWidth, HamburgerMargin, they all affect the button and not the icon itself). So for having naming consistency, I chose to use "HamburgerVisibility". But I agree that HamburgerButtonVisibility is the better name. Shall I change it?
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.
You are completely right. We can keep like it then :)
Added the HamburgerVisibility property as discussed in #1019.