bradyvercher / gistpress

WordPress plugin to add Gist oEmbed and shortcode support with caching.
GNU General Public License v2.0
143 stars 28 forks source link

Cache the style sheet #22

Open bradyvercher opened 11 years ago

bradyvercher commented 11 years ago

The cache for Gist HTML mostly removes the dependency on loading resources from GitHub, but the style sheet is still an external request. Ideally, it should be cached as well.

GaryJones commented 11 years ago

Would that not require adding it to the output as an internal style sheet? As long as it's only done as a fallback of some sort, then it's fine. But first choice should be an external style sheet, which the browser has likely already cached, even by the visitor having visited a gist on gists.giththub.com on previously.

bradyvercher commented 11 years ago

Yeah, it'd require serving it internally. My concern was eliminating the dependence on the GitHub servers if they are unavailable, but like you mentioned, that would eliminate the benefits of the browser cache for the external style sheet. In that case, I'm inclined to stick with what we have with one minor change:

The style sheet reference option should probably be primed on activation so it's available the first time a shortcode is used.

GaryJones commented 11 years ago

Yes, I came across a situation where it was unprimed yesterday. Question is - priming it with what sensible fallback?

bradyvercher commented 11 years ago

I'm not sure there is a sensible fallback route if it fails initially. It'd be easy to test if the option is empty any any point, so it could be done on requests to the edit post screen, but that could create lag if GitHub isn't reachable.

Maybe display a notice if the initial request fails with an note that it will be updated automatically when available?

GaryJones commented 11 years ago

I think that the chance of Github being down, just at the time that the first embedded Gist is being viewed on a site, is going to be an edge case, and I'd be happy to wait for an influx of issue reports before bulking out the plugin with an edge case solution.

Anyway - if it can't prime the style sheet value, then it won't also be able to prime the transient and post meta with HTML, so there won't really be anything to style.

bradyvercher commented 11 years ago

Good points. So we'll just attempt to prime on activation and not worry about the edge case.

GaryJones commented 11 years ago

Prime on plugin activation? With a call to which Gist? (presumably it's the same stylesheet value for each and every gist JSON.) Obviously one of your own that isn't going to be deleted any time soon would make sense.

bradyvercher commented 11 years ago

I have a couple of private Gists for testing, but it'd be ideal to set up a better one to use explicitly for the readme examples and it could double for this case.