v0.16.0 breaks left sidebar toggling (STB)

[jakevdp]

In v0.14.4 and older, clicking the ☰ icon in the upper left on a wide-screen view collapses the sidebar

In v0.15.0 and newer, clicking the icon instead highlights the sidebar, making the rest of the page dark (I think this is the intended behavior on narrow-width screens, where the sidebar is hidden by default).

We've had to pin v0.14.4 in JAX's doc builds to work around this. https://github.com/jax-ml/jax/blob/d990dcf24291fa4a89f92ed6923e9d070854359f/docs/requirements.txt#L3

cc/ @trallard

[trallard]

I want to add a bit more context here. We see this in Sphinx Book (which relies on PST), but recent PST releases introduced it.

See, for example, the previous (and what would be the expected behaviour as the hamburger menu is used there to collapse the sidebar) in Sphinx Book (in their stable docs, using PST=0.15.2, executablebooks/sphinx-book-theme@6e93460#diff-50c86b7ed8ac2cf95bd48334961bf0530cdc77b5a56f852c5c61b89d735fd711R38)

sphinx-book-correct.mp4

vs the docs in latest (using PST=0.16, https://github.com/executablebooks/sphinx-book-theme/blob/4a1a590c71414c061e14ee342ac712b54bca46c9/pyproject.toml#L40)

sphinx-book-dev.mp4

I need to do some digging up, but I am pretty sure this is a consequence of #1942 which made the overlay behave as a modal for accessibility purposes.

[trallard]

Adding a bit more data points

⛔ Broken sidebar collapse behaviour - using <dialog id="pst-primary-sidebar-modal"></dialog> and thus the modal pattern

• sphinx-book-theme==1.1.3
• pydata-sphinx-theme==0.16.0

✨ Sidebar toggle works as expected * - the Sphinx book theme folks had already patched some breaking changes we introduced via executablebooks/sphinx-book-theme#841

• sphinx-book-theme==1.1.3
• pydata-sphinx-theme==0.15.3 and pydata-sphinx-theme==0.15.4

Note that I added a * to works as expected above, as the Sphinx Book theme re-uses the toggle to collapse the sidebar. In this case, "works as expected" means "the toggle collapses the sidebar regardless of the display."

We introduced a breaking change in #1942, when we made the sidebar behave as a modal.
I do not think we should revert this, as it was added as part of the accessibility improvements we have introduced to the theme.

@gabalafou I see the following potential paths:

1. Work with the Sphinx book folks to patch the sidebar toggle/collapsing behaviour (though this seems like an anti-pattern and would undo the a11y enhancements we made here in PST)
2. We have an open feature request for a collapsible sidebar (Make it possible to collapse the primary sidebar on wide screens #1072), including the design proposal by @smeragoel so a better path seems to work towards adding this to PST and work with Sphinx book folks to use this vs relying on the current modal/toggle (the caveat is that this will of course take longer time than patching the current sidebar/toggle behaviour)

WDYT?

Tagging @agoose77 and @choldgraf for visibility on the Sphinx book side of things

[agoose77]

@trallard thank you for doing some diagnostics here. We were aware of this breaking change downstream, but have not yet had a chance to conclude next steps (see executablebooks/sphinx-book-theme#865 (comment) for more).

Over in Executable Books, we are low on maintainer time, and the Jupyter Book ecosystem is moving towards a non-Sphinx tech stack. So, my inclination is to drop support for the toggle-able sidebar in sphinx-book-theme, in order to avoid introducing technical debt and keep as close to upstream as is possible. We would definitely welcome a future PR here to add support for this behaviour upstream.

I'll wait to hear from @choldgraf, as this is a feature-loss and as such probably needs an additional maintainer's input.

[trallard]

Thanks for the swift response, @agoose77. I will probably add more details in the linked issue on SBT.

But after a quick check with @gabalafou from a usability and accessibility POV reverting to the old behaviour of the hamburger menu would be a no-go for PST.

We have had discussions around adding a collapsible behaviour directly in PST, but adding that new feature would take us a bit, so it's unlikely to make it to a PST release in, let's say, a few days/couple of weeks. IMO, this would be the most robust path forward as it will not revert the usability and accessibility improvements added so far. It will also follow industry standards for collapsible sidebars vs. relying on the mobile hamburger menu for this feature.

That means that in the meantime if you wanted to keep the previous behaviour on SBT around the toggleable sidebar you'd need to pin PST to 0.15.4 on SBT. Since this is a core feature of SBT it might make sense for now while we work on the collapsible sidebar on PST.

[choldgraf]

My feeling is that we don't have the maintainer capacity to fix this in the book theme, so while this is unfortunate, we should document the need to pin versions in order to avoid this behavior breaking. Then we should advocate for the PST to add sidebar collapsing in a future release.

That's under the assumption that the PST wants to support this feature, which it sounds like it does. If PST doesn't then the book theme might want to change how it handles the sidebar more drastically (but again I don't think we have the maintainer capacity to do this). In general, I recommend we keep the book theme as much of a light layer on top of the pydata theme as possible.

[agoose77]

Thanks @choldgraf. Pinning is a possibility here, provided that we can establish a time-line for the pin duration. I don't want to / can't commit any people in this project to develop that feature, but it would be really helpful to figure out a possible timeline in order to determine whether pinning in SBT is worth doing. Otherwise, I feel that we ought to intentionally remove the feature now whilst we have momentum and the issue is fresh.

[trallard]

I am in agreement with adding the collapsing feature to PST. This will reduce some of the maintenance burden on BST to by the sound of it.

I was planning to start a new release here which would take about 2-3 to call done between release candidates, patches needed and finalising a release. So any new features added starting now would only make it to subsequent PST releases.

Since this is a feature that we have discussed at length and have already done design and UX work it might be doable to include in the following release (or worst case scenario one after that).
Calendar wise that would put us somewhere around February next year.
Right now we are not adding a lot of new features but spending time prioritising bug fixes and maintenance so focusing on this addition seems doable.

I will let other folks in the team chip in shall they disagree or see potential blockers/delays I have not accounted for before making any final decisions though.