executablebooks / sphinx-book-theme

A clean book theme for scientific explanations and documentation with Sphinx
https://sphinx-book-theme.readthedocs.io
BSD 3-Clause "New" or "Revised" License
431 stars 197 forks source link

FIX: Scroll padding on top for anchor links #669

Closed choldgraf closed 1 year ago

choldgraf commented 1 year ago

The failing test is a lighthouse performance check, which is notoriously flaky and I think can be ignored

choldgraf commented 1 year ago

@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/

mathbunnyru commented 1 year ago

Thank you for such a quick response @choldgraf!

Now there is some extra text above the anchor. https://sphinx-book-theme--669.org.readthedocs.build/en/669/notebooks.html#code-blocks-and-image-outputs

Screenshot 2023-01-05 at 18 54 57
choldgraf commented 1 year ago

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?

mathbunnyru commented 1 year ago

I think it looks better now, but I still see the text from the previous part. I guess it's impossible to have both things.

Screenshot 2023-01-05 at 19 16 40
choldgraf commented 1 year ago

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.

mathbunnyru commented 1 year ago

Now it looks great to me 👍

choldgraf commented 1 year ago

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