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

Making an option to not use large icon in PugNotification #20

Merged
merged 1 commit into from Jul 14, 2015

Conversation

burnermanx
Copy link

Not setting a default LargeIcon, to make an option to use or not use a large icon in notification. Otherwise I'll get an Android icon (R.drawable.pugnotification_ic_launcher) plus smallIcon in notification.

I tested it in Sample App and it works. Just comment .largeIcon() line in PugNotification builder to test.

Review on Reviewable

@halysongoncalves
Copy link
Owner

In my review, nothing visualized difference with change. By default Android already includes a LargeIcon default, which is the very image of his avatar. The tests were carried out on lollipop and kitkat.

If you can attach screenshot to see the difference @burnermanx !

@burnermanx
Copy link
Author

Above, notification only with small icon, without large icon. Below, notification with large icon and small icon.

It's common to have apps making notifications without large icon. In the third notification, there is a CloudMagic's notification only with small icon.

Example

@halysongoncalves
Copy link
Owner

Nice! Will generate a new version with this fix!

halysongoncalves pushed a commit that referenced this pull request Jul 14, 2015
Making an option to not use large icon in PugNotification
@halysongoncalves halysongoncalves merged commit 6f3f2f8 into halysongoncalves:master Jul 14, 2015
@halysongoncalves
Copy link
Owner

Hi @burnermanx ,
The Pull Request was accepted by tomorrow and the new version will be available pugnotification!

@burnermanx
Copy link
Author

Thanks! I'll check it out. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants