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

Toggle primary sidebar button pops out that sidebar instead of collapsing it #865

Open
stevepiercy opened this issue Nov 12, 2024 · 10 comments

Comments

@stevepiercy
Copy link
Contributor

stevepiercy commented Nov 12, 2024

The PyData Sphinx Theme has changed their sidebar functionality to make it a modal pop-up for accessibility purposes. As a result, clicking the "toggle sidebar" button on wide-screens in this theme does not cause the sidebar to disappear, and instead makes it "pop" similar to what happens on mobile.

Here's the upstream issue:

Here's a suggested path forward

Original content

the following is the original post from @stevepiercy
On the Set up your development workflow, I did the following steps, skipping the sections in between these two.

When I click the Toggle primary sidebar button, the primary sidebar pops out and does not collapse.

Before clicking

Screenshot 2024-11-11 at 8 41 44 PM

After clicking

Screenshot 2024-11-11 at 8 41 33 PM

When I last tried this in mid-April 2024, this did not happen.

My theme depends on SBT, so I am very motivated to work on this issue. However I would like someone else to verify whether it is reproducible or just me before I spend time on it.

@danilopeixoto
Copy link

I could reproduce it using v1.1.3.

@stevepiercy
Copy link
Contributor Author

@danilopeixoto thanks for confirming.

I checked out the tag v.1.1.2, then proceeded to try documented ways to build the docs, none of which worked. Eventually I did the following.

# Create a Python 3.11 virtual environment at `.venv`, then proceed as follows
# Grab all the deps from `pyproject.toml` and install them,
# plus pin some versions and install sphinx-autobuild for a live HTML preview.
.venv/bin/pip install sphinx-book-theme-1.1.2 pydata-sphinx-theme==0.14.0 sphinx-autobuild \
ablog ipywidgets folium numpy matplotlib numpydoc myst-nb nbclient pandas plotly \
sphinx-design sphinx-examples sphinx-copybutton sphinx-tabs sphinx-togglebutton \
sphinx-thebe sphinxcontrib-bibtex sphinxcontrib-youtube sphinxext-opengraph
# build docs for live preview
.venv/bin/sphinx-autobuild --ignore "*.swp" -b html docs/ docs/_build/html

With sphinx-book-theme-1.1.2 and pydata-sphinx-theme==0.14.0, the issue does not appear. I tried to narrow it down further. Here's a summary of my findings.

  • >=0.14.0 <=0.15.2 - the menu works as expected.
  • >=0.15.3rc1 <=0.15.4 - it failed by neither closing nor popping up the sidebar, but instead the logo acquired focus.
    Screenshot 2024-11-18 at 11 56 28 PM
  • >=0.16.0rc0 - the issue reappears, where the sidebar pops out.

This is definitely an issue that was not caught in the upgrade of SBT to use the later versions of pydata-sphinx-theme.

For now, we're going to pin our requirements to pydata-sphinx-theme==0.15.2.

Sorry, I don't know where to begin to figure out the actual change in code that could cause this issue, but I hope someone now knows where to look between the versions. I could not find anything in the change log that jumped out at me.

@stevepiercy
Copy link
Contributor Author

Correction, for a temporary workaround, I had to set the following pins in my pyproject.toml.

dependencies = [
  "myst-parser==3.0.1",
  "pydata-sphinx-theme==0.15.4",
  "sphinx==7.3.7",
  "sphinx-book-theme==1.1.3",
]

@hennigfr
Copy link

Have been having the same issues with pydata-sphinx-theme==0.16.0, simply pinning pydata-sphinx-theme==0.15.4 solved it for me.
The problem seems to have been introduced with this PR; in particular these changes to the pydata javascript. It seems to me that in the pydata theme, the hamburger button and sidebar only appear on narrow and mobile screens, where the button makes the sidebar show as an overlay. The linked PR apparently did not take the book-theme's usage of the button into account.

@stevepiercy
Copy link
Contributor Author

@hennigfr correct, and thanks for hunting down the potential cause in those changes.

SBT is a child theme of PST. We need to adapt SBT to changes from its upstream dependencies. See https://sphinx-book-theme.readthedocs.io/en/stable/contributing/architecture.html#parent-theme-pydata-sphinx-theme.

