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

Include jQuery locally in themes. #143

Closed
tsenart opened this issue Sep 19, 2014 · 12 comments
Closed

Include jQuery locally in themes. #143

tsenart opened this issue Sep 19, 2014 · 12 comments
Labels
Milestone

Comments

@tsenart
Copy link

@tsenart tsenart commented Sep 19, 2014

It seems that most themes are loading jQuery from https://code.jquery.com/jquery-1.10.2.min.js.
Sometimes this halts the loading of the whole page. Wouldn't it make sense to serve this resource ourselves instead?

@d0ugal
Copy link
Member

@d0ugal d0ugal commented Oct 1, 2014

This is an intersting one. I think we probably need to make it configurable if we want to change anything. I think the two requirements I see are:

  • Using a CDN is a sensible default - typically it should be faster unless you do hard refreshes.
  • Documentation should also work offline.

At the moment you can solve this with a custom theme, however, I can see that isn't ideal. I can't think of another solution at the moment.

@tomchristie
Copy link
Contributor

@tomchristie tomchristie commented Oct 1, 2014

I'd probably go with just including it locally. Imperfect but good-enough and doesn't require any extra config, documentation etc...

@d0ugal
Copy link
Member

@d0ugal d0ugal commented Oct 1, 2014

Ok, lets vendor jQuery :) Unfortunately I think it means we need to include it n times, where n is the number of themes.

@tsenart
Copy link
Author

@tsenart tsenart commented Oct 1, 2014

Can't you have a build step which copies vendored resources to each theme for each release artefact? That way you avoid duplication of code in the repository.

@d0ugal
Copy link
Member

@d0ugal d0ugal commented Oct 1, 2014

Yup, good idea - it just adds some complexity. How do we know if a theme needs jquery? What if they use different versions etc.

@tomchristie
Copy link
Contributor

@tomchristie tomchristie commented Oct 1, 2014

where n is the number of themes.

I think we should only be including the MkDocs and ReadTheDocs themes. The bootswatch stuff outghta get packaged seperately rly.

@tsenart
Copy link
Author

@tsenart tsenart commented Oct 1, 2014

Regardless of packaging the themes separately, each theme ought to define their dependencies in a consistent manner. What solution you use for that is up to you, but once you have it, it's easy to track what depends on what and satisfy those dependencies.

@tomchristie
Copy link
Contributor

@tomchristie tomchristie commented Oct 1, 2014

I would just avoid any notion of dependancies as much as possible, just include the theme wholesale. It's the simplest possible thing that works.

@tsenart
Copy link
Author

@tsenart tsenart commented Oct 1, 2014

I appreciate keeping things simple. Despite the fact you already have unmanaged dependencies, I'm sure once it becomes a hassle, you'll pick the right tool for the job.

@d0ugal d0ugal added the Enhancement label Oct 1, 2014
@d0ugal d0ugal modified the milestone: 0.10.0 Oct 7, 2014
@tomchristie tomchristie changed the title Slow jQuery load times Include jQuery locally in themes. Nov 7, 2014
@tomchristie
Copy link
Contributor

@tomchristie tomchristie commented Nov 7, 2014

Updating title with what I think is reasonable upshot of this.

@shvchk
Copy link

@shvchk shvchk commented Nov 12, 2014

One of the use cases is generating docs for completely isolated environment (i.e. with no internet access at all), so an option to have all resources locally (js, css, fonts, everything) would be really nice.

@tomchristie
Copy link
Contributor

@tomchristie tomchristie commented Nov 12, 2014

Yeah, agree that all the resources should be locally available.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

4 participants