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

feat(version-switcher): add version switcher #433

Closed
wants to merge 3 commits into from

Conversation

ThuWangzw
Copy link
Contributor

Refs: #23

@drammock @MegChai @choldgraf @tupui FYI

I implement a draft version switcher:
image

  1. In this implementation we use config.json rather than conf.py. @MegChai has explained clearly in #23.
  2. We enable you to change version of a specific page. For example, if you are in xxx.com/1.0/guide/info.html and swicth to version 1.1. The page will be redirected to xxx.com/1.1/guide/info.html rather than xxx.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 in config.json.
  3. You can change the style of version switcher by changing the style of #version-dropdown and #version-menu, which are id of version switcher's elements. You can also change their class dynamicly.
  4. If an error occurs(e.g. config.json doesn't exist), it will be like this to give a notice:
    image
    and you can check the details in console.
  5. A locale switcher is also implemented and I will make a PR if needed :-).

How to use version switcher?

  1. add two options in html_theme_options in conf.py: config_json_url, which means the URL of config.json; use_version_swicth, which means if you want to enable the version switcher(True of False)
  2. add template html version-switcher.html to somewhere you like. For example, it's in navbar_end in our website.
  3. add a config.json in the place of config_json_url. name and labels must be unique. It will be like:
    {
          "items": [
              {
                  "name": "v1.0",
                  "url": "1.0",
                  "labels": []
              },
              {
                  "name": "v1.3",
                  "url": "1.3",
                  "labels": ["stable"]
              },
              {
                  "name": "v1.4",
                  "url": "1.4",
                  "labels": ["latest"]
              },
          ],
          "default": "stable"
      }
  1. organize your website properly in file system. For example:
root
 - 1.0
 -- index.html
 - 1.3
 -- index.html
 - 1.4
 -- index.html
 - stable
 -- index.html

Then you can view your website on http://localhost/stable/index.html.

Feel free to share your ideas! 😄

@ThuWangzw
Copy link
Contributor Author

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'.
This is an example in our website:

    "config_json_url": "/doc/config.json",
    "switchers": ["version"],

The switchers option has 2 effects:

  1. As we are designing both version switcher and locale switcher, this option indicates which switcher to use. 'version' in this array means enabling version switcher.
  2. The order of the switcher indicates the order in URL. For example, if the option is ["version", "locale"], the URL will be like localhost/1.0/en/guide/index.html. If you change the option to ["locale", "version"], the URL will be like localhost/en/1.0/guide/index.html.

@drammock
Copy link
Collaborator

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 config.json approach (if you parse the JSON in conf.py to get the variables available to sphinx, the template should just work). In contrast, your proposed implementation is ~150 lines of Javascript.

@ThuWangzw
Copy link
Contributor Author

ThuWangzw commented Jul 22, 2021

I'm not familiar with Jinja and I have following questions:

  1. Is Jinja able to fetch config.json from server? Based on previous disscussion it's a need to fetch config.json and generate html dynamicly.
  2. Can I switch among the versions of a specific page?

We enable you to change version of a specific page. For example, if you are in xxx.com/1.0/guide/info.html and swicth to version 1.1. The page will be redirected to xxx.com/1.1/guide/info.html rather than xxx.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 in config.json.

@choldgraf
Copy link
Collaborator

@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?

@drammock
Copy link
Collaborator

drammock commented Jul 22, 2021

@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.

Is Jinja able to fetch config.json from server?

You can load config.json in conf.py; the variables in conf.py are available to Jinja site-wide.

Can I switch among the versions of a specific page?

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 wrote it adapted it to work with this theme.

@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
Copy link
Contributor

A layout suggestion: IMHO it would make sense to put the version switcher next to the logo; e.g. as a mockup:

grafik

It should then be colored more lightly to not competete too much with the logo.

@tupui
Copy link
Contributor

tupui commented Jul 25, 2021

@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).

@choldgraf
Copy link
Collaborator

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

@ThuWangzw
Copy link
Contributor Author

@drammock Here's what's my ~150 lines Javascript do:

  1. Download .json from server(~ 10 lines). I don't think Jinja can do this.

