-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Decouple the theme JS from readthedocs.org #3968
Decouple the theme JS from readthedocs.org #3968
Conversation
The lint errors are unrelated but there is an eslint error which is relevant. I believe I'll have to ignore it though since I specifically want to not have a global require. The point of this is to conditionally include JavaScript only for versions of the theme that require it. |
The whole point is to conditionally require the theme JS if it has not already been included.
Will this also include sphinx_rtd_theme if a different theme is used? |
Yes it will. That is how it currently works before this change as well. The sphinx_rtd_theme JavaScript is used (even on other themes) to power the version selector menu. This change did not change that. |
OK.
This feels a bit uncleanly separated, but I don't have a good working suggestion at this time. |
I don't disagree. This is also what I highlighted as an area for improvement above when I said:
I just view that as a bigger change. |
What's the state of this? Should we be moving forward w/ this now that 0.3.1 is out? |
I believe it should be rolled out. |
This is backwards compat with all versions of the theme right, so we can merge this, and it will work across all versions? |
It is supposed to, yes. |
Fix Build: Convert md to rst in docs
The whole point is to conditionally require the theme JS if it has not already been included.
…edocs.org into decoupling-theme-org
This PR changes the JavaScript injected onto docs pages. It does not include the Read the Docs theme JS if it is already included. After readthedocs/sphinx_rtd_theme#614 (presumably sphinx_rtd_theme>0.3.0, edit >=0.4.0) it will use the version of JS specific to the theme version. This PR does not rely on the other and can be merged independently.
This PR also documents why certain fixes are necessary and the versions and themes they are necessary on.
Related to but does not rely on readthedocs/readthedocs-sphinx-ext#39.
One area for improvement is that we probably don't need to source the entire theme JS (it comes from Bower). Instead, we should separate the theme JS from the JS needed to power the version selector menu.