pydata / pydata-sphinx-theme

A clean, three-column Sphinx theme with Bootstrap for the PyData community
https://pydata-sphinx-theme.readthedocs.io
BSD 3-Clause "New" or "Revised" License
557 stars 300 forks source link

Wrong table of content section highlighted #1889

Open cosenal opened 1 week ago

cosenal commented 1 week ago

When I go to a section, I expect the same section to be highlighted and any subsections expanded in the table of contents sidebar. Instead the incorrect section is highlighted (see screenshot).

Screenshot 2024-06-18 at 20 43 26

To reproduce: https://mitiq.readthedocs.io/en/stable/apidoc.html#core-utilities (it happens in local builds as well, not only on readthedocs builds.)

pydata-sphinx-theme version: 0.15.2

Configuration: https://github.com/unitaryfund/mitiq/blob/bf82e84410e1805437e15f1d26b0c5be8e8dcb8a/docs/source/conf.py#L337

Tested on Firefox 127 on Mac OS 14.5.

drammock commented 1 week ago

When I go to the provided URL, I see this (which looks correct to me):

Screenshot 2024-06-18 at 13-51-06 API-doc — Mitiq 0 37 0 documentation

If I push the up-arrow one time to scroll up a tiny bit, then I see the sidebar section highlighting that you see --- even if that prior subsection is barely visible below the top navbar. So I think this is a case of the scrollspy offset (which is set to 180px) being too small for your taste.

@gabalafou any input here w/r/t usability considerations around scrollspy? In my experience it's hard to find a combination of offset & margin/padding that everyone will agree "feels" like the transition happens in the right place, but I'm happy to be wrong if we can reach consensus on a change to our current behavior.

Side note: I was able to "break" the scrollspy on that page by manually dragging around the scrollbar to get just barely above the transition point between the "Tools..." and "Core utilities" sections. At that point, the highlighting on "Raw" subsection went away and "Tools..." stayed highlighted until getting as far down as the second subsection of "Core utilities" (i.e., "mitiq interface"). That is different from what you see, might have something to do with padding/margins on the subsection elements, and might be considered a bug in bootstrap (would need to investigate further).

cosenal commented 1 week ago

Interesting that you see something different. I tried on other browsers and other resolutions and I still see the section highlighted in my original screenshot 🤔

Carreau commented 1 week ago

I can reproduce and I think this is because of the scroll offset due to the nav bar. This is a recurrent problem when scrolling to a target, and it might be a problem in bootstrap we cant' do much about.

See the following video, when I scroll a tiny bit, leading to the last element of "tools for error mitigation" to disapear from the top of the screen, but already hidden by the navbar, then the sidebar is correct.

https://github.com/pydata/pydata-sphinx-theme/assets/335567/16c269ba-f484-4db9-bac1-cee4bd6804f3

cosenal commented 1 week ago

Thanks for reproducing it, @Carreau.

Some more info that may help debugging this: the behavior used to be correct in an old version of our apidoc, see https://mitiq.readthedocs.io/en/v0.34.0/apidoc.html#core-utilities, which was on pydata-sphinx-theme==0.11.0. When we upgraded the theme dependency to pydata-sphinx-theme==0.15.2, the new faulty behaviour started to manifest itself. Unfortunately it was a big bump, so it may be hard to pinpoint what is causing this, but here is to say that it used to work at some point with the same theme.

Carreau commented 1 week ago

bootstrap scroll spy has data-offset="180" on the element, it seem from newer bootstrap docs that it's now data-bs-offset, though locally data-offset is respected, so I'm not sure.

trallard commented 1 week ago

The scroll highlight behavior has been consistently flaky. IIRC @gabalafou and I discussed this briefly a while back. I had not noticed the offset re the navbar so will let Gabriel chime in. At the time it seemed that a significant overhaul of this was not justified enough in terms of effort/impact.