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

Load scripts before plugins that might use them. #83

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mtstickney
Copy link

@mtstickney mtstickney commented Jan 24, 2016

Body elements are loaded in the order that they are seen; by loading
at the end of the body, they are not available for content that needs them,
like the github plugin in the sidebar. For the github plugin, this results
in a '$ is not defined' error and the repo list is never updated. Moving
the scripts to ensures the scripts are loaded before dependent
content.

Update: note that this fixes #79.

@duilio
Copy link
Owner

duilio commented Jun 5, 2016

shouldn't the plugin just wait the dom to be ready in order to have all deps loaded correctly?

Body elements are loaded in the order that they are seen; by loading
at the end of the body, they are not available for content that needs them,
like the github plugin in the sidebar. For the github plugin, this results
in a '$ is not defined' error and the repo list is never updated. Moving
the scripts to <head> ensures the scripts are loaded before dependent
content.
@mtstickney
Copy link
Author

Not in this case. The github script is already using $.domReady(), but $ hasn't been defined yet, so the handler doesn't even get registered. You'd need to load that script before the plugins in any case (I think that's uh... ender.js, maybe?).

At the very least ender.js should be moved into <head>; moderizr and octopress.js might not technically need it, but it's safe to move them as well, and I don't think there's much downside.

@atodorov
Copy link
Contributor

atodorov commented Jun 5, 2016

As far as I can tell the original Octopress theme has these scripts in the section.

@mtstickney
Copy link
Author

I think you're mistaken: _include/head.html has modernizr, octopress.js, and ender.js all loaded in <head> (that's the 2.0 tag, the 2.5 branch is the same except they're using jquery instead of ender.js). It's also pretty easy to verify empirically: move the scripts into <head> or above the sidebar include and the plugin works; don't, and it doesn't.

@atodorov
Copy link
Contributor

atodorov commented Jun 5, 2016

@mtstickney I meant <head> but wiki escaping failed me :(

@mtstickney
Copy link
Author

I see, apologies for misunderstanding (still probably good to have the reference, though).

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.

Regression: github projects sidebar
3 participants