-
Notifications
You must be signed in to change notification settings - Fork 328
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
feat(version-switcher): add version switcher #433
Conversation
Hi all! I abandoned the rough URL-splitting method and use an array to work as a URL template in new commit. We don't need the 'use_version_swicth' option but use an array named 'switchers'.
The
|
Did you try the Jinja-template-based approach suggested in two of my previous comments? The Jinja template is 11 lines long, and if I'm not mistaken, it should work fine with your |
I'm not familiar with Jinja and I have following questions:
|
@drammock what if we used a combination of your Jinja template to build the structure of the drop-down, and then use some light JavaScript to pull the latest drop-down list from a website and populate the drop-down fields with the elements of that list. That could be a nice mix of the two? |
That's basically what I've been encouraging @ThuWangzw to try. I don't have any special attachment to the Jinja solution, other than the fact that it seems simpler / easier to maintain --- since so many widely-used sites are now relying on the theme, I am pushing for a simpler solution in hopes it will "just work" across all of those sites. I don't want to risk creating headaches for lots of downstream users or create urgent bugfix demands on the theme maintainers.
You can load
I hadn't considered that. I don't know without trying it, though I suspect you might need some javascript to get that part of the functionality. But here again, I'm skeptical that you need ~150 lines of javascript to make this work... the javascript we use on the MNE-Python site to do that is 35 lines (see here), and probably could be made even shorter by someone who knows Javascript better than I did when I @ThuWangzw if you don't want to try this approach, let me know and I can give it a shot. Who knows, maybe it won't end up working and we'll end up going with your all-javascript approach in the end. But I think it's important to at least try out the simpler solution before commiting to this one. |
@timhoffm as far as I understood the discussion, they wanted to make this configurable. @ThuWangzw Is it still the case? I also prefer to have it close to the logo as when we are on a mobile it would still appear (this is what I did with SciPy). |
Yes I think we'd want this to be an HTML snippet that could be "included" in a section of the page similar to other components |
@drammock Here's what's my ~150 lines Javascript do:
If you load config.json in conf.py, the situation discussed here is still not solved. In my opinion, the only way is to render the webpage dynamically. So you need to load
So my solution is ~ 20 lines Jinja + ~ 150 lines Javascript. I think Javascript is capable of doing some complicated work such as dynamic rendering, error handling, and so on. It can also enhance the scalability and robustness of our system. |
You are correct about this. I realize now that loading the json via conf.py does allow you to have older doc versions benefit from updates to the JSON file, but only if you rebuild the older doc versions. Clearly it's better if they just read the JSON upon page load. @ThuWangzw @MegChai I've just opened PR #436 with an alternate implementation. I think it is simpler and I think it will still work, but I think you have more experience with Javascript than I do, so I'd appreciate if you (and everyone else) took a look to see how it can be improved / if it seems better than what you have here. |
@drammock Your implementation is also a great idea. In a sense, it is actually more stable.
The JavaScript script runs every time the user visits the website then reads the JSON file from someplace on the sever, so we don’t need to rebuild the old document. BUT in the current solution, we must ensure that the structure of I think the problem now seems to be that we seem to be unable to predict others' demands. @choldgraf What's your opinion? Do we have a way to make both solutions work -- Users can choose to rebuild all documents every time to get completed items through updating the I would like to remind you that another benefit of this scheme is that since each link is dynamically generated, we can switch versions between specific web pages without having to return to the home page every time. website/v1.3/somepage.html -> (change version to 1.4) -> (not to website/v1.4/index.html) -> website/v1.4/somepage.html While it brings benefits, it also introduces risks. When a page newly added in the new version jumps back to the old version, it will be displayed as a 404 page. We need to consider the frequency of this situation although I subjectively think that most of the time, people tend to visit the |
I think my implementation actually does not have this problem. It populates the menu with links to each version's homepage first, then updates them only if the more specific page actually exists: |
Cool idea. @ThuWangzw FYI |
It should be possible to satisfy both situations by allowing something like the versions variable to be either a url pointing to the json versions file, or a dict representing the json version.
There should be a graceful fallback to the base page. The PR I put up had this check in it: It looks like CPython manually updates all doc branches/builds of their version switcher and last year they moved this content out to a separate repo The current |
Honestly I don't think anyone will want to have to rebuild old versions of their docs. I think we should go with the approach of populating the menu items at load time, so that old versions of the docs will automatically include links to newer versions.
To preserve the benefits mentioned above, I think this would need to be done with Jinja in the HTML that defines the switcher dropdown. Maybe something like
Otherwise, if we try to do that logic within As a side note, for easiest compatibility with the JSON structure I think |
Good idea! I follow your idea and implement this feature.
|
@ThuWangzw @MegChai I think this is a great feature and I'd like to see it incorporated into the theme. But I think by now it's clear that we disagree about how it should be implemented. I'd like to invite you (and others) to look at my implementation in #436 (which after a rebase is now in mergable state) and comment on what exactly you think is missing from my approach or is superior in your approach. That way, the differences will be easier to see and the maintainers can make a decision on each point, and we can build off of your or my implementation (whichever is closer to what the maintainers desire). If you're worried about commit credit, I'm happy to incorporate review suggestions or PRs into my fork, so that some of the commits have your name on them (and the merged PR should then show us as co-authors). You can see my implementation in action here on the demo site: https://pydata-sphinx-theme--436.org.readthedocs.build/en/436/ Note that one aspect of the implementation that won't work on a pull-request-based build of the demo site: the JS cannot change the dropdown links from homepage to the corresponding specific page, because the cross-domain AJAX request (which checks to see if the corresponding specific page exists in the other doc versions) gets blocked by CORS policy (you'll see lots of things like |
After looking at both implementations, I think we should go with #436 after incorporating some of the ideas actually written here, particularly, I like the idea of having the JS code in the More details here: #23 (comment) |
Refs: #23
@drammock @MegChai @choldgraf @tupui FYI
I implement a draft version switcher:
![image](https://user-images.githubusercontent.com/26575579/126424181-724ac80e-c670-4e69-bc3c-99b99f306442.png)
xxx.com/1.0/guide/info.html
and swicth to version 1.1. The page will be redirected toxxx.com/1.1/guide/info.html
rather thanxxx.com/1.1/index.html
. @drammock's advice about specifying a URL template is a good idea and I will implement this in the next commits. However, in current commit I just split the URL to['xxx.com', '1.0', 'guide', 'info.html']
and check which element is the version's name by comparing the string with versions' name inconfig.json
.#version-dropdown
and#version-menu
, which are id of version switcher's elements. You can also change their class dynamicly.and you can check the details in console.
How to use version switcher?
html_theme_options
inconf.py
:config_json_url
, which means the URL ofconfig.json
;use_version_swicth
, which means if you want to enable the version switcher(True of False)version-switcher.html
to somewhere you like. For example, it's innavbar_end
in our website.config.json
in the place ofconfig_json_url
.name
andlabels
must be unique. It will be like:Then you can view your website on
http://localhost/stable/index.html
.Feel free to share your ideas! 😄