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

Make the nav sidebar's scroll-to-current-page behaviour less jumpy #1293

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

Conversation

tsibley
Copy link

@tsibley tsibley commented Mar 29, 2022

This avoids scrolling the sidebar when the link is already in view by
using "nearest" for the vertical axis ("block") instead of the default
of "start". This behaviour is much better when clicking on items in the
current viewport, especially if the current viewport matches the initial
viewport.

It's still a little jumpy when the current link isn't visible in the
initial viewport, because the sidebar position still changes across page
loads. But the new behaviour solves the big problem on initial home
page loads with a long sidebar where the logo and project title would be
immediately scrolled out of view.

Related to #824.


Quick video demonstrating before/after.

@tsibley tsibley requested a review from a team as a code owner March 29, 2022 23:54
@nienn
Copy link
Contributor

nienn commented Mar 30, 2022

Hi @tsibley, thank you for submission!
This probably needs more in-depth testing with multiple platforms and edge cases, but I think it can greatly improve the user experience with the sidebar.

@tsibley
Copy link
Author

tsibley commented Mar 31, 2022

@nienn Yep, understood about testing! Data on caniuse.com shows pretty good support for {block: "nearest"} relative to browser-usage across the board, with the notable exception of Safari (on macOS and iOS).

image

I suspect (but haven't tested) that when the options object parameter isn't supported, as in Safari, it will be treated as a truthy value for the alignToTop boolean parameter and so will in effect act like the current behaviour.

Would be good to test all this empirically, of course. Does RTD have the resources to do this?

This avoids scrolling the sidebar when the link is already in view by
using "nearest" for the vertical axis ("block") instead of the default
of "start".  This behaviour is much better when clicking on items in the
current viewport, especially if the current viewport matches the initial
viewport.

It's still a little jumpy when the current link isn't visible in the
initial viewport, because the sidebar position still changes across page
loads.  But the new behaviour solves the big problem on initial home
page loads with a long sidebar where the logo and project title would be
immediately scrolled out of view.

Related to readthedocs#824.
@tsibley tsibley force-pushed the trs/nav-sidebar-scrollIntoView branch from aff50d9 to b8535ae Compare July 8, 2022 17:27
@tsibley
Copy link
Author

tsibley commented Jul 8, 2022

Repushed after rebasing onto the current tip of master and also regenerating the built theme.js asset so it matches the source file.

tsibley added a commit to nextstrain/sphinx-theme that referenced this pull request Jul 8, 2022
Overrides the default static/js/theme.js from sphinx-rtd-theme with our
customized copy that fixes the initial scroll-into-view behaviour for
the nav sidebar, c.f. <readthedocs/sphinx_rtd_theme#1293>.
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.

2 participants