8000 Update layout.html for sphinx themes by alexrudy · Pull Request #14893 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]

Skip to content

Update layout.html for sphinx themes #14893

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

Merged
merged 4 commits into from
Aug 24, 2019
Merged

Conversation

alexrudy
Copy link
Contributor
@alexrudy alexrudy commented Jul 26, 2019

PR Summary

Matplotlib had a copy of the base sphinx theme layout.html – but this file was slowly getting out of sync with Sphinx.

It turns out that Sphinx (and the default theme, alabaster) have added a lot of theming customization hooks, and it is possible to make the docs almost identical without carrying around an entire copy of layout.html. This allows matplotlib to take advantage of the maintenance done on layout.html by Sphinx upstream, without having to merge those changes occasionally into matplotlib itself.

Here are some examples of the difference between generating the docs before and now:

  • A <meta viewport=... tag has been added.
  • A reference to custom.css is now emitted (this is harmless, but is the now supported way to provide CSS overrides to alabaster, the default sphinx theme).
  • Additional HTML attributes have been added in some places where appropriate (e.g. role=, which should improve screen reader accessibility).
  • The footer is now wrapped in a <footer> <footer /> tag.

As far as I can tell, there are no material differences to the way the docs look, but I would appreciate more eyes/further feedback.

PR Checklist

This isn't really a python change, so no tests are provided, and it shouldn't be user-facing (docs should look the same after this change) so I don't think it needs a "what's new" or "API changelog".

@QuLogic
Copy link
Member
QuLogic commented Jul 26, 2019

Does this require a new version of Sphinx? If so, then that warrants a changelog entry.

@alexrudy
Copy link
Contributor Author

I don't think this requires a change in the sphinx version we use, but I did test it with 1.8.5 and I can test it with 1.8.1 (the minimum version we support) to be sure.

@timhoffm
Copy link
Member

@alexrudy
Copy link
Contributor Author

Thanks for the catch, @timhoffm – Indeed, the change had dropped a <div class="footer"> from the template, I've put that back in.

@alexrudy
Copy link
Contributor Author

Note that this doesn't fix the issues in #13591 – that will have to come separately for full compatibility with Sphinx>=2 - but I'm thinking of trying to tackle that next.

@alexrudy
Copy link
Contributor Author

Also, I checked that this works just fine with sphinx==1.8.1 and sphinx==1.8.5 so I don't think we need to change version bounds yet.

<a href="http://sphinx-doc.org/">Sphinx</a> {{ sphinx_version }}.{% endtrans %}
{%- endif %}
{%- if sha %}
Doc version {{ sha }}.
Copy link
Member

Choose a reason for hiding this comment

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

I think, this should stay as well.

Copy link
Member

Choose a reason for hiding this comment

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

We need this because we build the docs from a different branch than the release.

<a href="http://sphinx-doc.org/">Sphinx</a> {{ sphinx_version }}.{% endtrans %}
{%- endif %}
{%- if sha %}
Doc version {{ sha }}.
Copy link
Member

Choose a reason for hiding this comment

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

We need this because we build the docs from a different branch than the release.

@tacaswell tacaswell added this to the v3.2.0 milestone Aug 12, 2019
Copy link
Member
@timhoffm timhoffm left a comment

Choose a reason for hiding this comment

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

Looks correct now.

@timhoffm timhoffm dismissed tacaswell’s stale review August 22, 2019 20:44

SHA entry is now kept.

@tacaswell tacaswell merged commit 9122a45 into matplotlib:master Aug 24, 2019
@tacaswell
Copy link
Member

Thanks!

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.

5 participants
0