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

Name in the version switcher dropdown button (not expanded) #1500

Closed
AlenkaF opened this issue Oct 9, 2023 · 9 comments
Closed

Name in the version switcher dropdown button (not expanded) #1500

AlenkaF opened this issue Oct 9, 2023 · 9 comments
Labels
kind: bug Something isn't working tag: javascript Pull requests that update Javascript code tag: UX Issues or improvements related to user experience

Comments

@AlenkaF
Copy link
Contributor

AlenkaF commented Oct 9, 2023

In Arrow dev branch we are using the latest version of the theme and we are fine tuning our settings before the new release in the coming week or two. I am struggling to find where to customize the name of the (not expanded) version switcher dropdown button and would be grateful to get your feedback if this is expected behaviour or I am missing something in our settings.

When I load the development version of the Arrow docs the text for the version switcher dropdown button shows "Choose version" but would like to change that to show which version I currently have open. In this case it should show "14.0 (dev)" which is the "name" item in the https://github.com/apache/arrow/blob/main/docs/source/_static/versions.json.

Screenshot 2023-10-09 at 09 59 36

I see that the text is set in the version switcher component

Choose version <!-- this text may get changed later by javascript -->

but I can't find where it is changed later by javascript so I can take use of it.

@12rambau
Copy link
Collaborator

12rambau commented Oct 9, 2023

Normally this text should be changed according to the names set up in the switcher.json file. It was working just fine but since a month we are facing JS issues from our side as well: #1416.

Unfortunately we have a hard time understanding what the problem is as we don't see the issue in PR build, preventing us from testing it.
In short if you are a JS expert and have the time to review our search it would be fantastic, if not maybe wait for next release.

@AlenkaF
Copy link
Contributor Author

AlenkaF commented Oct 9, 2023

Thank you for the quick response!
No, not a JS expert unfortunately :(

Happy to wait 👍

@amoeba
Copy link

amoeba commented Oct 12, 2023

Hi @AlenkaF, @12rambau: It looks like the issue has to do with this bit of logic: https://github.com/pydata/pydata-sphinx-theme/blob/main/src/pydata_sphinx_theme/assets/scripts/pydata-sphinx-theme.js#L375-L376. And it interacts with whatever ultimately gets set in the version variable in https://github.com/apache/arrow/blob/main/docs/source/conf.py#L212 and what's in the version property in versions.json.

Locally, I was able to fix the Arrow dev docs by setting ARROW_DOCS_VERSION="dev/" instead of it's usual value which is something like 14.0.0.dev298". It seems to me like whatever that's set to always needs to match a value in the version property in version.json for this to work reliably. Is the intent that the version property is sort of serving both as a version string and also as a URL path element?

I'm not totally sure what the change might be for this repo but I thought I'd share the above since I was able to reproduce the issue and fix it locally.

@AlenkaF
Copy link
Contributor Author

AlenkaF commented Oct 12, 2023

Thank you @amoeba for the help!

You are correct. The version supplied to the version_match of the switcher should be equal to the version specified in the versions.json and then the code uses the text specified in versions.json under the "name" item.

In Arrow case the versions do not match ("14.0.0.dev298" vs "dev/") and so we have to change the version we are passing to the switcher. It should be "dev/" for development build and "" (empty) for the build of the stable version of the docs.

I have a PR open to test this. If it works, this issue can be closed.

@drammock
Copy link
Collaborator

@amoeba @AlenkaF see what we do in our own docs:

# Define the version we use for matching in the version switcher.
version_match = os.environ.get("READTHEDOCS_VERSION")
# If READTHEDOCS_VERSION doesn't exist, we're not on RTD
# If it is an integer, we're in a PR build and the version isn't correct.
# If it's "latest" → change to "dev" (that's what we want the switcher to call it)
if not version_match or version_match.isdigit() or version_match == "latest":
# For local development, infer the version to match from the package.
release = pydata_sphinx_theme.__version__
if "dev" in release or "rc" in release:
version_match = "dev"

so yeah, generally speaking you may need to do a bit of gymnastics in your conf.py to get version_match to be the value you expect to match from the .json file.

@jorisvandenbossche
Copy link
Member

jorisvandenbossche commented Oct 12, 2023

But so this gymnastics is currently also wrong for pydata-sphinx-theme, which is the reason that our stable docs here (https://pydata-sphinx-theme.readthedocs.io/en/stable/index.html) have the same issue:

image

The above quoted logic turns the version match into "stable" or "latest" on read the docs, but our json file doesn't have those entries as "version" key:

[
{
"version": "dev",
"url": "https://pydata-sphinx-theme.readthedocs.io/en/latest/"
},
{
"name": "0.14.1 (stable)",
"version": "v0.14.1",
"url": "https://pydata-sphinx-theme.readthedocs.io/en/stable/",
"preferred": true
},

So for "latest" version, we change that to "dev" in conf.py so that matches, but for the "stable" version, that doesn't match "v0.14.1" in the json file.

@jorisvandenbossche
Copy link
Member

I don't know if we can change the versions.json to use "stable" instead (I think at some point in the past, this "version" key was used differently, and if we want to keep the version switcher working for older versions, this might need to stay the same?).
But alternatively, we can change version_match to __version__ in conf.py

@12rambau 12rambau added kind: bug Something isn't working tag: javascript Pull requests that update Javascript code tag: UX Issues or improvements related to user experience labels Oct 13, 2023
@drammock
Copy link
Collaborator

Thank you @jorisvandenbossche for connecting the dots for me... since I don't use RTD to host any of my own projects, and don't have admin access to RTD account for this theme, I never realized that RTD is setting "stable" as the READTHEDOCS_VERSION variable.

alternatively, we can change version_match to __version__ in conf.py

I think this is the right way forward, I'll open a PR.

@drammock
Copy link
Collaborator

fixed by #1512 (for our docs anyway). For downstream user's docs, they'll still need to maybe do some things in conf.py to end up with a version match string that will successfully match what is in their versions.json file

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug Something isn't working tag: javascript Pull requests that update Javascript code tag: UX Issues or improvements related to user experience
Projects
None yet
Development

No branches or pull requests

5 participants