-
Notifications
You must be signed in to change notification settings - Fork 8.3k
[expo-notifications] Badge count module #6869
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
Conversation
239d2b0
to
a3029b6
Compare
315287c
to
eda53b8
Compare
eda53b8
to
da8f285
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My only comment here is instead of rejecting/throwing when we're unable to set the badge count, I wonder if we should just resolve the promise with false
(and with true
when we are successful). In general I think rejecting should be reserved for actual errors and exceptions, and here we expect a fair amount of devices in production to have this failure -- which developers have no control over. Also, there's not really any alternative to this behavior so I think most developers would just catch the error and then do nothing. (or have an unhandled promise rejection) --> resolving with false
is better and people can handle it if they want to.
This would also take care of the weird test case -- we can just expect either true or false and the test will only fail if an actual exception occurs.
da8f285
to
6d9afc1
Compare
Following your suggestion I have changed the
|
It uses `badgin` which we already use in `expo`.
6d9afc1
to
b467006
Compare
This does not work on Android, but returns a promise with "true": Notifications.setBadgeCountAsync(0); |
Why
We want to let people set badge counter on demand.
How
ShortcutBadger
applicationIconBadgeNumber
badgin
API
Test Plan
Tested the module on iOS 13 (worked ✅), Android 7.1 (Samsung, default launcher, 👎), Android 7.1 (Samsung, ADW Launcher, ✅), Android 9 (Nokia, default launcher, 👎), web (worked ✅). In order to be able to work around unsupported launchers I added a simple infrastructure inside
NewNotifications.js
test suite to skip tests on unsupported devices.