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(local-notifications): use app icon for default #1510

Closed
wants to merge 1 commit into from

Conversation

mburger81
Copy link

waiting for custom icons we suggest to use at least default application icon before using ic_dialog_info

waiting for custom icons we suggest to use at least default application icon before using ic_dialog_info
@jcesarmobile
Copy link
Member

the problem with using app icon is, most of them are not valid notification icons and will just show a white circle or square

@mburger81
Copy link
Author

On what is this depending? The icon it self, or the android platform? In our test case the app icon is simple better then the default info icon.

We could also do a PR for a custom small icon like cordova does. But we are not sure where should we should configure it?
Do you want it As I a dedicated main property or in the extra property? For what is the extra property? There is no documentation about that.
We are not native programmers so sorry if this this is a "stupid" question

@RoderickQiu
Copy link
Contributor

RoderickQiu commented Jul 29, 2019

the problem with using app icon is, most of them are not valid notification icons and will just show a white circle or square

But on my own project, it shows the app's icon, if action bar icons are not set. (After using the code) (Tested only on HUAWEI EMUI 9 (Android P))

So it should be OK to merge that code?

@RoderickQiu
Copy link
Contributor

However, I think request icon as a parameter seems to be better.
But I'm not experienced in native coding...

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.

3 participants