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

Would you like to pull that stuff ? #26

Closed
wants to merge 18 commits into from
Closed

Would you like to pull that stuff ? #26

wants to merge 18 commits into from

Conversation

Golmote
Copy link
Contributor

@Golmote Golmote commented Mar 1, 2011

I'm going to continue exploring your code, and trying to find optimisations and new ideas for you ;)

Great job !

Added some braces... and some literal notation too. =°
…, removed some "var" statements, used the || operator for default values and moved the closures out of the setTimeout functions for better code legibility.
…n to literal and stored Spark function call in a variable out of the loop.
…lement (some IE fix...) and removed some 'var' statements.
@Olical
Copy link
Owner

Olical commented Mar 3, 2011

After downloading your branch and testing with a document that basically tests every function of Spark I can see that is it working for the most part. Although I am getting this error when animating because the data function is not working for some reason.

Uncaught TypeError: Object [object DOMWindow] has no method 'data'

Pretty strange if you ask me, why would only one function die? It just cant find this.data?

@Golmote
Copy link
Contributor Author

Golmote commented Mar 3, 2011

Could you give me that test document ? I would be able to investigate this issue ;)

@Olical
Copy link
Owner

Olical commented Mar 4, 2011

Okay, I will e-mail you the document :) and I have merged in some of the changes, not all of them though because some where conflicting with a few bits I had changes, but really helpful all the same. Thanks for the effort Golmote, keep it up!

This pull request was closed.
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

Successfully merging this pull request may close these issues.

2 participants