You can load config.json in conf.py; the variables in conf.py are available to Jinja site-wide.

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 config.json from your server dynamically rather than load it in conf.py.

  1. Parse URL to define which parts contain the version information(~ 30 lines). Yes, I can use Jinja rather than Javascript. However, we want to add other switchers like a locale switcher in future work. I implement this function in a reusable way, so you can also use this function to parse other information.

  2. Check if the version information contained in the URL is valid(~ 15 lines). It can enhance robustness.

  3. Error handling (~ 5 lines). If an error occurs, the switcher will become an error button to give notice.

  4. Render (~ 30 lines). If all information is right, render the web page.

  5. Comments, blank lines...

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.

@drammock
Copy link
Collaborator

drammock commented Jul 28, 2021

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 config.json from your server dynamically rather than load it in conf.py.

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.

@ChaiByte
Copy link
Contributor

ChaiByte commented Jul 29, 2021

@drammock Your implementation is also a great idea. In a sense, it is actually more stable.

but only if you rebuild the older doc versions.

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 config.json is stable. If there are breaking changes in the future, users might need to edit the html_theme_options and rebuild the old version document then.


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 docs/_static/switcher.json file (#436), or just keep it as it is. If users do not want to rebuild, then use this plan. Our initial design inspiration actually came from #276. I don't know if @greglucas still pays attention to this matter.

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 website/stable/xxx page.

@drammock
Copy link
Collaborator

drammock commented Jul 29, 2021

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

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:

https://github.com/pydata/pydata-sphinx-theme/pull/276/files#diff-58fdf939eadc82b289cba1608a6c9c63eab60ecfe590dc3f57b3c57f82650d51R25-R36

@ChaiByte
Copy link
Contributor

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

I think my implementation actually did not have this problem. It populates the menu with links to each versions homepage first, then updates then only if the more specific page actually exists

Cool idea. @ThuWangzw FYI

@greglucas
Copy link

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.

theme_versions = url/to/versions.json or theme_versions = { json-like dict that can be included}

There should be a graceful fallback to the base page. The PR I put up had this check in it:
https://github.com/pydata/pydata-sphinx-theme/pull/276/files#diff-58fdf939eadc82b289cba1608a6c9c63eab60ecfe590dc3f57b3c57f82650d51R25-R36

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 docsbuild-scripts https://github.com/python/docsbuild-scripts/blob/main/templates/switchers.js
Discussion thread: https://mail.python.org/pipermail/doc-sig/2020-June/004200.html
There is some good code for fallbacks in there to draw inspiration from as well, but I think the whole point of this implementation is to move away from manually backporting all doc builds just to add a version to the switcher :)

The current versions.json as you have above is quite verbose with all of the options and having an "items" level. The original thread had a link to Bokeh's versions.json file https://docs.bokeh.org/versions.json which seems like a good rough template to follow.

@drammock
Copy link
Collaborator

I think the problem now seems to be that we seem to be unable to predict others' demands. (@MegChai)

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.

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. (@greglucas)

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

{% if switcher_dict is defined and switcher_dict is not none %}
    # use the mapping provided in conf.py in the variable switcher_dict
{% else %}
    # getJSON() and construct mapping from that
{% endif %}

Otherwise, if we try to do that logic within conf.py we're still stuck with needing to re-build old versions of the docs in order for them to pick up changes to the JSON.

As a side note, for easiest compatibility with the JSON structure I think switcher_dict would actually need to be a list of dicts (one dict per version) with each dict having keys language and/or version (whatever is needed for the URL template), and maybe also name (to allow the menu item to say "stable" or "latest" instead of a specific version number, if desired).

@ThuWangzw
Copy link
Contributor Author

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. @drammock

Good idea! I follow your idea and implement this feature.
Moreover, thanks to @greglucas 's idea, I simplify the config.json. Now it looks as follows:

{
    "version": [
        "1.3",
        "1.4",
        {
            "name": "1.5",
            "label": "stable"
        }
    ]
}

config["version"] is a list of versions. Each element indicates a version (string or object are both ok).

@drammock
Copy link
Collaborator

@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 Cross-Origin Request Blocked: The Same Origin Policy disallows reading the remote resource at https://pydata-sphinx-theme.readthedocs.io/en/v0.6.0/index.html. (Reason: CORS header ‘Access-Control-Allow-Origin’ missing). in the browser console). This should not happen on the real doc site, because the request will originate from the same domain instead of from ...org.readthedocs.build

@damianavila
Copy link
Collaborator

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 src/js/index.js file: https://github.com/pydata/pydata-sphinx-theme/pull/436/files#r755515548

More details here: #23 (comment)

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

Successfully merging this pull request may close these issues.

None yet

8 participants