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

✨ ENH: Support version and locale switcher #360

Closed
wants to merge 2 commits into from

Conversation

cheekyshibe
Copy link
Contributor

@cheekyshibe cheekyshibe commented Mar 30, 2021

Refs: MegEngine#1 / #276 / #23

@greglucas FYI.

Create a simple version switcher to change versions and locales.

Features

  • User can enable version switch feature by setting theme_use_version_switch=True and set theme_version_switch_json_url = "/versions.json", then you will get an nicely dropdown menu that render everything from /versions.json
  • User can enable locale dropdown as well by enable theme_version_switch_enable_locale
    • different locales may have different versions (I believe this will happen frequently due to translation progress)
    • If the current version doesn't exist in the target locale, it will redirect to the default version in the target locale

@cheekyshibe cheekyshibe changed the title ✨ ENH: Support version switcher ✨ ENH: Support version and locale switcher Mar 30, 2021
@choldgraf
Copy link
Collaborator

Excellent thanks for picking this one up! Is there a way that we could demo this in the documentation? The theme isn’t translated to any other languages, but perhaps we can try this with the version switcher to see how things look?

@cheekyshibe
Copy link
Contributor Author

Excellent thanks for picking this one up! Is there a way that we could demo this in the documentation? The theme isn’t translated to any other languages, but perhaps we can try this with the version switcher to see how things look?

We have not yet provided the English version on our official website (so as you can see, the paths of en and zh are actually the same site). Related translation work may be carried out in the next few months. The locale switcher now only supports English and Chinese(中文),which might need further improvement.

Here is the way we tested the version switcher (you can use it the same way on demo site locally):

  • We generated several fake different version doc websites (just modified the version variable in source/conf.py and put |version| in API reference .rst file), or you can use the same pages.
  • Put them together in a tmp folder and write version.json file.
  • Start the local web server then verify.

@greglucas
Copy link

@MegChai, thanks for picking this up! At first glance it looks good on your site. For the language selector on your site, it looks like it may get cluttered if there is more than ~2 possibilities there, did you consider adding another dropdown selector for that?

I'll try and review this and test it out on a different local site build within the next couple of days.

@cheekyshibe
Copy link
Contributor Author

@MegChai, thanks for picking this up! At first glance it looks good on your site. For the language selector on your site, it looks like it may get cluttered if there is more than ~2 possibilities there, did you consider adding another dropdown selector for that?

I'll try and review this and test it out on a different local site build within the next couple of days.

You are right. The support for multi-language should be like what we have done for multi-version. Maybe it can also be generated from version.json and using some UI like this:

image

@choldgraf
Copy link
Collaborator

yep @MegChai using a dropdown for this as well sounds like a good idea, since many projects will have several languages.

Copy link

@greglucas greglucas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MegChai, I tried to test it out on a local project and ran into some issues with finding the proper version number from the URL (I think my local build is missing that path), so my dropdown selector was never getting populated. I also left a few other thoughts and comments here too. Overall, I think this is definitely heading in the right direction!

@@ -21,3 +21,8 @@ search_bar_position = sidebar
navigation_with_keys = True
show_toc_level = 1
navbar_align = content
use_version_switch = True

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this (and enable_locale) should probably default to False start as an opt-in in case other people are already rolling their own version switcher?

</li>
<li class="nav-item">
<span id="locale-switcher">
<!-- placeholder for locale switcher -->

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace with dropdown like above. I also liked your suggestion of putting the country flags in there, I'm not sure how hard it is to go from locale to UTF-8 flag code, but it would be a nice touch to add.

}
if (pathname.slice(-pageName.length) !== pageName) {
// Sphinx generated pages should have exactly same suffix
throw 'page suffix do not match requirements';

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to throw here (and below)? Or do, we want to just fail gracefully somehow. If any of the parsing failed, maybe remove the version switcher element altogether? Or fill it with a go-to-home button instead. I'm not sure what the best path is.

// This is base URL without any version or locate.
let globalBaseURL = parts.join('/')

return {pageName, baseURL, currentVersion, currentLocale, globalBaseURL};

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I was testing this locally, I was getting an empty string for currentVersion, which caused everything downstream to fail for me. I think this may be because the project is serving up the html build directory /index.html and not /en/v1.3/index.html. I'm not sure if this has to do with Sphinx options when I set up the project or not, but I think this will probably need to be robust against local builds for testing too where the URL may not be fully populated with locale and version.

@jorisvandenbossche
Copy link
Member

@MegChai thanks for the PR!

A few suggestions:

@cheekyshibe
Copy link
Contributor Author

This branch has been not maintained for a long time, @ThuWangzw will work on another branch.

He will open a new pull request as soon as possible.

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.

4 participants