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

Allow a callback to be passed for the cancelCallback argument to firebaseRef.on() #24

Merged
merged 2 commits into from
Oct 27, 2014
Merged

Conversation

insin
Copy link
Contributor

@insin insin commented Oct 23, 2014

Adds API suggested in #19

Not sure how to add a test to your suite for this, but have checked it at least works locally in react-hn against the Hacker News API, which errors when attempting to load delayed comments

…baseRef.on()

Not sure how to add a test to your suite for this, but have checked it at least works locally in react-hn against the Hacker News API, which errors when attempting to load delayed comments
@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling c3b8881 on insin:cancelCallback into c13e4aa on firebase:master.

insin added a commit to insin/react-hn that referenced this pull request Oct 23, 2014
Display a notificatiom to the user
Vendoring a version of Reactfire which allows you to pass a cancelCallback until FirebaseExtended/reactfire#24 is merged or similar functionality is added
@jwngr
Copy link

jwngr commented Oct 26, 2014

Hey @insin - I apologize for not responding to your issue or this pull request for such a long time. I'll try to be more responsive in the future.

Thanks for this PR! I really like this addition and want to add it to the library. I have one requests before merging this in though. The test suite is failing in Travis because of the change you made. Can you please update these lines to include a dummy callback function (function() {}) as the third parameter? This will make the test suite pass.

Thanks! I'm looking forward to getting this merged in!

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 060b29f on insin:cancelCallback into c13e4aa on firebase:master.

jwngr pushed a commit that referenced this pull request Oct 27, 2014
Allow a callback to be passed for the cancelCallback argument to firebaseRef.on()
@jwngr jwngr merged commit 537a3bd into FirebaseExtended:master Oct 27, 2014
@jwngr
Copy link

jwngr commented Oct 27, 2014

Thanks again for the PR @insin! We'll release a new version of ReactFire later this week with this change included. We may want to get a few of the other open PRs in as well.

randytsmith pushed a commit to randytsmith/newspost that referenced this pull request Apr 1, 2018
Display a notificatiom to the user
Vendoring a version of Reactfire which allows you to pass a cancelCallback until FirebaseExtended/reactfire#24 is merged or similar functionality is added
dezine2dev added a commit to dezine2dev/newspost that referenced this pull request Jul 4, 2018
Display a notificatiom to the user
Vendoring a version of Reactfire which allows you to pass a cancelCallback until FirebaseExtended/reactfire#24 is merged or similar functionality is added
nathaniel83 added a commit to nathaniel83/react-hacker-news that referenced this pull request Oct 3, 2018
Display a notificatiom to the user
Vendoring a version of Reactfire which allows you to pass a cancelCallback until FirebaseExtended/reactfire#24 is merged or similar functionality is added
@FirebaseExtended FirebaseExtended locked and limited conversation to collaborators Dec 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants