-
Notifications
You must be signed in to change notification settings - Fork 188
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
Reveal.js 3.8.0 to 3.9.2 support #301
Conversation
9740588
to
d7a139c
Compare
Also, highlightjs |
Tonight's work:
|
@obilodeau How can I help with this pull request? If you have specific tasks, please tell me. Hopefully I've fixed the docinfo issue in #324 |
ccc94e2
to
af80238
Compare
Rebased on top of #324. |
Highlight.js is feature parity with 3.8.0. I'm going to deal with 3.8.0 to Next-up: go through all configuration options and see if I'm missing anything |
It currently fails rendering
Asciidoctor-templates-compiler 0.6.0 workarounds the issue by using a fork of ruby-beautify
Including default template change
Dropped a lot of old cruft that accumulated
Co-Authored-By: Guillaume Grossetie <[email protected]>
Pushed doctests |
Lazy loading is not supported at the moment and would require a lot of testing and changes. I have video-heavy presentations and never felt it was required. Also, it can be implemented later without causing a breaking change. Let's not implement it now. |
This is ready to go in. I have a big presentation coming up so I would like to release this relatively shortly (in about a week max). @Mogztter if you want #320 to get in for 4.0.0, I would advise you to get it in shape quickly. I would prefer it if it does get in. Plus there is a lot of doctest around highlightjs so I feel confident. @mojavelinux there's an API question that you might want to look at in this review Lastly, @Mogztter since you asked how you can help, there's #326 that I would like to get done soon. I'm not very good at maintainable CSS so if you can give it a try I would appreciate it. |
Speaking of #326, I need to do some vertical alignment and as far as I know the only way to do it reliably is to wrap the content of a slide in a |
I had the feeling that it was achievable without doing so. Isn't it working
for the other example I provided in the issue?
|
I don't want to pollute the pull request about the reveal.js upgrade, so here's my reply: #326 (comment) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, good job 👍
I left some nitpicks but otherwise 🚀
Alright, I will work on it tomorrow 😉 |
Fixed whitespace |
…stitution Also wrapped up support for step-by-step highlights on top and updated tests
Draft commit for reveal.js 3.8.0 support.
This work is postponed to have a very stable 3.0.0 that will support reveal.js versions spanning from 3.0 to 3.7. See #296.
Needs work:
<mark>
syntax described here does not workI'm not sure per-slide iframe preloading options are supportedhash
feature w/ and w/o history and see if direct links still works