feat(Android): add all alarm types to trigger alarmManager#749
Conversation
|
Also I'm on windows and I didn't manage to use scripts or run E2E tests, I only managed to run jest tests |
Codecov Report
@@ Coverage Diff @@
## main #749 +/- ##
==========================================
+ Coverage 77.25% 77.33% +0.08%
==========================================
Files 34 34
Lines 1701 1711 +10
Branches 566 569 +3
==========================================
+ Hits 1314 1323 +9
- Misses 337 338 +1
Partials 50 50 |
|
Oh I see I have to write a db migration, I'm not sure if I can do that |
|
again: Oh I see I don't have to 😅 the trigger object is saved in bytes and is retrieved at some point later |
|
thanks for the contribution 💯 very grateful. would you be able to add some more notes about the api changes or what the api would now look like i.e. how would a developer use this new functionality? It will help when reviewing the p/r. OK, I've looked more into your p/r and I love the changes! To help with the review, can you give some more detail on why you need this change and/or a use case(s)? |
| notificationModel.getId().hashCode(), | ||
| launchActivityIntent, | ||
| mutabilityFlag); | ||
| AlarmManagerCompat.setAlarmClock( |
There was a problem hiding this comment.
why do you need to set pendingLaunchIntent for type SET_ALARM_CLOCK and not for the other types?
There was a problem hiding this comment.
from what I remember, when using the setAlarmClock api, it may show an alarm icon in some versions and devices, that when you touch the icon, it will use the pending intent. and so I remember it being mandatory. I'm not sure if it is, but as far as I remember, it was required.
There was a problem hiding this comment.
so Even if it is not required, I'd like the icon to launch the app that has caused it to appear (as it is common UX for alarm apps I believe).
|
well I use notifee in my app (al-azan) as the core functionality of setting alarms for specific times (to make it super simple, my app is an alarm clock that adjusts It's time everyday) the reason for different alarm type is because depending on how you use them, they have different set of behaviors. and I think having |
|
also I remember adding some notes to the library for the new usage, correct me if I have forgotten to also I tried to make it a non breaking change, this shouldn't break existing apps |
|
when can this PR be merged ? 🙏🏻 @helenaford |
|
Hello 👋, this PR has been opened for more than 2 months with no activity on it. If you think this is a mistake please comment and ping a maintainer to get this merged ASAP! Thanks for contributing! You have 15 days until this gets closed automatically |
|
comment |
|
thank you, merged 🙏 |
| //... | ||
| alarmManager: { | ||
| allowWhileIdle: true, | ||
| type: AlarmType.SET_EXACT_AND_ALLOW_WHILE_IDLE, |
There was a problem hiding this comment.
from https://developer.android.com/develop/background-work/services/alarms/schedule#repeating:
A poorly-designed alarm can cause battery drain and put a significant load on servers. For this reason, on Android 4.4 (API level 19) and higher, all repeating alarms are [inexact alarms](https://developer.android.com/develop/background-work/services/alarms/schedule#inexact).
Would be nice if this is documented somewhere
resolves #655
hi @helenaford
I tried to change all places I thought I should for this change, still I may have missed something
please let me know if there's any problem