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

Using caption with different toc sections instead of JS/CSS hack. #1025

Merged
merged 1 commit into from
Aug 5, 2015

Conversation

dhermes
Copy link
Contributor

@dhermes dhermes commented Jul 30, 2015

Using caption with different toc sections instead of JS/CSS hack.

This fits within the Sphinx construct and would allow our docs to be built on readthedocs.org.


Building with tox -e docs we get:

screen_shot_014


Building with LOCAL_RTD=True tox -e docs we get:

screen_shot_015

AFAICT, by using :titlesonly: and some other rules we can get rid of the expandable sections (or just not use h1, or change maxdepth).

@dhermes dhermes added docs do not merge Indicates a pull request not ready for merge, due to either quality or timing. labels Jul 30, 2015
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jul 30, 2015
@dhermes
Copy link
Contributor Author

dhermes commented Jul 30, 2015

Note the dont-merge label. I just wanted to discuss the issues with either approach.

@dhermes
Copy link
Contributor Author

dhermes commented Jul 31, 2015

Previous comments that were addressed in later changes:


This fits within the Sphinx construct and would allow our docs to be built on readthedocs.org.

Unfortunately, this doesn't quite agree with our CSS (we need some left-hand padding):
screen_shot_011

I got this inspiration / technique from (source):
https://docs.readthedocs.org/en/latest/


As a test to see if our docs would build in the readthedocs theme, I swapped out html_theme for 'sphinx_rtd_theme' and turned off our custom html_style. The resulting output is a mess (I think our custom template is to blame):

screen_shot_012

@dhermes dhermes removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jul 31, 2015
@dhermes
Copy link
Contributor Author

dhermes commented Aug 4, 2015

@tseaver I fixed the failure (was really just #1029 showing) and rebased against master. The merging of #1026 makes this "ready to go" for activating RTD.

@dhermes
Copy link
Contributor Author

dhermes commented Aug 4, 2015

I jumped the gun on getting it up on RTD, looks pretty silly 😄
screen_shot_002

@tseaver and @jgeewax, what are your readthedocs.org usernames so I can add you as admins of the project?

@tseaver
Copy link
Contributor

tseaver commented Aug 4, 2015

Mine is tseaver.

This fits within the Sphinx construct and would allow our docs to
be built on readthedocs.org.
@dhermes
Copy link
Contributor Author

dhermes commented Aug 4, 2015

@tseaver Added you as a maintainer. Are you reviewing this?

@dhermes
Copy link
Contributor Author

dhermes commented Aug 4, 2015

@tseaver Any issues with these changes?

@tseaver
Copy link
Contributor

tseaver commented Aug 5, 2015

Sorry, I thought from the botched rendering in the last example it wasn't ready to go. LGTM.

dhermes added a commit that referenced this pull request Aug 5, 2015
Using caption with different toc sections instead of JS/CSS hack.
@dhermes dhermes merged commit 789508a into googleapis:master Aug 5, 2015
@dhermes dhermes deleted the use-sphinx-captions branch August 5, 2015 02:05
@dhermes
Copy link
Contributor Author

dhermes commented Aug 5, 2015

@tseaver Do you know much about RTD?

It's using the old Sphinx theme for some reason (though not locally via LOCAL_RTD=True tox -e docs):
http://gcloud-python.readthedocs.org/en/latest/

screen_shot_017

@dhermes
Copy link
Contributor Author

dhermes commented Aug 5, 2015

I think it might be my use of

if ON_READ_THE_DOCS or LOCAL_READ_THE_DOCS:
  templates_path = []
else:
  templates_path = ['_templates']

i.e. I think RTD uses _templates.

@dhermes
Copy link
Contributor Author

dhermes commented Aug 5, 2015

Scratch that, it was the use of html_theme = 'classic', which gets picked up by us here on RTD.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants