Skip to content
This repository has been archived by the owner on Aug 26, 2022. It is now read-only.

Bug 950945: Fix and improve MDN Sphinx template #3954

Merged
merged 8 commits into from
Sep 9, 2016

Conversation

jwhitlock
Copy link
Contributor

@jwhitlock jwhitlock commented Aug 30, 2016

  • Fix the ./manage.py generate_sphinx_template command
  • Update the template to match the current MDN JS and CSS
  • Omit some more items from the Sphinx template, such as social graph and Google Analytics
  • Add generating the documentation to the documentation
  • Update the docs to use the new version of the theme.

This PR is used to generate the changes in mdn/sphinx-theme#10, which is necessary to release mdn-sphinx-theme version 2016.0 and fix some issues with the current documentation.

@stephaniehobson I'd appreciate a code review, especially of base.html but I don't expect you to generate the docs locally unless you are looking to level up in backend development.

Fix errors when running ./manage.py generate_sphinx_template to generate
the Sphinx template for the MDN theme at:

https://github.com/mdn/sphinx-theme

In addition, fix some issues in the template:

* Single-escape the title rather than double-escape
* Copyright always covers current year
* Add an empty <div class="page-buttons"> so wiki.js runs successfully
* Add class="text-content" so documentation content is styled correctly
* IE shims for old versions have the correct path
Omit the badges from the documentation, and preserve linebreaks in the
quick links table.
Omit social tags, search nav, and Google Analytics initialization in the
MDN Sphinx template.
@jwhitlock
Copy link
Contributor Author

Force-push to remove the upgrade to unreleased mdn-sphinx-theme 2016.0. One thing at a time...

@codecov-io
Copy link

codecov-io commented Aug 30, 2016

Current coverage is 86.20% (diff: 100%)

Merging #3954 into master will increase coverage by 0.12%

@@             master      #3954   diff @@
==========================================
  Files           146        145     -1   
  Lines          8912       8841    -71   
  Methods           0          0          
  Messages          0          0          
  Branches       1195       1188     -7   
==========================================
- Hits           7671       7621    -50   
+ Misses         1002        980    -22   
- Partials        239        240     +1   

Powered by Codecov. Last update 4c2d362...46e55b9

@@ -233,11 +236,11 @@

{{ providers_media_js() }}
{% javascript 'main' %}

{% if not is_sphinx %}
Copy link
Contributor

Choose a reason for hiding this comment

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

We're tracking everything else on the docs, we might as well track the links.

Copy link
Contributor

Choose a reason for hiding this comment

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

Pointed out on IRC that we include analytics.js but not ga.js so removing this makes sense.

@stephaniehobson
Copy link
Contributor

Left some comments on mdn/sphinx-theme#10

Sync the version of Jinja2 in docs.txt with default.txt.

docs.txt contains the requirements for building the documentation, which
is normally not done in the development environment, but having the same
versions will mean that it is safe to install them alongside other
development requirements.
Documentation can be built in the VM, or experienced Python developers can
create a new virtualenv to install the requirements and build the
documentation, in a similar way to ReadTheDocs.
The <!DOCTYPE> tag is in the Jinja2 rendered output, but is not parsed
as a token by html5lib. The html5lib docs imply that the "dom"
treebuilder might parse this, but the default "etree" one does not. In
any case, add it as the first element.
@stephaniehobson
Copy link
Contributor

Comments the other repo were addressed 👍

@stephaniehobson stephaniehobson merged commit 609f6e5 into master Sep 9, 2016
@jwhitlock jwhitlock deleted the sphinx_template_950945 branch September 24, 2016 19:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants