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

Get default data #49

Merged
merged 6 commits into from
Aug 20, 2014
Merged

Get default data #49

merged 6 commits into from
Aug 20, 2014

Conversation

undoZen
Copy link
Contributor

@undoZen undoZen commented Aug 18, 2014

Just picked up from where @SimonDegraeve left on #46 . Fix bug and add tests.
Also, add two test cases for stores listen to other stores.

@SimonDegraeve
Copy link
Contributor

Nice job. I have been buzy looking deeper into yahoo's dispatchr, fetchr and routr and like you said in this comment the dehydrate/rehydrate feature is quiet awesome.

Appreciate you fix this PR.

@SimonDegraeve
Copy link
Contributor

Also it would be nice to handle the defaultCallback in createStore and ListenerMixin in a same module (see explanations)

@undoZen
Copy link
Contributor Author

undoZen commented Aug 19, 2014

All done. @spoike please review and merge it.

@spoike spoike added this to the 0.1.7 milestone Aug 20, 2014
@spoike
Copy link
Member

spoike commented Aug 20, 2014

This needs to be documented in the README.md. I'll try to merge this in the evening after work (around 19:00Z+02:00), to make the 0.1.7 release. Feel free to add a blurb about it if @undoZen or @SimonDegraeve has time to do so. :-)

@spoike
Copy link
Member

spoike commented Aug 20, 2014

Four test cases in testling fail with the following error message:

TypeError: 'undefined' is not a function (evaluating 'defaultCallback.bind(listener)')

They fail because the solution is using Function.prototype.bind which is not available on all browsers.

@spoike
Copy link
Member

spoike commented Aug 20, 2014

I've changed it for the 0.1.7 merge to use Function.prototype.apply and Function.prototype.call instead.

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.

3 participants