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

Introduce plugin framework #622

Merged
merged 2 commits into from
Oct 2, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
/js/impress.min.js
/node_modules
/npm-debug.log
12 changes: 12 additions & 0 deletions build.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
var buildify = require('buildify');

buildify()
.load('src/impress.js')
.concat(['src/plugins/navigation/navigation.js',
'src/plugins/resize/resize.js'])
.save('js/impress.js');
/*
* Disabled until uglify supports ES6: https://github.com/mishoo/UglifyJS2/issues/448
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is now uglify-es module for ES6 support, so we would need to switch to that to support it.

Not that it needs to be done in this PR, but just a note for future.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool. I'll keep that in mind as a future improvement. I really like to provide the minified experience, but felt that not adopting language improvements was eventually holding back progress too much.

.uglify()
.save('js/impress.min.js');
*/
253 changes: 156 additions & 97 deletions js/impress.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: This is now generated. Makes more sense to comment on the individual files under src/

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not exactly sure if keeping generated file in repo is a good thing.
It needs to be kept up to date with rest of the code (you need to remember to build it before commit) and having all the changes doubled makes PRs bit confusing.

That said, I understand the idea behind it (to have it ready to use), so I don't have any other solution now.

Maybe in future we would be able to not have it in repo, but have it built and published to some CDN, so built version would be available on-line, while not being in the repo itself.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Moving here as the reply works now)

I'm not exactly sure if keeping generated file in repo is a good thing.
It needs to be kept up to date with rest of the code (you need to remember to build it before commit) and having all the changes doubled makes PRs bit confusing.

True. It does of course happen that one forgets to do the final build before commit. We could add an automatic check to prevent that though.

For PRs / git diff I don't personally mind, I get used to skipping that quite fast.

That said, I understand the idea behind it (to have it ready to use), so I don't have any other solution now.
Maybe in future we would be able to not have it in repo, but have it built and published to some CDN, so built version would be available on-line, while not being in the repo itself.

For now, the absolutely biggest issue of course was simply backward compatibility with upstream, and IMO that is important consideration going forward as well. There should be compelling reasons to break with this custom, more than just something being maybe being a bit better. That is to say, I'm not strictly disagreeing with publishing to a CDN or users having to build js/impress.js, but IMO there's also nothing horribly bad with the way it is now and there's value in just continuing with what is already established.

Personally I do like the end user workflow where you can just git clone and point to the js/impress.js file, without, for example, needing to use npm at all. When integrating some of the extras plugins, it always annoyed me how nonstandard that is across projects. For one, you need to download a zip file. For another, you need to npm install. For a third, you need to npm run build dist whatever, and then a lot of new files appear, and you need to figure out which one you actually want. Ease of use of impress.js is really good right now, so lets try to keep it.

/*jshint bitwise:true, curly:true, eqeqeq:true, forin:true, latedef:true, newcap:true,
noarg:true, noempty:true, undef:true, strict:true, browser:true */
/*global window*/

// You are one of those who like to know how things work inside?
// Let me show you the cogs that make impress.js run...
Expand Down Expand Up @@ -213,7 +214,7 @@

