Optional event emit instead of function call for action buttons #1378

Merged
merged 3 commits into from Dec 14, 2016

Projects

None yet

4 participants

@akz92
Contributor
akz92 commented Nov 16, 2016 edited

Description

Add the option to emit events instead of executing functions on action buttons click.

Related Issue

#1192

Motivation and Context

It's easier and IMO better to work with events instead of functions in this kind of situation.

How Has This Been Tested?

I only tested the event emission.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
@macdonst
Member

@akz92 this looks like a good start but can you fill out the PR template so I know what has and hasn't been done. Also, only one file seems to have been changed even though you have 10 commits so your doc changes are missing.

@akz92
Contributor
akz92 commented Nov 16, 2016

@macdonst ok. I opted to remove the doc changes because I wasn't sure about how to add the details about this. I'll fill out the PR template and then I'll change the docs.

@macdonst
Member

@akz92 yeah, add the docs back in. I can modify them if required.

akz92 added some commits Nov 12, 2016
akz92 Emit event using action button callback name
Update emit description

Update action button callback instructions

Update action button callback instructions

Update PAYLOAD.md

Add conditional to execute function or emit event

Remove changes to doc

[fix] payload doc

get payload file from master

[fix] indentation

[fix] code style
9b803b2
akz92 Add documentation
[fix] Doc
bd92f58
@akz92
Contributor
akz92 commented Nov 19, 2016

Added the docs back. @macdonst I tried to keep it simple since this is a secondary behavior, please let me know if you think we need to add more details to the docs.

@macdonst
Member
macdonst commented Dec 2, 2016

@akz92 would you be interested in making a PR on the v2.0.x branch where I use ES2015?

@akz92
Contributor
akz92 commented Dec 2, 2016

Sure @macdonst, just give me a few days.

@naveedahmed1

Great job @akz92! @macdonst any update on this, when should we expect it to be merged?

@macdonst macdonst merged commit e92e951 into phonegap:master Dec 14, 2016

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
@macdonst
Member

@akz92 thanks for the PR. I've merged it into master and I'll get it into a 1.9.2 release.

@akz92
Contributor
akz92 commented Dec 14, 2016
@macdonst
Member

@akz92 I'm doing a rebase of your changes to the v2.0.x branch right now. So it may not be necessary. Hang tight.

@macdonst macdonst added a commit that referenced this pull request Dec 14, 2016
@macdonst macdonst Fixing my merge error for PR #1378 Optional event emit instead of fun…
…ction call for action buttons
af710b0
@macdonst
Member

@akz92 okay, I merged your code into 2.0.0 and fixed my merge screw up. It is now officially part of the 2.0.0 branch. Thanks again for your awesomeness. Do you have a twitter account?

@akz92
Contributor
akz92 commented Dec 14, 2016
@naveedahmed1

Is the Changelog updated? I see an entry of v1.9.2 (2016-07-09)

https://github.com/phonegap/phonegap-plugin-push/blob/master/CHANGELOG.md

but the date is (2016-07-09) which I believe is the date of v1.9.1

@naveedahmed1 naveedahmed1 referenced this pull request in driftyco/ionic-native Dec 14, 2016
Closed

Ionic 2 Native Push lacks Support for Actions #868

@fredgalvao
Contributor

@naveedahmed1 You're correct, the date was not properly generated: e02ff67#commitcomment-20189903

@macdonst macdonst added a commit that referenced this pull request Jan 4, 2017
@macdonst macdonst Fixing my merge error for PR #1378 Optional event emit instead of fun…
…ction call for action buttons
170d7dd
@macdonst macdonst added a commit that referenced this pull request Jan 6, 2017
@macdonst macdonst Fixing my merge error for PR #1378 Optional event emit instead of fun…
…ction call for action buttons
5205cf7
@macdonst macdonst added a commit that referenced this pull request Jan 12, 2017
@macdonst macdonst Fixing my merge error for PR #1378 Optional event emit instead of fun…
…ction call for action buttons
1f2fd67
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment