pydata-sphinx-theme icon indicating copy to clipboard operation
pydata-sphinx-theme copied to clipboard

Fix RTD version switcher hiding

Open drammock opened this issue 11 months ago • 6 comments

(supercedes) closes #705 closes #1092

WIP to fix the problem of the RTD version switcher disappearing on pages where the left sidebar is suppressed. The solution here is to piggyback on the theme's built-in version switcher, and just place the RTD switcher wherever the theme switcher was requested to go. Let's see if it works, if it does I'll update docs etc accordingly.

drammock avatar Aug 02 '23 15:08 drammock

hey @12rambau this is getting pretty close:

Screenshot_2023-08-05_12-00-07

maybe you can help with the last mile? Here's what remains:

  • [ ] needs documentation, I'll do that (have a local draft already)
  • [ ] test failures you can ignore, I'll fix them; they are because on this PR I'm embedding the RTD flyout instead of our own version switcher, so the file regression test fails.
  • [ ] the RTD flyout is not getting copied to the narrow-screen sidebar position. I tried to hack your MutationObserver code so that when RTD injects the flyout, it gets cloned, and then the onclick handler gets recreated (JS node cloning never copies event handlers). AFAICT the mutation observer is never even running (based on console.log entries not showing up). Any idea what is going wrong there?

drammock avatar Aug 05 '23 17:08 drammock

May I ask a perhaps controversial question: do we really need the Read The Docs version switcher?

gabalafou avatar Feb 21 '24 20:02 gabalafou

May I ask a perhaps controversial question: do we really need the Read The Docs version switcher?

It does more stuff than our in-built one does, and some users seem to want / prefer it. 🤷🏻 One solid advantage it has for RTD users is that it is config-free (no JSON file to keep updated) as it gets info about all the versions from the user's RTD account config / history.

Note also that RTD is testing out a rewritten version of their flyout which this PR won't handle... but it's an opt-in beta addon at the moment, and (I think?) RTD will continue to support the old version for quite some time, so if we can figure out the last missing bits here I still think this is a worthwhile addition to PST.

drammock avatar Feb 27 '24 18:02 drammock

Note also that RTD is testing out a rewritten version of their flyout which this PR won't handle... but it's an opt-in beta addon at the moment

Hi @drammock 👋🏼 . I'm one of the Read the Docs developers working on the new addons implementation for the flyout. The new implementation won't use a HTML blob that themes will need to inject as-is, but instead, it will expose all the data as a JSON object via a JavaScript CustomEvent where the theme could subscribe to render the flyout when that data is ready.

I wrote a POC for this implementation in the Read the Docs Sphinx's theme at https://github.com/readthedocs/sphinx_rtd_theme/pull/1526. It would be good if you can take a look at it and provide some feedback. I'll be happy to receive it 🙏🏼

RTD will continue to support the old version for quite some time, so if we can figure out the last missing bits here I still think this is a worthwhile addition to PST.

We don't plan to deprecate the old flyout immediately, but we will stop promoting it and promote more projects migrating to the new addons because it will have a lot more features and it will be easy to maintain and customize.

humitos avatar Feb 28 '24 15:02 humitos

Hey, @humitos - sniping here - as we have been auditing and remediating accessibility here, we have noticed some accessibility improvements needed in the RTD flyout (color contrast, keyboard navigation, and such). Where is the best place to pass this feedback? Is the linked POC ok? or would it be best to create a whole new issue?

trallard avatar Feb 29 '24 17:02 trallard

@trallard hi 👋🏼

Where is the best place to pass this feedback? Is the linked POC ok? or would it be best to create a whole new issue?

The work for the "generic" implementation of the new addons flyout lives in this repository https://github.com/readthedocs/addons so you can create an issue there.

However, if you are talking about the "integration of the flyout in the Read the Docs Sphinx theme", you can open an issue in the Sphinx's theme at https://github.com/readthedocs/sphinx_rtd_theme.

If you think that both have these accessibility issues, you can open an issue on both 😂

humitos avatar Feb 29 '24 18:02 humitos