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

Version warningbar #1354

Merged
merged 19 commits into from
Jul 19, 2023
Merged

Version warningbar #1354

merged 19 commits into from
Jul 19, 2023

Conversation

drammock
Copy link
Collaborator

@drammock drammock commented Jun 20, 2023

(supersedes) closes #1335
(supersedes) closes #780

This PR adds an optional specialized announcement banner to warn users who are viewing versions of the docs other than the latest stable release. It can coexist with the generic announcement banner (it gets stacked above it) and is styled similarly except that it has the "danger" background color.

  • requires new node dependency https://www.npmjs.com/package/compare-versions to compare version strings
  • relies on the presence of the version switcher JSON file to know what the latest stable release is --- specifically, a new version switcher key preferred=true is used
  • enabled in conf.py / html_context
  • message adapts depending on whether viewed docs are an old version / the dev version

crossref: scientific-python/summit-2023#10 (comment)

@drammock drammock marked this pull request as ready for review July 4, 2023 16:47
@drammock
Copy link
Collaborator Author

drammock commented Jul 4, 2023

all green! ready for review/merge. You can see the warning bar here: https://pydata-sphinx-theme--1354.org.readthedocs.build/en/1354/

cc @stefanv (sorry it's taken this long to get it finalized!)

@stefanv
Copy link

stefanv commented Jul 5, 2023

This is great, thanks @drammock!
I can't test this on the built pages, of course, but wanted to verify that there's also a warning on older versions? It seems that way from the PR description.

Copy link

@stefanv stefanv left a comment

Choose a reason for hiding this comment

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

The PR reads well, and the code is easy to follow; thanks!

docs/user_guide/announcements.rst Outdated Show resolved Hide resolved
@drammock
Copy link
Collaborator Author

drammock commented Jul 6, 2023

This is great, thanks @drammock! I can't test this on the built pages, of course, but wanted to verify that there's also a warning on older versions? It seems that way from the PR description.

There will be a warning on older versions that were built after the inclusion of this feature, and had the feature turned on at build time. Any older versions will not show the warning unless someone goes in and hard-codes one, or they already include something similar to the JS that this PR is based on (i.e. MNE-Python's version, sklearn's version).

I suppose we could add to the docs some template HTML that people could copy-paste as a one-time addition to older versions of their docs? Or a standalone JS script and the associated <script> tag? The resulting warning bar might be mostly devoid of styling (since we can't rely on the presence of certain classes) but might be better than nothing, WDYT?

@stefanv
Copy link

stefanv commented Jul 6, 2023

Yes, I think that's be a great addition!

@drammock
Copy link
Collaborator Author

adding a crossref here to @bryevdv's comments in #780 about what it would take for this to work with bokeh (key bit pullquoted below):

if Bokeh 4.3.7 is the latest release, then both of these links will work, and both should not have the warning banner:

https://docs.bokeh.org/en/4.3.7/
https://docs.bokeh.org/en/latest/

And I just don't see how that is feasible unless the page can compare its actual package version. I.e. compare "4.3.7" which is the value of DOCUMENTATION_OPTIONS.VERSION for both pages (and not "latest" since "latest" is not a real version) to some fixed, unambiguous declaration in the remote JSON like "latest": "4.3.7"

This is a note-to-self (or possibly note-to- @12rambau ) to double check that what we're doing here will work for Bokeh, before merging.

@drammock
Copy link
Collaborator Author

drammock commented Jul 18, 2023

After looking at Bokeh's current switcher.json file I'm fairly sure the implementation here should work fine for their use case (namely: two entries in switcher.json can have the same version value but different name and url values, and if both match the switcher_version_match value then neither one will get the warning banner). I tested this on a local build by hacking our conf.py to set release and version_match to both be 0.13.3 and adding a new entry to switcher.json have "version": "v0.13.3" but a different "name" property (and no "preferred" on that new entry). This worked: no version warning banner!

However it revealed an interesting/subtle bug which is that if multiple entries in switcher.json all match the version_match value from conf.py, then the switcher button itself will display the name value from the last matching entry in the JSON file. So in the case of Bokeh, regardless of whether someone were viewing the docs at the "latest" URL or at the "3.2.0" URL, the switcher button would always say "3.2.0" if the "name": "latest" entry came first in the JSON. I just pushed af158fe which fixes this by choosing which version to show on the button based on whether it's preferred or not, and if none of the matching entries are preferred, using the first match.

Below are the necessary changes to the Bokeh JSON to make it work optimally with this PR (not valid JSON because I put in comments to point out the new lines):

[
  {
    "name": "latest",
    "version": "3.2.0", // <--- NEW LINE
    "url": "https://docs.bokeh.org/en/latest/",
    "preferred": true // <--- NEW LINE
  },
  {
    "version": "3.2.0",
    "url": "https://docs.bokeh.org/en/3.2.0/"
  },
  {
    "version": "3.1.1",
    "url": "https://docs.bokeh.org/en/3.1.1/"
  },
  {
    "version": "3.0.3",
    "url": "https://docs.bokeh.org/en/3.0.3/"
  },
  {
    "version": "2.4.3",
    "url": "https://docs.bokeh.org/en/2.4.3/"
  },
  {
    "version": "2.3.3",
    "url": "https://docs.bokeh.org/en/2.3.3/"
  },
  {
    "version": "2.2.3",
    "url": "https://docs.bokeh.org/en/2.2.3/"
  },
  {
    "version": "2.1.1",
    "url": "https://docs.bokeh.org/en/2.1.1/"
  },
  {
    "name": "dev (3.3)",
    "version": "dev", // <--- NEW LINE
    "url": "https://docs.bokeh.org/en/dev-3.3/"
  }
]

@stefanv
Copy link

stefanv commented Jul 19, 2023

Nice catch, Dan! I hope this can be rolled out now.

Copy link
Collaborator

@12rambau 12rambau left a comment

Choose a reason for hiding this comment

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

I'm arriving after the war, sorry for the delay. I checked the code, nothing to see there @drammock and @stefanv, you did a very good job at coding and evaluating this modification. It's unfortunate that we cannot automatically backport the version number for version build prior from this modification but boh....

Not that I also like the design and it will continue to work with the "bak to top" widget (it listens to an empty pixel under the navbar). I think it should be merged as soon as possible and released so that it's included in new builds.

@drammock
Copy link
Collaborator Author

I think it should be merged as soon as possible and released so that it's included in new builds.

I'll take that as permission to press the green button. Now we just have a few block-release items to go... impact: block-release Should block a release from happening. Only use if this is a critical problem we don't want to ship

@drammock drammock merged commit 29fcd08 into pydata:main Jul 19, 2023
14 checks passed
@drammock drammock deleted the version-warningbar branch July 19, 2023 20:29
@choldgraf
Copy link
Collaborator

Third time's the charm! It is very satisfying to see this finally get in. Thanks all!

I think we are overdue for a release!

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