Refactored impress.js into an object while maintaining backward-compatibility with v0.3 #112

Closed
wants to merge 20 commits into
from

Conversation

Projects
None yet
2 participants
@jeady

jeady commented Mar 4, 2012

I refactored the impress.js source into an object with a couple of goals:

  • Easier to extend and customize
  • More concise source
  • More comments in the source
  • Backwards compatibility

I've managed to shave ~40 lines of code while (in my opinion at least) enhancing readability. The new impress function can either be constructed with new impress() in which case most CSS styles and parameters can be customized before beginning the presentation, or if simply called using impress() should return the same API you provided in v0.3. This is an enormous refactoring, I understand, so if you don't want to accept such a large pull request no worries at all, this was largely for my own use. I thought I'd offer my changes back in case anybody else is interested. Impress.js is fantastic, by the way. I'm really looking forward to seeing the kinds of presentations and websites that come out of tools like impress.js.

Keep up the fantastic work and let me know if you'd like to integrate my changes but would like me to make any more modifications first,
James

jeady added some commits Feb 29, 2012

Cleaned up setBrowserSpecificProperty, removed memoization because th…
…e overhead of not memoizing doesn't justify the added code complexity
Removed rootId as a parameter to start(), now use impress.root member…
… to change the root to something other than #impress
Began cleaning up start. Moved a bunch of initial setup code to be co…
…nfigurable before calling start, removed unnecessary spaces, etc.
First pass, full refactoring. Goto, prev, and next are public methods…
… now. Consolidated steps information into steps member variable
@bartaz

This comment has been minimized.

Show comment Hide comment
@bartaz

bartaz Mar 4, 2012

Member

Impressive refactoring indeed.

But it's way to much for me to use it as it is. And I don't agree with all decisions you made there. I don't see allowing to use new impress() syntax as an improvement - there is no need for it at all (especially if you assume there can be only one impress object). Also removing memoization from prefixing function - it maybe saved you 2-3 lines of code, but it really is useful to save this results as most of the time the same properties are checked - transition transition-delay transform, etc. It doesn't make sense to check for them every time (I know it's just a short for loop).

So, thanks for your input. For sure I will be looking at your code in more details to search for inspiration and improvements.

Member

bartaz commented Mar 4, 2012

Impressive refactoring indeed.

But it's way to much for me to use it as it is. And I don't agree with all decisions you made there. I don't see allowing to use new impress() syntax as an improvement - there is no need for it at all (especially if you assume there can be only one impress object). Also removing memoization from prefixing function - it maybe saved you 2-3 lines of code, but it really is useful to save this results as most of the time the same properties are checked - transition transition-delay transform, etc. It doesn't make sense to check for them every time (I know it's just a short for loop).

So, thanks for your input. For sure I will be looking at your code in more details to search for inspiration and improvements.

@bartaz bartaz closed this Mar 31, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment