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

Suggestion - HamburgerMenu Scrolling #954

Closed
amkuchta opened this Issue Feb 16, 2017 · 15 comments

Comments

Projects
None yet
4 participants
@amkuchta
Contributor

amkuchta commented Feb 16, 2017

Issue

If the window is sized to be smaller than the height of the HamburgerMenu, OptionsListView overlaps ButtonsListView. Additionally, the Scrollbar is only visible when the HamburgerMenu is expanded.

Suggestion

Ideally, the PaneRoot would not be allowed to have a height smaller than is required to show all of the items in the pane. My suggestion is to use a DockPanel wrapped in a ScrollViewer to host the ListBox controls and to move the ScrollViewer scrollbar to the left side to have it constantly visible, when needed. An example implementation is shown below:

<ScrollViewer VerticalAlignment="Stretch" FlowDirection="RightToLeft">
  <DockPanel VerticalAlignment="Stretch" FlowDirection="LeftToRight">
    <ListBox x:Name="ButtonsListView" DockPanel.Dock="Top" VerticalAlignment="Top">
      <ListBoxItem ... />
      <ListBoxItem ... />
      <ListBoxItem  ... />
    </ListBox >
    <ListBox x:Name="OptionsListView" DockPanel.Dock="Bottom" VerticalAlignment="Bottom">
      <ListBoxItem  ... />
      <ListBoxItem ... />
      <ListBoxItem  ... />
    </ListBox >
  </DockPanel>
</ScrollViewer>
@Odonno

This comment has been minimized.

Show comment
Hide comment
@Odonno

Odonno Feb 16, 2017

Collaborator

What is the use case? Why not having a Trigger when the Height is smaller to reduce the number of items in the Pane?

Collaborator

Odonno commented Feb 16, 2017

What is the use case? Why not having a Trigger when the Height is smaller to reduce the number of items in the Pane?

@deltakosh

This comment has been minimized.

Show comment
Hide comment
@deltakosh

deltakosh Feb 16, 2017

Contributor

You can also update the hamburgermenu template in your application to add the DockPanel

Contributor

deltakosh commented Feb 16, 2017

You can also update the hamburgermenu template in your application to add the DockPanel

@deltakosh

This comment has been minimized.

Show comment
Hide comment
@deltakosh

deltakosh Feb 16, 2017

Contributor