Here's a PR preview build where you can demo the issue in SBT.

https://sphinx-book-theme--851.org.readthedocs.build/en/851/

Should SBT follow PST's lead and show the hamburger icon for the primary navigation sidebar only on mobile, not on desktop, instead of the old behavior of allowing users to toggle the display of the left navigation on all devices?

I would say, yes, because (1) it makes sense to display only the tools you need, (2) it would match the behavior of the right side page contents button, (3) one fewer variation to maintain separately from the parent theme, and (4) the second bullet point in https://sphinx-book-theme.readthedocs.io/en/stable/contributing/style.html#design-principles-and-inspiration.

Can we get a blessing to proceed on the best path forward from a maintainer @choldgraf or active contributor @agoose77?

As a side note, I'm concerned that there are several open PRs piling up since the last release on June 12, mostly dependabot. I'd be happy to chat in real time about this specific issue and overall maintenance.

@agoose77
Copy link
Collaborator

I recall engaging on this topic somewhere, but I can't seem to find the thread any more.

As this thread explores, this bug comes from an upstream change in pydata-sphinx-theme that subtly changed the behavior of the sidebar.

At face value, the toggleable sidebar is an important feature of sphinx-book-theme that allows a reader to hide the navigation content in-preference for a full-screen reading experience. I'm reluctant to drop that because of an upstream change. There is a need to balance overheads of maintaining deviations from upstream themes and our own feature set. Hopefully we can find a path that requires little work to implement.

@agoose77
Copy link
Collaborator

agoose77 commented Nov 25, 2024

@choldgraf do you have any heuristic here for the best path of action? I think it will take some work to coordinate with upstream about fixing this to restore the original behavior, and I am concerned that we don't have the bandwidth. Is a toggle-able sidebar a deal-breaker for this theme?

For the JB ecosystem, the myst-theme behaves identically to pydata-sphinx-theme, so we're moving away from this there.

@choldgraf
Copy link
Member

It sounds like the PyData theme is interested in adding the ability to collapse the sidebar, but it may not happen for another few months. With that in mind, I think we have two options. My suggestion is to go with option 1, but I invite suggestions otherwise.

Two options:

  1. Remove the toggle button on wide screens, and re-add it when the PyData theme natively supports it. In this scenario, we just remove this functionality to avoid confusion. The button will show up on narrow screens, but not wide screens, so you'll be stuck with the sidebar on wide screens. When The PyData theme re-adds this, then we can support it the same way they do.
  2. Pin to an earlier version of PyData until this is fixed. In this case, we change the pinning of pydata to <0.16. We can un-pin it when the feature is added.

I'd prefer 1 because then we gain the benefit of the improvements being made in the PyData theme, particularly around accessibility which is important. The ability to collapse the sidebar on widescreens feels like a "nice to have" but not a critical feature.

If there aren't any strong objections, then I'll plan to do this whenever I have a moment to look at the codebase again. If there are strong objections, I'll consider them but no promises - as @agoose77 mentions this project has very little maintenance bandwidth and we need to make choices that prioritize our own sanity. :-)

@stevepiercy
Copy link
Contributor Author

@agoose77's comment makes it sound like Sphinx Book Theme may become EOL in the not too distant future, as y'all plan to move away from a Sphinx stack. Is that correct?

Knowing for sure would help me decide whether to remove SBT from my stack and depend on PST only instead, cherry-picking the features from SBT that I want to support in my theme. Can y'all provide a confirmation of your plan and any insight to a timeline? Thank you!

@choldgraf
Copy link
Member

I think your analysis is reasonable - the book theme won't be discontinued, but we've intentionally been pushing features up into the pydata theme so that more people can benefit from it.

We've also got more folks helping to maintain and improve the pydata theme in general (for example by making the accessibility improvements that led to this particular feature breaking, which is a long term good thing but a short term inconvenience).

My hope is that the book theme can be a lightweight reskinning of the pydata theme that requires minimal maintenance. That way consumers of the book theme can keep using it with a more sustainable foundation.

Does that make sense / help?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants