chariotsolutions / phonegap-nfc

PhoneGap NFC Plugin
MIT License
706 stars 559 forks source link

Missing callback parameter in removeNdefListener #148

Closed jakobwitte closed 9 years ago

jakobwitte commented 9 years ago

In phonegap-nfc.js on line 432 the callback parameter has been removed from the removeNdefListener function which triggers an exception when the function is called:

Fix:

removeNdefListener: function (win, fail) {
    document.removeEventListener("ndef", callback, false);
    cordova.exec(win, fail, "NfcPlugin", "removeNdef", []);
}

-->

removeNdefListener: function (callback, win, fail) {
    document.removeEventListener("ndef", callback, false);
    cordova.exec(win, fail, "NfcPlugin", "removeNdef", []);
}
JohnMcLear commented 9 years ago

good spot @jakobwitte , why not submit a pull request with your fix? :)

JohnMcLear commented 9 years ago

FYI @jakobwitte one would normally close the issue once the pull request gets merged

Also in your pull requests you'd usually have a descriptive title such as "implemented missing callback in removeNdefListener"

Also I recommend linking issues <> pull requests by stating the issue # in the pull request or the pull request in the issue, such as this: https://github.com/chariotsolutions/phonegap-nfc/pull/149 <-- note that this is now referenced in the PR

Just wanna help you out, thanks!

jakobwitte commented 9 years ago

Thank you for your comments. I'm quite new to github and I'm not familiar with the processes in here so your comments are much appreciated.

On 24 October 2014 23:06, John McLear notifications@github.com wrote:

FYI @jakobwitte https://github.com/jakobwitte one would normally close the issue once the pull request gets merged

Also in your pull requests you'd usually have a descriptive title such as "implemented missing callback in removeNdefListener"

Also I recommend linking issues <> pull requests by stating the issue # in the pull request or the pull request in the issue, such as this: #149 https://github.com/chariotsolutions/phonegap-nfc/pull/149 <-- note that this is now referenced in the PR

Just wanna help you out, thanks!

— Reply to this email directly or view it on GitHub https://github.com/chariotsolutions/phonegap-nfc/issues/148#issuecomment-60449332 .

JohnMcLear commented 9 years ago

My pleasure, feel free to follow me to see how I use it

http://github.com/johnmclear

I can't guarantee consistent good practice but I try.

don commented 9 years ago

This is also an issue with the other remove methods. Issue #142 was a bad idea.