This differential implements native Android module to remove notifications for a given thread. In future differentials we will use to to remove threadID <-> notificationID mapping from redux.
Details
Details
Build the app. Import this module in JavaScript and call this method. Passing any string should work (not crashing).
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Branch
- marcin/eng-2501
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| native/android/app/src/main/java/app/comm/android/notifications/CommAndroidNotifications.java | ||
|---|---|---|
| 28 | Can we use this in any version of OS? In CommNotificationsHandler we are checking if android.os.Build.VERSION.SDK_INT >= android.os.Build.VERSION_CODES.M. | |
| 31–32 | This is a strange name. A CharArray is basically a String, but I don't think it makes sense to include a type in a variable name. | |
| native/android/app/src/main/java/app/comm/android/notifications/CommAndroidNotificationsPackage.java | ||
| 22–26 | It's a good practice to avoid mutating things where possible. In this case, the code can be replaced by e.g. List.of or Collections.singletonList. Obviously, if the returned array may be mutated, we can't use these. | |
| native/android/app/src/main/java/app/comm/android/notifications/CommAndroidNotifications.java | ||
|---|---|---|
| 28 | This was added in API level 23. I will add the if sttement. | |
| 31–32 | This is my oversight. Initially I used getCharArray method instead of getString. | |
| native/android/app/src/main/java/app/comm/android/notifications/CommAndroidNotificationsPackage.java | ||
| 22–26 | This is required by react-native: https://reactnative.dev/docs/native-modules-android#register-the-module-android-specific | |
| native/android/app/src/main/java/app/comm/android/notifications/CommAndroidNotificationsPackage.java | ||
|---|---|---|
| 22–26 | At this moment I will introduce urgent changes from this differential (other comments), but I oblige myself to try compiling app with List.of instead of mutable ArrayList. If it works I will introduce the change. | |
| native/android/app/src/main/java/app/comm/android/notifications/CommAndroidNotifications.java | ||
|---|---|---|
| 38 ↗ | (On Diff #21239) | Is there any risk of a null pointer exception? Eg. is extras always set? |
Comment Actions
As for Android 5.0, yeah I agree we shouldn't worry about rescinds for such an old version
| native/android/app/src/main/java/app/comm/android/notifications/CommAndroidNotifications.java | ||
|---|---|---|
| 38 ↗ | (On Diff #21239) | Looking at the docs: https://developer.android.com/reference/android/app/Notification#extras, we don't have Nonnull annotation here, therefore the check is necessary. Thanks for catching. |