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

Page Sidebar to have dynamic height #92

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

teckliew
Copy link
Member

@teckliew teckliew commented Aug 20, 2018

Allow sidebar to have dynamic height and overscroll so that scrolling the sidebar won't affect the scrolling of the main content.

Also resized the sidebar's width to make room for the scrollbar.

**maybe have sidebar position fixed, so that its height is the height of the window?
The only thing I haven't figured out is how to shrink the sidebar height to make way for the footer when scrolling the main content to the bottom.

Enact-DCO-1.0-Signed-off-by: Teck Liew [email protected]

@Djspaceg
Copy link
Member

It will be important to verify that this change works well on pages with varying lengths of content; (some that have a scroller, some that don't). There was extensive testing done on screen sizes and page layout for all types and amounts of content, sidebar content amount, and all scenarios like those.

@teckliew
Copy link
Member Author

teckliew commented Aug 20, 2018

Yep, the proposed change makes sidebar's height equal the height of the main content to its right:

  • If the main content is taller than the sidebar, then it'll appear exactly the same as how it is now.
  • If the sidebar is taller, then it will be the height of main content with its own scrolling.

What would make it better is if the sidebar is the height of the window, but the footer makes it a bit complicated to implement that.

@lishammel
Copy link

@teckliew , @webOS101 - are these changes still needed? should they be merged in? Thanks.

@webOS101
Copy link
Contributor

Let's discuss when @Djspaceg returns. It's definitely an improvement, but I think we might can do it even better.

@Djspaceg Djspaceg self-assigned this Mar 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants