-
Notifications
You must be signed in to change notification settings - Fork 28
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 impressConsole.js a self contained impress.js plugin #22
Make impressConsole.js a self contained impress.js plugin #22
Conversation
This patch makes necessary changes to adhere to the new impress.js plugin api, as described here: https://github.com/henrikingo/impress.js/blob/myfork/src/plugins/README.md See #19 for a thorough discussion on what that means for impressConsole in particular. This patch implements the changes described in that issue. Even if you don't care about such a plugin api, this introduces the following enhancements for any user of impress-console: * impressConsole will now call init() on itself automatically when you call impress().init(). This means you only need to include impressConsole.js file with a <script>, and it will work out of the box. * The default css is inlined into impressConsole.js. This means that impressConsole.js is now a self contained file and doesn't depend on any other file (e.g. the css files) for working. * All changes are backward compatible. For example, you can still call `impressConsole().init('css/impressConsoleDark.css')` if you want.
@@ -74,10 +76,12 @@ | |||
'</body></html>'; | |||
|
|||
// Default css location | |||
var cssFile = "css/impressConsole.css"; | |||
var cssFileOldDefault = "css/impressConsole.css"; |
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.
The old default is simply used in the new init() to compare and possibly print a deprecation message.
@@ -324,7 +342,7 @@ | |||
// Add clock tick | |||
consoleWindow.timerStart = new Date(); | |||
consoleWindow.timerReset = timerReset; | |||
consoleWindow.clockInterval = setInterval('impressConsole("' + rootId + '").clockTick()', 1000 ); | |||
consoleWindow.clockInterval = setInterval(allConsoles[rootId].clockTick, 1000 ); |
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.
This doesn't change anything, but internally there is now no dependency on calling window.impressConsole(). Hypothetically in a distant future such a global object could be removed (but there's no pressing need to actually do so).
|
||
// Returns a string to be used inline as a css <style> element in the console window. | ||
// Apologies for length, but hiding it here at the end to keep it away from rest of the code. | ||
var cssStyleStr = function(){ |
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.
There was much more CSS than I had realized when I proposed this change. Let me know if you feel bad about this. Still, upon reflection, I felt that inlining this default css has the benefit that you only need impressConsole.js somewhere, and don't need any other files and don't even need to call impress().init() at all to use the console, it's initialized automatically.
// Returns a string to be used inline as a css <style> element in the console window. | ||
// Apologies for length, but hiding it here at the end to keep it away from rest of the code. | ||
var cssStyleStr = function(){ | ||
return `<style> |
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.
EcmaScript 6 introduced template string
syntax, which can also be used for multiline strings. I figured since the policy in impress.js is to only target newest borwsers anyway, it is ok to use it. If you disagree, I'm happy to change to
'...' +
'...' + etc...
Also applies this pull request, which makes this possible: regebro/impress-console#22
Press 'P' to show a speaker console in a separate window. Supports: - Navigation controls - This slide and next slide preview screens - Speaker notes - Clock and timer Also applies this patch, which makes impressConsole.js follow the new impress.js plugin standard: regebro/impress-console#22 Note: As impressConsole is now a plugin, it is included by default. You no longer need to include it with a separate <script> tag. Nor do you need to call its init() method.
Press 'P' to show a speaker console in a separate window. Supports: - Navigation controls - This slide and next slide preview screens - Speaker notes - Clock and timer Also applies this patch, which makes impressConsole.js follow the new impress.js plugin standard: regebro/impress-console#22 Note: As impressConsole is now a plugin, it is included by default. You no longer need to include it with a separate <script> tag. Nor do you need to call its init() method.
This patch makes necessary changes to adhere to the new impress.js
plugin api, as described here:
https://github.com/henrikingo/impress.js/blob/myfork/src/plugins/README.md
See #19 for a thorough
discussion on what that means for impressConsole in particular. This
patch implements the changes described in that issue.
Even if you don't care about such a plugin api, this introduces the following
enhancements for any user of impress-console:
you call impress().init(). This means you only need to include
impressConsole.js file with a <script>, and it will work out of
the box.
impressConsole.js is now a self contained file and doesn't depend
on any other file (e.g. the css files) for working.
impressConsole().init('css/impressConsoleDark.css')
if you want.