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

feat(android): support custom local notification icon #1830

Merged
merged 3 commits into from
Aug 1, 2019
Merged

feat(android): support custom local notification icon #1830

merged 3 commits into from
Aug 1, 2019

Conversation

nikosdouvlis
Copy link
Contributor

@nikosdouvlis nikosdouvlis commented Jul 31, 2019

Support for android custom local notification icon.

Added a smallIcon property to LocalNotificatoin. Can accept values of both res://icon_name and icon_name types (to stay consistent with the attachments' url prop). Will only look inside the res/drawables directory since the mipmap one is supposed to be used only for launcher icons.

If no custom icon is set or if the custom one cannot be found, the standard android info icon will be used instead.

If this gets merged, the docs need to be updated to make clear that the custom icon needs to follow Android's guidelines (see the top answer here: https://stackoverflow.com/questions/45318614/why-is-my-smallicon-for-notifications-always-greyed-out) or else the it won't be displayed.

Closes #1752
Closes #1510

Copy link
Member

@jcesarmobile jcesarmobile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a comment, but not sure if the icon should be provided every time as part of the notification payload or if it's better to make a plugin preference to be added in the plugins.LocalNotifications or android sections of capacitor.config.json, or even in the AndroidManifest.xml as the push icon, that way configuring it once is enough. Or maybe both approaches but if it's passed as part of the payload make it take precedence?

@nikosdouvlis
Copy link
Contributor Author

My initial thought was to have an API similar to cordova-plugin-local-notifications. Since it was the most popular cordova local notifications plugin I guess most of the teams migrating from cordova to capacitor are already familiar with.

But I guess you're right, most apps (like ours) won't need to configure the icon for each notification individually - so a global config makes sense.

So I pushed a new commit, which allows configuring the smallIcon in capacitor.config.json like:

  "plugins": {
    "LocalNotifications": {
      "smallIcon": "ic_stat_icon_config_sample"
    }
  }

but also keeps the previous functionality - allowing to override the default icon per notification.

Copy link
Member

@jcesarmobile jcesarmobile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You've put the icons on capacitor project instead of in the example project.
I've moved them to the proper place so those sample icons don't get distributed on every capacitor project.

Other than that, looks good to me, will merge once tests finish.

Thanks!

@jcesarmobile jcesarmobile merged commit f7c42db into ionic-team:master Aug 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Local Notifications icon cannot be changed
2 participants