-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
feat(broadcaster): add Broadcaster plugin #877
Conversation
pluginRef: 'broadcaster', | ||
repo: 'https://github.com/bsorrentino/cordova-broadcaster', | ||
platforms: ['Android', 'iOS'], | ||
install: 'ionic plugin add cordova-plugin-broadcaster' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this line is not needed since we do not have a custom install command for this plugin
* @return {Promise<any>} Returns a promise that resolves when an event is received | ||
*/ | ||
@Cordova() | ||
static addEventListener(eventName: string): Promise<any> { return; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be a good idea to return an observable here.
@Cordova({
observable: true
})
static addEventListener(eventName: string): Observable<any> { return; }
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What would be the difference?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A promise resolves once, an observable can fire as many times as you want until you unsubscribe from it.
It should also have the following decorator instead of just observable: true
, this will make sure we unsubscribe from the event when unsubscribing from the observable:
@Cordova({
observable: true,
clearFunction: 'removeEventListener',
clearWithArgs: true
})
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah yes correct, will do
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
by the way, I should probably add the removeEventListener
definition, but to work it needs the function passed in the observable or the addEventListener
function, which seems a bit strange, doesn't it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No you don't need to add the removeEventListener
. The @Cordova
decorator will do the magic needed once you add the options I mentioned in my last comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding the clearFunction
option will call that function when you unsubscribe from the observable. The clearWithArgs
option will pass the same arguments passed to addEventListener
(which are needed in this case).
- also remove the listener when clearing observable
Fixes #875
plugin: https://github.com/bsorrentino/cordova-broadcaster