-
Notifications
You must be signed in to change notification settings - Fork 200
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: Scroll padding on top for anchor links #669
Conversation
@mathbunnyru how does this look to you? Maybe @consideRatio is looking at this as well? you can see a demo here: https://sphinx-book-theme--669.org.readthedocs.build/en/669/ |
Thank you for such a quick response @choldgraf! Now there is some extra text above the anchor. |
Hmmm good point - it is a bit tricky to get just right, because if the top padding is too small then we will accidentally highlight the wrong section and the original problem will come back. I reduced the padding at the top in the latest commit to try to strike a balance. WDYT? |
OK how's this? It means that if we have two headers directly after one another, we might accidentally highlight the second, but maybe that's not such a big deal and also not common. |
Now it looks great to me 👍 |
OK I cleaned up a few other points that @mathbunnyru noted - will merge this if tests are happy since it's fixing a few bugs |
The failing test is a lighthouse performance check, which is notoriously flaky and I think can be ignored