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

Right hand TOC sidebar not using full screen height #827

Closed
at055612 opened this issue Jan 12, 2022 · 5 comments · Fixed by #828
Closed

Right hand TOC sidebar not using full screen height #827

at055612 opened this issue Jan 12, 2022 · 5 comments · Fixed by #828

Comments

@at055612
Copy link
Contributor

As you can see in this screen grab (of the bottom right corner of the screen) the right hand TOC sidebar is not using all the available screen when not scrolled to the bottom, so the vertical divider is short of the bottom.

image

If you scroll all the way down it is fine as the footer takes up the space.

image

Will raise a PR for the fix.

@LisaFC
Copy link
Collaborator

LisaFC commented Jan 13, 2022

Oh well spotted! Not sure how that happened...

@LisaFC
Copy link
Collaborator

LisaFC commented Jan 13, 2022

Actually I think I do know how it happened - we initially didn't have nearly as many additional repo and print options at the top, now that they're there they take up a lot of sidebar real estate.

One thing I have noticed in your preview is that using the full height does make it look quite different on long pages with a lot of headings (but maybe that's not a bad thing?)
https://deploy-preview-828--docsydocs.netlify.app/docs/adding-content/navigation/

Anyone else have thoughts? @narrenfrei?

at055612 added a commit to at055612/docsy that referenced this issue Jan 14, 2022
@chalin
Copy link
Collaborator

chalin commented Jan 14, 2022

One thing I have noticed in your preview is that using the full height does make it look quite different on long pages with a lot of headings (but maybe that's not a bad thing?)

It does look different, but I feel that it now looks correct IMHO.

@chalin
Copy link
Collaborator

chalin commented Jan 14, 2022

@LisaFC - the fix LGTM. Shall I merge or do you want to do it?

@LisaFC
Copy link
Collaborator

LisaFC commented Jan 14, 2022

Merge away!

chalin added a commit to at055612/docsy that referenced this issue Jan 14, 2022
chalin added a commit that referenced this issue Jan 14, 2022
Co-authored-by: Patrice Chalin <chalin@users.noreply.github.com>
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 a pull request may close this issue.

3 participants