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

DOC: Restore banner indicating docs are unreleased #26219

Merged
merged 1 commit into from
Jul 16, 2023

Conversation

QuLogic
Copy link
Member

@QuLogic QuLogic commented Jun 29, 2023

PR summary

This was dropped when switching to the pydata-sphinx-theme. They have a method of adding an announcement, but because it's a pure text substitution, we can't have it automatically search like our old banner.

While the theme also supports loading from an http resource, that would mean writing some JavaScript instead of automatically creating this div at build time.

So override the theme component, and create the search link at build time.

I'm only a bit annoyed that the announcement overlays the normal navigation bar. I'm not sure it's really possible to avoid that without wrapping both in their own sticky div, but that would require overriding the entire theme layout, and that's a bit too much for my taste.

PR checklist

@QuLogic QuLogic added this to the v3.8.0 milestone Jun 29, 2023
@QuLogic QuLogic force-pushed the restore-unreleased-banner branch from 45c932f to 4eed16f Compare June 29, 2023 06:29
@QuLogic
Copy link
Member Author

QuLogic commented Jun 29, 2023

I'm only a bit annoyed that the announcement overlays the normal navigation bar. I'm not sure it's really possible to avoid that without wrapping both in their own sticky div, but that would require overriding the entire theme layout, and that's a bit too much for my taste.

For this bit, I've added in a shift to the normal navigation bar based on its minimum height from CSS. This won't work everywhere (i.e., if text is wrapped), but is a bit better than nothing.

@melissawm
Copy link
Member

There is an un-merged implementation of this in the pydata-sphinx-theme: pydata/pydata-sphinx-theme#1335

I have actually copy-pasted a previous solution also from the pydata-sphinx-theme here: napari/napari-sphinx-theme#132

Ultimately the napari team is going in another direction but I just wanted to point out existing implementations in case this helps.

@QuLogic QuLogic force-pushed the restore-unreleased-banner branch from 4eed16f to 999e291 Compare June 29, 2023 19:35
@QuLogic
Copy link
Member Author

QuLogic commented Jun 29, 2023

I was aware of the pydata-sphinx-theme PR; I'm hoping we can migrate to it once it's been released. I think we won't have a reason to put an announcement on the devdocs, so I think it's okay to steal that space to make the warning.

Anyway, we already have people confused about unreleased features, so it's probably best to restore this sooner rather than later.

@QuLogic QuLogic marked this pull request as ready for review June 29, 2023 23:34
@@ -0,0 +1,11 @@
{% set header_classes = ["bd-header-announcement", "container-fluid"] %}
{%- if '+' in release %}
Copy link
Member

@ksunden ksunden Jul 5, 2023

Choose a reason for hiding this comment

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

Do we have access to all variables from conf.py here? if so would it be better to do is_release_build here?

is_release_build = tags.has('release') # noqa

@QuLogic
Copy link
Member Author

QuLogic commented Jul 5, 2023

@tacaswell also noted that we may want to do announcements on releases, and this doesn't quite allow that, but I think I can make a small tweak to fix that.

This was dropped when switching to the pydata-sphinx-theme. They have a
method of adding an announcement, but because it's a pure text
substitution, we can't have it automatically search like our old banner.

While the theme also supports loading from an http resource, that would
mean writing some JavaScript instead of automatically creating this div
at build time.

So override the theme component, and create the search link at build
time.
@QuLogic QuLogic force-pushed the restore-unreleased-banner branch from 999e291 to df5d59a Compare July 6, 2023 00:06
@QuLogic
Copy link
Member Author

QuLogic commented Jul 6, 2023

OK, should work in all 3 cases: release build, unreleased build, and an announcement.

@greglucas greglucas merged commit c0d9c98 into matplotlib:main Jul 16, 2023
@QuLogic QuLogic deleted the restore-unreleased-banner branch July 17, 2023 17:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants