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 highlight.js from a CDN #21

Closed
mojavelinux opened this issue Apr 2, 2015 · 3 comments · Fixed by #320
Closed

Load highlight.js from a CDN #21

mojavelinux opened this issue Apr 2, 2015 · 3 comments · Fixed by #320

Comments

@mojavelinux
Copy link
Member

Load highlight.js from a CDN like we do with the HTML5 output instead of using the bundled version. Of course, we can still recognize the property of highlightjs dir so that it's possible to use the bundled version, but we should be consistent with the HTML5 output by default.

@ggrossetie
Copy link
Member

I agree the version in the reveal.js plugin is outdated, see: hakimel/reveal.js#2562

We should use a copy of https://github.com/hakimel/reveal.js/blob/master/plugin/highlight/highlight.js without highlight.js.

@obilodeau What do you think? The downside is that we will need to sync this file when we upgrade reveal.js.

@ggrossetie
Copy link
Member

Not sure about the CDN part, considering that we are using a local copy of reveal.js but otherwise 👍

ggrossetie added a commit to ggrossetie/asciidoctor-reveal.js that referenced this issue Jan 22, 2020
ggrossetie added a commit to ggrossetie/asciidoctor-reveal.js that referenced this issue Jan 22, 2020
@obilodeau
Copy link
Member

obilodeau commented Jan 23, 2020

I was never particularly affected by this so don't have a big incentive for this. That said, I don't mind merging it. Just make sure changes to the release process are documented. Like looking at upstream (both reveal.js' integration and highlight.js) and merging back before releases, etc.

Edit: my previous wording wrongly reflected that I didn't really care

ggrossetie added a commit to ggrossetie/asciidoctor-reveal.js that referenced this issue Feb 8, 2020
ggrossetie added a commit to ggrossetie/asciidoctor-reveal.js that referenced this issue Feb 8, 2020
ggrossetie added a commit to ggrossetie/asciidoctor-reveal.js that referenced this issue Feb 8, 2020
ggrossetie added a commit to ggrossetie/asciidoctor-reveal.js that referenced this issue Feb 8, 2020
ggrossetie added a commit to ggrossetie/asciidoctor-reveal.js that referenced this issue Feb 9, 2020
ggrossetie added a commit to ggrossetie/asciidoctor-reveal.js that referenced this issue Feb 9, 2020
obilodeau pushed a commit that referenced this issue Feb 9, 2020
* Allows us to load it from a CDN (resolves #21)
* Rely on a smaller core and allow the load of additional languages (a breaking change)
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 a pull request may close this issue.

3 participants