fix: isAppInForeground #603
Conversation
Fixes issue with registering: WARN registerHeadlessTask or registerCancellableHeadlessTask called multiple times for same key 'app.notifee.foreground-service-headless-task' WARN registerHeadlessTask or registerCancellableHeadlessTask called multiple times for same key 'app.notifee.notification-event' WARN [notifee] no background event handler has been set. Set a handler via the "onBackgroundEvent" method.
This reverts commit c1abf82.
|
|
| return true; | ||
| ReactContext reactContext = getReactContext(); | ||
| if (reactContext == null) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Not sure what the purpose of this code was, but it was always failing with ClassCastException
There was a problem hiding this comment.
mmm, this is a part of code I've not dealt with much. What you've done with using getReactContext() makes sense
| @@ -4,8 +4,8 @@ import { | |||
| AndroidLaunchActivityFlag, | |||
| AndroidCategory, | |||
| AndroidImportance, | |||
| AndroidFlags, | |||
There was a problem hiding this comment.
This fixes double loading of module which caused that onBackgroundEvent was not properly registered
There was a problem hiding this comment.
thank you, this is a fix for the testing app only, correct?
| // Manual steps: | ||
| // 1. Minimize app | ||
| // 2. Open notification drawer | ||
| // 3. Tap on 'First Action' | ||
| // 4. Make sure you see: | ||
| // > WARN Received a ACTION_PRESS Background event in JS mode. | ||
| // > WARN Notification Cancelled first_action |
There was a problem hiding this comment.
Not sure how you guys test notification tapping. On Android it is possible to simulate some actions with adb but that's not robust since you need to know exact point for tapping, e.g.:
adb shell input keyevent KEYCODE_HOME
adb shell cmd statusbar expand-notifications
adb shell input tap 990 1269Note that on Android 9 this test passes without the fix in isAppInForeground.
On Android 12, however, the fix is necessary for this test to pass.
Hence the fix is needed for this test to pass on both Android 7 and Android 12
Codecov Report
@@ Coverage Diff @@
## main #603 +/- ##
=======================================
Coverage 77.24% 77.24%
=======================================
Files 34 34
Lines 1700 1700
Branches 546 546
=======================================
Hits 1313 1313
Misses 386 386
Partials 1 1 |
|
@mieszko4 thank you for this fix! And, I appreciate the thorough testing and detailed explanation! |
This PR fixes two issues:
test_react_native/example.Because
AndroidFlagswas imported with'@notifee/react-native/src'and everything else was imported'@notifee/react-native'it caused double initialization of NotifeeApiModule which caused the following errors showing whenyarn tests_rn:android:testis run:isAppInForeground.The issue started to show in Android 12 because that version Android for some reason considers
IMPORTANCE_FOREGROUNDfor importance of the running app even if app is minimized.Closes #404