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

Fix the initial scroll-into-view behaviour of the nav sidebar #27

Merged
merged 1 commit into from
Jul 11, 2022

Conversation

tsibley
Copy link
Member

@tsibley tsibley commented 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.

Testing

  • Built docs locally and confirmed the scroll-into-view behaviour is better as expected.
  • Confirm on RTD PR build.

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>.
@tsibley
Copy link
Member Author

tsibley commented Jul 8, 2022

Comparing these two pages on RTD between stable and this PR

it seems like the behaviour is not quite the same as what I see locally (though it's still improved, but not as much as appears locally).

@tsibley
Copy link
Member Author

tsibley commented Jul 8, 2022

It's very close though, and the behaviour is identical locally and on RTD for other nav items. The difference for those particular links seems to be that the containing top-level item ("Paragraph Level Markup") is (for me) just barely longer than the viewport height when rendered on RTD with the RTD panel at the bottom of the sidebar, but not when locally without the RTD panel. Like I said though, the behaviour is still an improvement, esp. for top-level items that aren't as long.

@tsibley tsibley requested a review from a team July 8, 2022 18:02
Copy link
Member

@victorlin victorlin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clicked around on RTD stable vs PR builds, looks good!

@tsibley tsibley merged commit 10335fe into main Jul 11, 2022
@tsibley tsibley deleted the trs/fix-nav-sidebar-initial-scroll branch July 11, 2022 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

2 participants