// And that's where interesting things will start to happen.
// It's the core `impress` function that returns the impress.js API
// for a presentation based on the element with given id ('impress'
// for a presentation based on the element with given id ("impress"
// by default).
var impress = window.impress = function( rootId ) {

Expand Down Expand Up @@ -419,9 +420,8 @@
// Used to reset timeout for `impress:stepenter` event
var stepEnterTimeout = null;

// `goto` API function that moves to step given with `el` parameter
// (by index, id or element), with a transition `duration` optionally
// given as second parameter.
// `goto` API function that moves to step given as `el` parameter (by index, id or element).
// `duration` optionally given as second parameter, is the transition duration in css.
var goto = function( el, duration ) {

if ( !initialized || !( el = getStep( el ) ) ) {
Expand Down Expand Up @@ -496,16 +496,15 @@
// being animated separately:
// `root` is used for scaling and `canvas` for translate and rotations.
// Transitions on them are triggered with different delays (to make
// visually nice and 'natural' looking transitions), so we need to know
// visually nice and "natural" looking transitions), so we need to know
// that both of them are finished.
css( root, {

// To keep the perspective look similar for different scales
// we need to "scale" the perspective, too
// For IE 11 support we must specify perspective independent
// of transform.
perspective: ( config.perspective / targetScale ) + "px",

// To keep the perspective look similar for different scales
// we need to 'scale' the perspective, too
transform: scale( targetScale ),
transitionDuration: duration + "ms",
transitionDelay: ( zoomin ? delay : 0 ) + "ms"
Expand All @@ -526,8 +525,7 @@
// account.
//
// I know that this `if` statement looks scary, but it's pretty simple when you know
// what is going on
// - it's simply comparing all the values.
// what is going on - it's simply comparing all the values.
if ( currentState.scale === target.scale ||
( currentState.rotate.x === target.rotate.x &&
currentState.rotate.y === target.rotate.y &&
Expand All @@ -543,22 +541,20 @@
activeStep = el;

// And here is where we trigger `impress:stepenter` event.
// We simply set up a timeout to fire it taking transition duration
// (and possible delay) into account.
// We simply set up a timeout to fire it taking transition duration (and possible delay)
// into account.
//
// I really wanted to make it in more elegant way. The `transitionend` event seemed to
// be the best way to do it, but the fact that I'm using transitions on two separate
// elements and that the `transitionend` event is only triggered when there was a
// transition (change in the values) caused some bugs and made the code really
// complicated, cause I had to handle all the conditions separately. And it still
// needed a `setTimeout` fallback for the situations when there is no transition at
// all.
// needed a `setTimeout` fallback for the situations when there is no transition at all.
// So I decided that I'd rather make the code simpler than use shiny new
// `transitionend`.
//
// If you want learn something interesting and see how it was done with `transitionend`
// go back to
// version 0.5.2 of impress.js:
// go back to version 0.5.2 of impress.js:
// http://github.com/bartaz/impress.js/blob/0.5.2/js/impress.js
window.clearTimeout( stepEnterTimeout );
stepEnterTimeout = window.setTimeout( function() {
Expand Down Expand Up @@ -667,28 +663,45 @@

} )( document, window );

// NAVIGATION EVENTS

// As you can see this part is separate from the impress.js core code.
// It's because these navigation actions only need what impress.js provides with
// its simple API.
// THAT'S ALL FOLKS!
//
// In future I think about moving it to make them optional, move to separate files
// and treat more like a 'plugins'.
( function( document, window ) {
// Thanks for reading it all.
// Or thanks for scrolling down and reading the last part.
//
// I've learnt a lot when building impress.js and I hope this code and comments
// will help somebody learn at least some part of it.

/**
* Navigation events plugin
*
* As you can see this part is separate from the impress.js core code.
* It's because these navigation actions only need what impress.js provides with
* its simple API.
*
* This plugin is what we call an _init plugin_. It's a simple kind of
* impress.js plugin. When loaded, it starts listening to the `impress:init`
* event. That event listener initializes the plugin functionality - in this
* case we listen to some keypress and mouse events. The only dependencies on
* core impress.js functionality is the `impress:init` method, as well as using
* the public api `next(), prev(),` etc when keys are pressed.
*
* Copyright 2011-2012 Bartek Szopka (@bartaz)
* Released under the MIT license.
* ------------------------------------------------
* author: Bartek Szopka
* version: 0.5.3
* url: http://bartaz.github.com/impress.js/
* source: http://github.com/bartaz/impress.js/
*
*/
/* global document */
( function( document ) {
"use strict";

// Throttling function calls, by Remy Sharp
// http://remysharp.com/2010/07/21/throttling-function-calls/
var throttle = function( fn, delay ) {
var timer = null;
return function() {
var context = this, args = arguments;
clearTimeout( timer );
timer = setTimeout( function() {
fn.apply( context, args );
}, delay );
};
var triggerEvent = function( el, eventName, detail ) {
var event = document.createEvent( "CustomEvent" );
event.initCustomEvent( eventName, true, true, detail );
el.dispatchEvent( event );
};

// Wait for impress.js to be initialized
Expand All @@ -700,19 +713,6 @@
// need to control the presentation that was just initialized.
var api = event.detail.api;

// KEYBOARD NAVIGATION HANDLERS

// Prevent default keydown action when one of supported key is pressed.
document.addEventListener( "keydown", function( event ) {
if ( event.keyCode === 9 ||
( event.keyCode >= 32 && event.keyCode <= 34 ) ||
( event.keyCode >= 37 && event.keyCode <= 40 ) ) {
event.preventDefault();
}
}, false );

// Trigger impress action (next or prev) on keyup.

// Supported keys are:
// [space] - quite common in presentation software to move forward
// [up] [right] / [down] [left] - again common and natural addition,
Expand All @@ -726,30 +726,71 @@
// positioning. I didn't want to just prevent this default action, so I used [tab]
// as another way to moving to next step... And yes, I know that for the sake of
// consistency I should add [shift+tab] as opposite action...
document.addEventListener( "keyup", function( event ) {
var isNavigationEvent = function( event ) {

if ( event.shiftKey || event.altKey || event.ctrlKey || event.metaKey ) {
return;
// Don't trigger navigation for example when user returns to browser window with ALT+TAB
if ( event.altKey || event.ctrlKey || event.metaKey ) {
return false;
}

if ( event.keyCode === 9 ||
( event.keyCode >= 32 && event.keyCode <= 34 ) ||
( event.keyCode >= 37 && event.keyCode <= 40 ) ) {
switch ( event.keyCode ) {
case 33: // Page up
case 37: // Left
case 38: // Up
api.prev();
break;
case 9: // Tab
case 32: // Space
case 34: // Page down
case 39: // Right
case 40: // Down
api.next();
break;
}
// In the case of TAB, we force step navigation always, overriding the browser
// navigation between input elements, buttons and links.
if ( event.keyCode === 9 ) {
return true;
}

// With the sole exception of TAB, we also ignore keys pressed if shift is down.
if ( event.shiftKey ) {
return false;
}

// For arrows, etc, check that event target is html or body element. This is to allow
// presentations to have, for example, forms with input elements where user can type
// text, including space, and not move to next step.
if ( event.target.nodeName !== "BODY" && event.target.nodeName !== "HTML" ) {
return false;
}

if ( ( event.keyCode >= 32 && event.keyCode <= 34 ) ||
( event.keyCode >= 37 && event.keyCode <= 40 ) ) {
return true;
}
};

// KEYBOARD NAVIGATION HANDLERS

// Prevent default keydown action when one of supported key is pressed.
document.addEventListener( "keydown", function( event ) {
if ( isNavigationEvent( event ) ) {
event.preventDefault();
}
}, false );

// Trigger impress action (next or prev) on keyup.
document.addEventListener( "keyup", function( event ) {
if ( isNavigationEvent( event ) ) {
if ( event.shiftKey ) {
switch ( event.keyCode ) {
case 9: // Shift+tab
api.prev();
break;
}
} else {
switch ( event.keyCode ) {
case 33: // Pg up
case 37: // Left
case 38: // Up
api.prev( event );
break;
case 9: // Tab
case 32: // Space
case 34: // Pg down
case 39: // Right
case 40: // Down
api.next( event );
break;
}
}
event.preventDefault();
}
}, false );
Expand All @@ -758,7 +799,7 @@
document.addEventListener( "click", function( event ) {

// Event delegation with "bubbling"
// Check if event target (or any of its parents is a link)
// check if event target (or any of its parents is a link)
var target = event.target;
while ( ( target.tagName !== "A" ) &&
( target !== document.documentElement ) ) {
Expand Down Expand Up @@ -786,8 +827,8 @@

// Find closest step element that is not active
while ( !( target.classList.contains( "step" ) &&
!target.classList.contains( "active" ) ) &&
( target !== document.documentElement ) ) {
!target.classList.contains( "active" ) ) &&
( target !== document.documentElement ) ) {
target = target.parentNode;
}

Expand All @@ -796,41 +837,59 @@
}
}, false );

// Touch handler to detect taps on the left and right side of the screen
// based on awesome work of @hakimel: https://github.com/hakimel/reveal.js
document.addEventListener( "touchstart", function( event ) {
if ( event.touches.length === 1 ) {
var x = event.touches[ 0 ].clientX,
width = window.innerWidth * 0.3,
result = null;

if ( x < width ) {
result = api.prev();
} else if ( x > window.innerWidth - width ) {
result = api.next();
}
// Add a line to the help popup
triggerEvent( document, "impress:help:add",
{ command: "Left &amp; Right", text: "Previous &amp; Next step", row: 1 } );

if ( result ) {
event.preventDefault();
}
}
}, false );
}, false );

} )( document );


/**
* Resize plugin
*
* Rescale the presentation after a window resize.
*
* Copyright 2011-2012 Bartek Szopka (@bartaz)
* Released under the MIT license.
* ------------------------------------------------
* author: Bartek Szopka
* version: 0.5.3
* url: http://bartaz.github.com/impress.js/
* source: http://github.com/bartaz/impress.js/
*
*/

/* global document, window */

( function( document, window ) {
"use strict";

// Throttling function calls, by Remy Sharp
// http://remysharp.com/2010/07/21/throttling-function-calls/
var throttle = function( fn, delay ) {
var timer = null;
return function() {
var context = this, args = arguments;
window.clearTimeout( timer );
timer = window.setTimeout( function() {
fn.apply( context, args );
}, delay );
};
};

// Wait for impress.js to be initialized
document.addEventListener( "impress:init", function( event ) {
var api = event.detail.api;

// Rescale presentation when window is resized
window.addEventListener( "resize", throttle( function() {

// Force going to active step again, to trigger rescaling
api.goto( document.querySelector( ".step.active" ), 500 );
}, 250 ), false );

}, false );

} )( document, window );

// THAT'S ALL FOLKS!
//
// Thanks for reading it all.
// Or thanks for scrolling down and reading the last part.
//
// I've learnt a lot when building impress.js and I hope this code and comments
// will help somebody learn at least some part of it.
4 changes: 1 addition & 3 deletions karma.conf.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,8 @@ module.exports = function( config ) {
// The QUnit tests
"test/helpers.js",
"test/core_tests.js",
"test/navigation_tests.js",
"src/plugins/navigation/navigation_tests.js",
// Presentation files, for the iframe
//"test/core_tests_presentation.html"
//{pattern: "test/core_tests_presentation.html", watched: true, served: true, included: false}
{pattern: "test/*.html", watched: true, served: true, included: false},
{pattern: "test/plugins/*/*.html", watched: true, served: true, included: false},
// JS files for iframe
Expand Down
Loading