I will close this one for now. The platform team is working on a Hamburger Menu replacement. I do not want to accept more PR on our version as it will be deprecated soon in favor of the platform one (https://developer.microsoft.com/en-us/windows/platform/features/navigationview/?q=hamburger)

Contributor

deltakosh commented Feb 16, 2017

I will close this one for now. The platform team is working on a Hamburger Menu replacement. I do not want to accept more PR on our version as it will be deprecated soon in favor of the platform one (https://developer.microsoft.com/en-us/windows/platform/features/navigationview/?q=hamburger)

@deltakosh deltakosh closed this Feb 16, 2017

@amkuchta

This comment has been minimized.

Show comment
Hide comment
@amkuchta

amkuchta Feb 16, 2017

Contributor

@deltakosh heard - thanks for the update! My use case is simply that my window has the ability to be shrunk to a point (vertically) where the number of items exceeds the available space. I don't understand why Options items are given priority over Standard items - to me, it is counter intuitive to hide anything on the window as it responds to size changes unless that is the intended effect. It also makes more sense to me to let users know immediately that scrolling is available, hence the suggestion to shift the scrollbar to the left.

Contributor

amkuchta commented Feb 16, 2017

@deltakosh heard - thanks for the update! My use case is simply that my window has the ability to be shrunk to a point (vertically) where the number of items exceeds the available space. I don't understand why Options items are given priority over Standard items - to me, it is counter intuitive to hide anything on the window as it responds to size changes unless that is the intended effect. It also makes more sense to me to let users know immediately that scrolling is available, hence the suggestion to shift the scrollbar to the left.

@deltakosh

This comment has been minimized.

Show comment
Hide comment
@deltakosh

deltakosh Feb 16, 2017

Contributor

Completely agree. options items list is declared after main list so it is rendered on top of it.
we should have added the two list view inside a grid with two rows

Contributor

deltakosh commented Feb 16, 2017

Completely agree. options items list is declared after main list so it is rendered on top of it.
we should have added the two list view inside a grid with two rows

@amkuchta

This comment has been minimized.

Show comment
Hide comment
@amkuchta

amkuchta Feb 16, 2017

Contributor

Concur - is there any way to push this up to the platform team as a suggestion to have it implemented there?

Contributor

amkuchta commented Feb 16, 2017

Concur - is there any way to push this up to the platform team as a suggestion to have it implemented there?

@deltakosh

This comment has been minimized.

Show comment
Hide comment
@deltakosh

deltakosh Feb 16, 2017

Contributor

I will for sure

Contributor

deltakosh commented Feb 16, 2017

I will for sure

@amkuchta

This comment has been minimized.

Show comment
Hide comment
@amkuchta

amkuchta Feb 16, 2017

Contributor

Thanks!

Contributor

amkuchta commented Feb 16, 2017

Thanks!

@Odonno

This comment has been minimized.

Show comment
Hide comment
@Odonno

Odonno Feb 16, 2017

Collaborator

@amkuchta @deltakosh Menu Items should be prioritized over Options Items. I don't know why it was not implemented this way. I think there is an easy fix for that like @deltakosh said.

Collaborator

Odonno commented Feb 16, 2017

@amkuchta @deltakosh Menu Items should be prioritized over Options Items. I don't know why it was not implemented this way. I think there is an easy fix for that like @deltakosh said.

@amkuchta

This comment has been minimized.

Show comment
Hide comment
@amkuchta

amkuchta Feb 16, 2017

Contributor

@Odonno personally, I think that having either one prioritized over the other is a mistake - I personally separated mine because the logical division just made sense, not because one was more important than the other. As for implementation to correct, either @deltakosh or mine is fine - I just know that mine works (I tested before I suggested 😆)

Contributor

amkuchta commented Feb 16, 2017

@Odonno personally, I think that having either one prioritized over the other is a mistake - I personally separated mine because the logical division just made sense, not because one was more important than the other. As for implementation to correct, either @deltakosh or mine is fine - I just know that mine works (I tested before I suggested 😆)

@deltakosh

This comment has been minimized.

Show comment
Hide comment
@deltakosh

deltakosh Feb 16, 2017

Contributor

We can still merge your fix if it is simple. I mean: the hamburger menu will be deprecated but in the meantime nothing prevents us to fix issues for the one we have

Contributor

deltakosh commented Feb 16, 2017

We can still merge your fix if it is simple. I mean: the hamburger menu will be deprecated but in the meantime nothing prevents us to fix issues for the one we have

@amkuchta

This comment has been minimized.

Show comment
Hide comment
@amkuchta

amkuchta Feb 16, 2017

Contributor

Oh, I meant implementation on the next iteration @deltakosh. If you'd like to implement the change here, I am all for it, but I understand the desire to wait for the next version. Any ideas on when that will be implemented?

Also, @punker76, I thought I would bring you into this, just so you are aware for MahApps.Metro

Contributor

amkuchta commented Feb 16, 2017

Oh, I meant implementation on the next iteration @deltakosh. If you'd like to implement the change here, I am all for it, but I understand the desire to wait for the next version. Any ideas on when that will be implemented?

Also, @punker76, I thought I would bring you into this, just so you are aware for MahApps.Metro

@deltakosh

This comment has been minimized.

Show comment
Hide comment
@deltakosh

deltakosh Feb 16, 2017

Contributor

The next version will be done by platform team. I cannot share the ETA yet.
So please go ahead and submit a fix :)

Contributor

deltakosh commented Feb 16, 2017

The next version will be done by platform team. I cannot share the ETA yet.
So please go ahead and submit a fix :)

@amkuchta

This comment has been minimized.

Show comment
Hide comment
@amkuchta

amkuchta Feb 17, 2017

Contributor

@deltakosh @Odonno corrected via #960, the pull request is awaiting approval now. Thanks for the feedback on the suggestion!

Contributor

amkuchta commented Feb 17, 2017

@deltakosh @Odonno corrected via #960, the pull request is awaiting approval now. Thanks for the feedback on the suggestion!

@nmetulev

This comment has been minimized.

Show comment
Hide comment
@nmetulev

nmetulev Jun 21, 2017

Member

FYI, we are reverting this back to the earlier behavior in order to match the behavior in the new NavigationView. See #1229

Member

nmetulev commented Jun 21, 2017

FYI, we are reverting this back to the earlier behavior in order to match the behavior in the new NavigationView. See #1229

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment