[PushNotificationIOS] Added badge icon number and alert action #2931

alvaromb · 2015-09-22T10:42:10Z

Added support for setting an optional badge icon number and alert action when using local notifications.

christopherdro · 2016-02-05T16:50:39Z

@alvaromb Any chance you can rebase this against master?
@nicklockwood Do you see anything holding this back from getting merged in?

alvaromb · 2016-02-05T22:39:45Z

I don't know if this got merged, completely forgot 😆

I can update the PR or send another one.

chirag04 · 2016-03-06T17:01:09Z

@alvaromb alertAction is already added in master. Do you want to rebase and add support for applicationIconBadgeNumber?

alvaromb · 2016-03-07T07:32:53Z

Ok, I'll try to do a rebase today.

ghost · 2016-03-12T18:00:25Z

@vjeux would you mind taking a look at this pull request? It's been a while since the last commit was reviewed.

vjeux · 2016-03-12T18:16:54Z

I have 0 context on this but the other reviewers might

facebook-github-bot · 2016-03-12T18:17:15Z

@alvaromb updated the pull request.

mkonicek · 2016-03-20T15:25:53Z

I'll try assigning this to @majak as all the iOS pull requests are assigned to Nick and this doesn't scale :)

alvaromb · 2016-03-20T16:11:55Z

Sorry folks, I'm currently having a gargantuan amount of work right now, didn't have time to look at this. Do you want me to rebase anything?

mkonicek · 2016-03-21T00:39:42Z

@alvaromb That's understandable :) Let's wait for @majak first (please reassign to someone else than Nick if you're too busy) to see how we should proceed with this PR.

joshuapinter · 2016-04-09T18:33:14Z

Hey Guys, I'd love to have this in here. Is there anything I can do to help, like a rebase against Master since alertAction has already been added? Let me know.

joshuapinter · 2016-04-10T02:36:11Z

As a means of getting this into master as quick as possible (I need this for our next release) I created a new PR with the same changes on the master branch.

The new PR is #6894.

Thanks for your effort on this @alvaromb! 👍

alvaromb · 2016-04-10T06:51:54Z

Thanks Joshua! :)

Enviado desde mi iPhone

El 10 abr 2016, a las 4:37, Joshua Pinter notifications@github.com escribió:

As a means of getting this into master as quick as possible (I need this for our next release) I created a new PR with the same changes on the master branch.

The new PR is #6894.

Thanks for your effort @alvaromb on this!

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub

mkonicek · 2016-04-22T14:17:08Z

I'd like at least one person with Obj-C knowledge to take a look but everyone seems too busy. Maybe I'll have to learn Obj-C :)

This one seems simple enough for me to review.

@facebook-github-bot shipit

alvaromb · 2016-04-22T14:20:14Z

Please ping me if you need some info about the PR @mkonicek 😃

facebook-github-bot · 2016-04-22T14:22:59Z

Thanks for importing. If you are an FB employee go to Phabricator to review.

joshuapinter · 2016-04-22T14:25:50Z

Thanks for taking a look @mkonicek. Because this PR took so long to get merged there was a bunch of code changes to master so I ended up creating a new PR with @alvaromb's changes except based off the latest master branch: #6894.

I don't know if this one or #6894 is easier for you to merge in, but they accomplish the same thing.

Over to you! :)

alloy · 2016-04-22T14:34:23Z

Libraries/PushNotificationIOS/RCTPushNotificationManager.m

@@ -35,6 +35,8 @@ + (UILocalNotification *)UILocalNotification:(id)json
  UILocalNotification *notification = [UILocalNotification new];
  notification.fireDate = [RCTConvert NSDate:details[@"fireDate"]] ?: [NSDate date];
  notification.alertBody = [RCTConvert NSString:details[@"alertBody"]];
+  notification.alertAction = [RCTConvert NSString:details[@"alertAction"]] ?: nil;


Not a biggie, but I’m unsure what the point is of the extra ternary. Right now it reads to me like: if the result of converting is nil return nil instead.

alloy · 2016-04-22T14:35:56Z

@mkonicek I left some comments, hope it helps.

alvaromb · 2016-04-22T14:41:39Z

Thanks for the feedback @alloy. Definitely not the best code ever, I'll update the PR.

facebook-github-bot · 2016-04-22T14:54:17Z

@alvaromb updated the pull request.

alvaromb · 2016-04-22T14:56:55Z

I've added @alloy suggestions (thanks!) and updated the PR. /cc @joshuapinter

joshuapinter · 2016-04-22T15:07:16Z

@alvaromb Beauty! I'll go close my PR.

alloy · 2016-04-22T15:07:19Z

@alvaromb @mkonicek Yup, this looks good to me 👍

mkonicek · 2016-04-22T15:41:32Z

OK merging the updated code.

@facebook-github-bot shipit

facebook-github-bot · 2016-04-22T15:42:45Z

Thanks for importing. If you are an FB employee go to Phabricator to review.

mkonicek · 2016-04-29T10:15:26Z

@alvaromb Can you please rebase? There's a merge conflict with master, sorry about that.

…ations Improved applicationIconBadgeNumber handling and updated docs

ghost · 2016-04-29T10:32:19Z

@alvaromb updated the pull request.

alvaromb · 2016-04-29T10:32:28Z

There you go @mkonicek! Please let me know if you need anything else 😃

mkonicek · 2016-04-29T11:43:22Z

Thanks @alvaromb!

@facebook-github-bot shipit

ghost · 2016-04-29T11:46:26Z

Thanks for importing. If you are an FB employee go to Phabricator to review.

Summary: Added support for setting an optional badge icon number and alert action when using local notifications. Closes facebook#2931 Differential Revision: D3212448 Pulled By: mkonicek fb-gh-sync-id: 063efcdd259b2a43f39812f57a71e8489ab33653 fbshipit-source-id: 063efcdd259b2a43f39812f57a71e8489ab33653

facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Sep 22, 2015

mkonicek assigned majak Mar 20, 2016

mkonicek added the Triage - In Progress label Mar 20, 2016

joshuapinter mentioned this pull request Apr 10, 2016

Add applicationIconBadgeNumber to LocalNotification. #6894

Closed

mkonicek unassigned majak Apr 22, 2016

facebook-github-bot added GH Review: accepted Import Started This pull request has been imported. This does not imply the PR has been approved. and removed GH Review: review-needed labels Apr 22, 2016

alloy reviewed Apr 22, 2016
View reviewed changes

alvaromb force-pushed the local-notifications-params branch from 6f45cb1 to a1c441f Compare April 22, 2016 14:53

mkonicek removed the Triage - In Progress label Apr 22, 2016

facebook-github-bot added Import Failed and removed Import Started This pull request has been imported. This does not imply the PR has been approved. labels Apr 23, 2016

mkonicek added Needs Response from Author and removed Import Failed labels Apr 29, 2016

Added support for badge icon number and alert action in local notific…

a5bf1ab

…ations Improved applicationIconBadgeNumber handling and updated docs

alvaromb force-pushed the local-notifications-params branch from a1c441f to a5bf1ab Compare April 29, 2016 10:31

ghost added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Apr 29, 2016

ghost closed this in 84e6968 Apr 29, 2016

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PushNotificationIOS] Added badge icon number and alert action #2931

[PushNotificationIOS] Added badge icon number and alert action #2931

alvaromb commented Sep 22, 2015

christopherdro commented Feb 5, 2016

alvaromb commented Feb 5, 2016

chirag04 commented Mar 6, 2016

alvaromb commented Mar 7, 2016

ghost commented Mar 12, 2016

vjeux commented Mar 12, 2016

facebook-github-bot commented Mar 12, 2016

mkonicek commented Mar 20, 2016

alvaromb commented Mar 20, 2016

mkonicek commented Mar 21, 2016

joshuapinter commented Apr 9, 2016

joshuapinter commented Apr 10, 2016

alvaromb commented Apr 10, 2016

mkonicek commented Apr 22, 2016 •

edited

Loading

alvaromb commented Apr 22, 2016

facebook-github-bot commented Apr 22, 2016

joshuapinter commented Apr 22, 2016

alloy Apr 22, 2016

alloy commented Apr 22, 2016

alvaromb commented Apr 22, 2016

facebook-github-bot commented Apr 22, 2016

alvaromb commented Apr 22, 2016

joshuapinter commented Apr 22, 2016

alloy commented Apr 22, 2016

mkonicek commented Apr 22, 2016

facebook-github-bot commented Apr 22, 2016

mkonicek commented Apr 29, 2016

ghost commented Apr 29, 2016

alvaromb commented Apr 29, 2016

mkonicek commented Apr 29, 2016

ghost commented Apr 29, 2016

[PushNotificationIOS] Added badge icon number and alert action #2931

[PushNotificationIOS] Added badge icon number and alert action #2931

Conversation

alvaromb commented Sep 22, 2015

christopherdro commented Feb 5, 2016

alvaromb commented Feb 5, 2016

chirag04 commented Mar 6, 2016

alvaromb commented Mar 7, 2016

ghost commented Mar 12, 2016

vjeux commented Mar 12, 2016

facebook-github-bot commented Mar 12, 2016

mkonicek commented Mar 20, 2016

alvaromb commented Mar 20, 2016

mkonicek commented Mar 21, 2016

joshuapinter commented Apr 9, 2016

joshuapinter commented Apr 10, 2016

alvaromb commented Apr 10, 2016

mkonicek commented Apr 22, 2016 • edited Loading

alvaromb commented Apr 22, 2016

facebook-github-bot commented Apr 22, 2016

joshuapinter commented Apr 22, 2016

alloy Apr 22, 2016

Choose a reason for hiding this comment

alloy commented Apr 22, 2016

alvaromb commented Apr 22, 2016

facebook-github-bot commented Apr 22, 2016

alvaromb commented Apr 22, 2016

joshuapinter commented Apr 22, 2016

alloy commented Apr 22, 2016

mkonicek commented Apr 22, 2016

facebook-github-bot commented Apr 22, 2016

mkonicek commented Apr 29, 2016

ghost commented Apr 29, 2016

alvaromb commented Apr 29, 2016

mkonicek commented Apr 29, 2016

ghost commented Apr 29, 2016

mkonicek commented Apr 22, 2016 •

edited

Loading