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

Put the ReadTheDocs version switcher in the header navbar #705

Open
choldgraf opened this issue Jun 8, 2022 · 14 comments · May be fixed by #1398
Open

Put the ReadTheDocs version switcher in the header navbar #705

choldgraf opened this issue Jun 8, 2022 · 14 comments · May be fixed by #1398
Assignees
Labels
kind: enhancement New feature or request

Comments

@choldgraf
Copy link
Collaborator

choldgraf commented Jun 8, 2022

Context

ReadTheDocs will place a little hover widget over the page for most themes, this allows people to see a dropdown of versions and languages available. For example:

This switcher can be a little hard to spot, and also clashes with the visual style of the theme (e.g., it uses different colors and a UI layout, and doesn't change with dark theme etc).

Proposal

We should have an intentional location for the ReadTheDocs switcher somewhere in our theme layout. It could be placeable with a template component (e.g., readthedocs-switcher.html) and could be styled so that it works with this theme's structure/style.

Suggestion for implementation

ReadTheDocs will use JavaScript to inject this little widget on the server side when the page loads. This means that you can intercept this code and move the component wherever you like with some JavaScript.

Here's an example of code that does this for the book theme. It uses a MutationObserver to watch for the addition of the readthedocs HTML, and moves it to the sidebar:

https://github.com/executablebooks/sphinx-book-theme/blob/af6277a7276112ee915d901984193c49e56d5a76/src/sphinx_book_theme/assets/scripts/index.js#L189-L213

@choldgraf choldgraf added the kind: enhancement New feature or request label Jun 8, 2022
@choldgraf
Copy link
Collaborator Author

choldgraf commented Jun 8, 2022

cc @humitos or @pradyunsg if they know of a better way to do this :-) I also think this could be a useful component in the basic-ng theme (it would simply handle the placement of the RTD dropdown, not any styling)

@pradyunsg
Copy link
Contributor

pradyunsg commented Jun 8, 2022

I managed to do this in Lutra. :)

I'm on mobile, so no easy to click links for you. There's a CPython documentation build, linked to from an issue in Lutra's documentation. You can see how it looks there. Other than that, look at the variant-selector.html file.

Once we have some sort of concensus from RTD in the sphinx-basic-ng issue about RTD components, I'll add a component for this over there.

@humitos
Copy link

humitos commented Jun 8, 2022

Hi! I'm happy you are thinking and working on this. We have been thinking and trying to make the flyout menu more customizable and we wrote a design document some time ago to make it REST API-based instead of HTML blob, in particular, to allow people to do all this stuff in a clear and simple way: readthedocs/readthedocs.org#8052

Unfortunately, we weren't able to prioritize this work and it got staled 😢 . However, we re-added it to our roadmap and it's planned for Q3 --so, we should come back to it sooner than later 💪🏼

Now that you have thought about this and you probably have requirements/opinions/needings, it may be good if you can jump into that document and comment on some feedback you may have related to your experience trying to achieve this goal the hard way 😄

I don't remember if that document only talks about the technical side of this, but I know that we also talked about some "policy enforcement" (or similar) like not being able to remove it completely because it removes Read the Docs branding, and also communicate that it has to mention "Hosted at Read the Docs" somewhere on each page --currently, that's done in the flyout menu.

I know that I'm not giving any answer here 😅 , but I wanted to communicate the state of things at least.

@choldgraf
Copy link
Collaborator Author

choldgraf commented Jun 9, 2022

Just a quick thought there, I think that this would be very helpful! But creating a new API is a lot of design and implementation work + experimentation. I think the main goals here are much simpler than that. I'd argue for making iterative progress, and simply document + enable a pattern like:

  • The ReadTheDocs switcher widget will be inserted into the DOM just after < XXX >
  • To instead choose the DOM location where it is inserted, created an element with ID #readthedocs-widget-placeholder and it will be inserted there instead.
  • By default, the widget will load CSS that absolutely positions it in the bottom-right of the screen.
  • If you'd like to override this, you should define classes for < XXX >.

Basically all I need is for the XXXs to be filled in, which I hope is a lot simpler than creating a whole new API :-)

@ericholscher
Copy link

Yea, this is very similar to what we do with the ad placement, so it shouldn't be hard to modify our footer injection code to take this into account. The code that does this is pretty simple: https://github.com/readthedocs/readthedocs.org/blob/main/readthedocs/core/static-src/core/js/doc-embed/footer.js#L8-L15 -- and we could probably adopt similar logic to what we already have in the ad placement: https://github.com/readthedocs/readthedocs.org/blob/main/readthedocs/core/static-src/core/js/doc-embed/sponsorship.js#L37-L67

