Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Expose 'impress' namespace and allow programmatic configuration of steps objects #83

Closed
wants to merge 6 commits into
from

Conversation

Projects
None yet
3 participants
Contributor

medikoo commented Jan 29, 2012

See #76 for details

Owner

bartaz commented Jan 30, 2012

Thanks!

I liked the .bind approach... maybe we could use a Function.bind polyfill for non-supporting browsers?

I am thinking about some cleaning, too...

I don't really like the impressEl variable. Will have to think for some other name for it. Maybe the canvas one should be renamed too, to make them both similar.

I'd also like to simplify the validation. It looks like repeated code. Maybe it could be extracted as some validation function?

Contributor

medikoo commented Jan 30, 2012

@bartaz valid points.

I refrained from using .bind at first, to not to introduce shim just for this case, and other thing it's not that easy to come up with really good bind shim, introducing such should be done with care as users may want to use other tools with impress. However as lib will expand we can clearly benefit from bind so I've added best shim I know for that

Variable names are indeed not consequent, but I just wanted to change as least as possible from your code, I would rather call this pull as first step of transition into more organized code base which I think it's best if done step by step.
Let me know how you would like to have impressEl variable named.

Validation, I thought about that as well, but I guess I just wanted to present it in more direct way for you, but anyway I took out abstraction into two separate functions. Of course we may use simpler algorithm, but the way I've done it's probably the only bulletproof and up to ES conventions way to do that.

Contributor

medikoo commented Jan 30, 2012

@bartaz according to cleaning up, have you thought about separating project into different files ? I mean just for development, similarly as e.g. jQuery is maintained.

Member

FagnerMartinsBrack commented Feb 4, 2016

Hi, in an effort to clear up older issues/PRs we are pinging back to know if you are still tracking this request.

To give a little bit of context, recently a decision was made in the project to make the development more active and the first task is to clear up older issues like this one to see if the OP is still interested in keep it going.

If you want to keep it going I suggest creating one Pull Request for each atomic change, like one for variable renaming, etc.

@bartaz Is that integration branch still relevant? Should all these kind of things be opened in the master branch?

Member

FagnerMartinsBrack commented Feb 15, 2016

Closing due to the lack of feedback from the OP. If you want to work in the PR again, feel free to comment here to open up the discussion again.

As stated in the comments above this feature is really useful, therefore I created #516 to keep track of it.

Thank you for the PR!

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