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

Make Reveal more bundler-friendly #2524

Open
quilicicf opened this issue Nov 6, 2019 · 7 comments
Open

Make Reveal more bundler-friendly #2524

quilicicf opened this issue Nov 6, 2019 · 7 comments

Comments

@quilicicf
Copy link
Contributor

quilicicf commented Nov 6, 2019

What am I trying to do

I'm trying to use a bundler (parceljs in my case) to generate a self-sufficient HTML file containing a Reveal.js presentation.

This would have a lot of pros:

  • generate a presentation in a universal format (literally opened with a double-click on all OSes)
  • the presentation would be tiny (the bundler can minify, compress etc...) and even tinier if zipped
  • it would be easy to handle and pass around in an email or other means because it would be in a single file

How is my presentation structured

Reveal.js is installed in my project via npm.

I have an index.html file containing my slides that imports a JS file.

The JS file imports the CSS, plugins etc... and initializes Reveal.js.

I can provide some code if that helps.

The issues I am facing

Reveal.js is not currently bundler-friendly for a couple of reasons described below.

Plugins

Reveal.js assumes the plugins are served over HTTP and it loads them from their source by injecting a link element with src='relativeUrlSubPathToPlugin'.

This can't work if my presentation is in a single HTML file.

In my case, given I use a bundler, I'd expect to retrieve the plugin (the object with function init) via an ESM import and directly provide it to Reveal.js.

I have tried to hack around the injection process with no avail.

PDF printing

I would like to (ideally) embed both paper.css and pdf.css in my presentation so that people can print it the way they want once bundled.

They overlap if both included in the HTML file (and wreck the presentation).

This means that I have one of two solutions (at least that come to mind):

  • either ask the parceljs team to merge a PR that would allow to retrieve the content of a CSS file in a variable (like: import pdfCss from 'reveal.js/print/pdf.css') and inject it (conditionally) myself in the HTML, depending on the query parameter ?print-pdf
  • or make a change in Reveal.js that would allow both style sheets to be imported in the HTML and only applied if a specific class is added to body for example. Which would be done depending on the query parameter

Does that make sense?

I'd be happy to help with any implementation if you are convinced this would bring value to the project (and I hope it does).

Sorry for the big issue

@quilicicf
Copy link
Contributor Author

This is a PR show-casing one solution to the problems above ☝️

@quilicicf
Copy link
Contributor Author

There is still the issue of the speaker notes to tackle but it's a good start IMO.

@quilicicf
Copy link
Contributor Author

Work has started in PRs #2627 and #2633.

Stay tuned.

@quilicicf
Copy link
Contributor Author

So, last part of the plan is about plugins.

To make reveal.js fully bundler-friendly we need all the core plugins to be injectable with the new system implemented in PR 2627.

For most plugins, this is going to be super easy bu the speaker notes plugin is gonna be harder so let's focus on this one.

What needs to change

The speaker notes need to be loaded from the main page (on the same URL as the main presentation) if we want to bundle the whole presentation in a single file.

This means that the URL needs to specify which reveal presentation to connect to and that the presentation should open in notes mode. I'm thinking something like: http://localhost:8080/my_presentation.html?speaker-notes-for=${presentationId}.

When the presentation is open in notes mode, it should inject the relevant HTML/CSS and initiate the connection to the original presentation.

Another pro of this solution is that the slides are by default in the notes presentation, only some HTML injection and theming need to be done to show the speaker notes.

What else can be improved

I think some other improvements could be gained with such a solution:

  • Theming: users get access to the CSS of the speaker notes, allowing them to tweak the CSS which is currently not possible
  • Configuration: the configuration of the notes (layout for example) can be loaded directly from the Reveal.initialize
  • Timing preservation: one thing that can happen with the current implementation of the speaker notes is that the presenter accidentally closes the notes (happened to me once). Currently, the timing is only kept by the notes pop-up. This means closing it accidentally loses the timing information. I think we can cheaply store the timing info in local storage (every 10s maybe?) and avoid this by reloading it when re-opening the speaker notes

What's next

I think these modifications might prove pretty tough though and I don't want to start implementing something that has no chance of getting merged so I need your opinion on this @hakimel.

WDYT?

@hakimel
Copy link
Owner

hakimel commented Apr 15, 2020

I started working on plugin bundling a few days ago. The new recommended way to include plugins will be to use ESM, for example:

import Reveal from '/js/reveal.js';
import Zoom from '/plugin/zoom/zoom.js';
import Notes from '/plugin/notes/notes.js';
import Search from '/plugin/search/search.js';
import Markdown from '/plugin/markdown/markdown.js';
import Highlight from '/plugin/highlight/highlight.js';

let deck = new Reveal( document.querySelector( '.reveal' ), {
	controls: true,
	progress: true,

	dependencies: [ Zoom, Notes, Search, Markdown, Highlight ]
});

deck.initialize();

I still want to support IE11 too, so I'm transpiling es5 versions of each plugin that can be included using the old dependencies syntax if browser support is important:

dependencies: [
	{ src: 'dist/plugin/markdown.js' },
	{ src: 'dist/plugin/highlight.js' },
	{ src: 'dist/plugin/notes.js' }
]

A lot of things are still changing but once this is done I'll take a closer look at your proposal for how we can best associate the notes window.

@quilicicf
Copy link
Contributor Author

Awesome thanks!
I'll let you take a look then.
In the meantime I'd like to propose a new improvement which I can submit a PR for.

The main issue I'm facing with Reveal right now is that the UX for bootstrapping a new presentation is not exactly straight-forward. I always end up copy-pasting the last one and resetting it.

I'd like to propose a CLI tool for bootstrapping a new presentation that would allow the user to select the plugins/themes they want to install and generate the index.html + app.js + package.json with all required dependencies.

I think the best place for such a tool is this repository because I'd be able to list the core plugins/themes from the source and it would be easier to keep the CLI compatible with potential changes in here.

WDYT?

@quilicicf
Copy link
Contributor Author

Hi again!

I've played with the new imports and tried a lot of approaches to try and get a single, self-sufficient, HTML file at the end of my build and I struggle with the Notes plugin.

The problem

Trying to inject all the JS code of the presentation in a script tag results in tripping the HTML parser when the Notes plugin is used.

Basically, the plugin does something like this:

speakerNotesWindow.document.write('<html>...<script>...</script>...</html>');

The problem is that the HTML parser doesn't understand that the closing script tag here is in a JS string and it closes the parent script tag which completely breaks the page 😱

An illustration of the problem below. Note that the closing script tag in the JS string is colored as valid HTML code because the HTML parser is tripped.

<html>
  <script type="module">
    document.write('<script>console.log("whatever");</script>');
  </script>
</html>

What should we do

A simple way to fix this would be to follow this piece of advice and split the tags like this:

document.write("<"+"script>...</"+"script>");

Which should be easy to add in the current scripts, a simple source.replaceAll('<script>', '<"+"script>').replaceAll('</script>', '</"+"script>') would do.

This is a bit hacky but not terrible either and it solves the issue.

Note 1: I can open a PR to implement it or another solution if need be.
Note 2: Found a duplicate issue, will comment there too.

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

No branches or pull requests

2 participants