@choldgraf
Copy link
Collaborator Author

Yeah I find the ad placement thing totally fine to work with (once I found the documentation on it haha). This could be very similar. You might also also make some "good faith requests" in the docs like "please don't remove the ReadTheDocs branding, slogans, etc"

@12rambau
Copy link
Collaborator

Question: Why putting the RDT version swithcher in the header navbar and not at the bottom of the primary sidebar ? It's done in many theme (Furo, JupyterBook, insipid etc). There is not that much room left in the header navbar and things are going to get even more complex when we'll deal with #329.

@choldgraf
Copy link
Collaborator Author

The main reason I can think of is if some themes want the sidebar hidden on some pages (e.g., the landing page). I think if the search bar were ever moved to the header, then for some docs it would be quite common to have an empty sidebar otherwise.

@ericholscher
Copy link

@choldgraf We've discussed this a bit more on the RTD side, and we're looking at implementing logic that will inject the footer into a div with an id of #readthedocs-flyout-placement, similar to our ad client. I'll try and get a PR together this week that would ship next week.

@pradyunsg
Copy link
Contributor

pradyunsg commented Jun 20, 2022

x-ref: pradyunsg/sphinx-basic-ng#12

FWIW, I have been using a very horrible hacky blurb of HTML, that makes RTD embed logic work, by pretending to the an old version of Sphinx-RTD-theme: https://github.com/pradyunsg/lutra/blob/486e35b37d33ab65bc4d3de0c43314688327ff95/src/lutra/theme/lutra/partials/header/variant-selector.html#L3

Replacing that hack with an <div id="readthedocs-flyout-placement"></div> would be great! ^.^

PS: suggestion for a different name -- readthedocs-embed-flyout, which uses the JS files' name as a prefix, making it easier to correlate the two when you see them.

ericholscher added a commit to readthedocs/readthedocs.org that referenced this issue Jun 20, 2022
This PR does a few things:

* Lets themes define a `#readthedocs-embed-placement` element, where we will inject the footer
* Documents the flyout menu under Advanced Features

Refs pydata/pydata-sphinx-theme#705
ericholscher added a commit to readthedocs/readthedocs.org that referenced this issue Jun 27, 2022
This PR does a few things:

* Lets themes define a `#readthedocs-embed-placement` element, where we will inject the footer
* Documents the flyout menu under Advanced Features

Refs pydata/pydata-sphinx-theme#705

* Address review feedback

* Rename placement to readthedocs-embed-flyout
@drammock drammock linked a pull request Aug 2, 2023 that will close this issue
Carreau pushed a commit that referenced this issue Jul 29, 2024
Issue #705 can be
circumvented using the flyout version switcher. Explicitly drawing
attention to this in the documentation would be helpful to those
encountering this issue.
@dylanh724
Copy link

dylanh724 commented Aug 15, 2024

Apologies but how do we customize this? Is this essentially a theme override when RTD version flyout is detected?

@humitos
Copy link

humitos commented Sep 16, 2024

Hi all 👋🏼 . I just want to let you know that I've worked on a POC to integrate the Read the Docs version selector using Addons. This work could be used as an starting point for a better and more in-deep integration with your theme.

Peek 2024-09-16 16-27

I think that something similar could be done to integrate the language selector. Let me know if this example is useful to you.

@trallard
Copy link
Collaborator

Thank you @humitos! This looks pretty neat.

I think that something similar could be done to integrate the language selector. Let me know if this example is useful to you.

This also sounds fantastic! I have been looking into our localisation story recently, and I would love to explore how we could use this to add a language switcher.

I admit I still need to investigate what the upcoming changes to RTD addons mean for this theme, so I am quite behind on what the next steps would be for us.
Right now we can only use our switcher or the RTD one (

# Read the Docs functionality
) so at a first glance it would seem this one/or the other approach would remain, but based on your prototype we could then ensure they both have the same look and feel (already hinted at in another issue #1933 (comment)). If so I would be game to do that. What would the next steps for this look like @humitos?

Aside:
I can see there is a good amount of discussion in Furo already pradyunsg/furo#795 so I will follow that and open a meta issue of ours to align with the upcoming changes.

@Carreau Carreau self-assigned this Sep 19, 2024
@dylanh724
Copy link

Definitely looking forward to this - thanks for your great work :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants