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

Move JS to bottom of page #78

Merged
merged 12 commits into from
Mar 2, 2014
Merged

Move JS to bottom of page #78

merged 12 commits into from
Mar 2, 2014

Conversation

ehough
Copy link
Contributor

@ehough ehough commented Jan 5, 2014

Thought I'd try moving all the JS to the bottom of the page for performance purposes. Work fine in my testing. Curious to hear your feedback.

Thanks for this project!

@snide
Copy link
Collaborator

snide commented Jan 5, 2014

Thanks for the sub.

@ehough I'd rather not change the block names since they are the standard for Sphinx templates and as much as I don't like the way they have stuff setup, seems prudent to stick with convention.

@ericholscher Dunno how moving the JS to the bottom effects your loading in RTD. As I remember it, I tried something like this before, but your overwrites once it moved to RTD caused problems.

The JS being loaded is pretty minimal, so the gain isn't huge if that was/is the case.

@ericholscher
Copy link
Member

I don't remember exactly what broke. I believe that as long as you keep the
extrahead block up top, it should work on RTD. I imagine it was just stuff
we were trying to inject into the page. Though I think we assume jquery
will be loaded, so that would probably break as well, and if you're loading
jquery in the top, then everything else is likely trivial size-wise. So, I
think thats why we ended up where we are.

On Sun, Jan 5, 2014 at 5:04 PM, Dave Snider notifications@github.comwrote:

Thanks for the sub.

@ehough https://github.com/ehough I'd rather not change the block names
since they are the standard for Sphinx templates and as much as I don't
like the way they have stuff setup, seems prudent to stick with convention.

@ericholscher https://github.com/ericholscher Dunno how moving the JS
to the bottom effects your loading in RTD. As I remember it, I tried
something like this before, but your overwrites once it moved to RTD caused
problems.

The JS being loaded is pretty minimal, so the gain isn't huge if that
was/is the case.


Reply to this email directly or view it on GitHubhttps://github.com//pull/78#issuecomment-31617093
.

Eric Holscher
Maker of the internet residing in Portland, Or
http://ericholscher.com

@ehough
Copy link
Contributor Author

ehough commented Jan 6, 2014

FWIW I don't see any jQuery-dependent JS in rtfd/readthedocs.org that is not addressed by this PR.

@ehough
Copy link
Contributor Author

ehough commented Jan 14, 2014

You can see a demo of this now at http://tubepress.readthedocs.org/en/latest/. Feel free to test at your convenience!

(it also includes #69)

@ericholscher
Copy link
Member

Seems to work fine. I'm good with merging it.

@ericholscher
Copy link
Member

Though we need to keep the block names to keep parity with Sphinx.

@ehough
Copy link
Contributor Author

ehough commented Feb 28, 2014

Thought I'd check in to see if we can get this PR merged.

Though we need to keep the block names to keep parity with Sphinx.

I addressed this in e16b8a8 by re-using Sphinx's footer block.

As a reminder, you can view a demo of this PR at http://docs.tubepress.com/

@snide
Copy link
Collaborator

snide commented Feb 28, 2014

Sorry, I've been busy and forgot about this one. I'll check this out now.

snide added a commit that referenced this pull request Mar 2, 2014
@snide snide merged commit 34e56c8 into readthedocs:master Mar 2, 2014
@snide
Copy link
Collaborator

snide commented Mar 2, 2014

K, checked out. Apologize for the long wait. Back in startup land so I'm back to hacking on the weekends with this.

@ehough ehough deleted the move-javascript-to-bottom branch March 4, 2014 05:02
mgeier added a commit to mgeier/sphinx_rtd_theme that referenced this pull request Nov 4, 2018
It was moved away from <head> to the bottom of the page in readthedocs#78.

This is a compressed version of readthedocs#545 by @Blendify, which fixes readthedocs#328.
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.

3 participants