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

Proposal: Improve Navigation Sidebar UX by making it scroll-able. #914

Closed
Dajust opened this issue Feb 19, 2017 · 7 comments
Closed

Proposal: Improve Navigation Sidebar UX by making it scroll-able. #914

Dajust opened this issue Feb 19, 2017 · 7 comments
Labels

Comments

@Dajust
Copy link

Dajust commented Feb 19, 2017

I'm forced to scroll the main content to the bottom before I could see the rest content on the Navigation Sidebar content hidden at the bottom.

Can I help? ✋

@TheLarkInn
Copy link
Member

Yes absolutely, we would love a PR for this. Do you already know the solution in mind?

@skipjack
Copy link
Collaborator

skipjack commented Feb 19, 2017

This was fixed in #906, not sure if it was deployed yet though.

EDIT: Just realized this is still waiting to be tested... @Dajust wanna give it a test? Let me know if you see any issues, if not I can merge so we can get it deployed.

@Dajust
Copy link
Author

Dajust commented Feb 19, 2017

@TheLarkInn Sure! Just waiting for approval to go ahead. @skipjack Awesome fix in #906 !

@skipjack skipjack added the UI/UX label Feb 19, 2017
@Dajust
Copy link
Author

Dajust commented Feb 19, 2017

Sure @skipjack! let me fork the repo, I'll test and let you know ASAP!

@Dajust
Copy link
Author

Dajust commented Feb 19, 2017

Nice job @skipjack, I've tested it. However, IMHO, I think the UX still lags. Below are few ways I feel we could improve on it.

Scroll action on Sidebar should not affect the Main Section. Vice versa.
When I scroll to the top or the bottom of the Sidebar, I don't want the Main Section scroll to kick in.
When I scroll to the bottom of the Sidebar, then try to scroll to the top of the Main Section, I lost the scroll state of the Sidebar. Frustrating. See how Gitbook does this for example.

I understand that our challenge here is the Top-bar. Using JavaScript to calculate when and when not to make the Sidebar position fixed based on when the Top-bar is in/out of the page.

But how about we forget JS for a moment. Use flex-box to implicitly make the Topbar, Sidebar, and Main Section fixed, then make their contents scrollable. I guess we could achieve a lot more with flex-box than I can possibly explain here. We could also style the scrollbar to look nicer.

I'll be more than glad to assist!

@skipjack
Copy link
Collaborator

skipjack commented Feb 21, 2017

@Dajust thanks for testing.

Scroll action on Sidebar should not affect the Main Section. Vice versa. When I scroll to the top or the bottom of the Sidebar, I don't want the Main Section scroll to kick in.

I get what you're saying, but I don't think it's that clear cut... see my comments on using flex below.

When I scroll to the bottom of the Sidebar, then try to scroll to the top of the Main Section, I lost the scroll state of the Sidebar. Frustrating.

This I do understand, I think always applying the maxHeight, instead of only when it's fixed, would fix this issue. I might add this to that PR prior to merging.

But how about we forget JS for a moment. Use flex-box to implicitly make the Topbar, Sidebar, and Main Section fixed, then make their contents scrollable. I guess we could achieve a lot more with flex-box than I can possibly explain here.

I thought about this a lot while implementing the fixed sidebar. As you say if we display: flex and height: 100vh the entire site wrapper, flex: 0 0 auto the <Navigation /> and <Footer />, and then flex: 1 1 auto the content then no JavaScript trickery is needed to fix the sidebar. However this also means that on mobile screens the <Navigation /> and <Footer /> would always be visible unless we did some trickery to hide those. It looks like react headroom, something we've already discussed using, could still be implemented with the parent prop to show/hide the top navigation bar on scroll of the content but I'm not sure how we'd hide the footer. Either way I'd like to give this a little more thought first because it would be a significant change.

We could also style the scrollbar to look nicer.

I would stay away from customizing the scrollbar but we do have plans for a nicer overall design for the sidebar component (see #438).

I'll be more than glad to assist!

We're happy to have your help, and appreciate you creating issues to kick some ideas around before jumping into implementation. I'll keep you in mind as I'm going through the last couple UI/UX issues and I'll give the layout changes you proposed some more thought.

@skipjack
Copy link
Collaborator

Closing with merge of #906.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants