This differential brings back the second half of functionality we expect from tapping notification. When notification is tapped we start the app from intent stored in notification. CommAndroidNotificationEventEmitter listents for intents. If they come from notification, it sends parsed
version of this notification to JavaScript as an event. JavaScript uses this event to navigate to thread.
Details
Details
Build the app. Kill it. Send notification. Tap it. Ensure app opens to a thread. Repeat but without killing it - just put it into background.
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
The test plan should also include testing this functionality while the app is running / backgrounded.
| native/android/app/src/main/java/app/comm/android/notifications/CommAndroidNotificationsEventEmitter.java | ||
|---|---|---|
| 29 ↗ | (On Diff #21038) | Can we be consistent with our approach by introducing a new class instead of implementing the interface in CommAndroidNotificationsEventEmitter? |
| native/push/push-handler.react.js | ||
| 6 ↗ | (On Diff #21038) | Are we still using the library? |
| native/push/push-handler.react.js | ||
|---|---|---|
| 6 ↗ | (On Diff #21038) | As of this differential we are still using this library. We import getFirebase from firebase.js which uses this library. Future diffs will remove this dependency. |
| native/android/app/src/main/java/app/comm/android/notifications/CommAndroidNotificationsEventEmitter.java | ||
|---|---|---|
| 52–63 ↗ | (On Diff #21249) | It would be great if we could reuse the logic instead of copying the code. One option might be to save CommAndroidNotificationsActivityEventListener instance and call its onNewIntent method. We can also modify CommAndroidNotificationsActivityEventListener by introducing a new method that accepts only the RemoteMessage. |
| native/android/app/src/main/java/app/comm/android/notifications/CommAndroidNotificationsEventEmitter.java | ||
|---|---|---|
| 52–63 ↗ | (On Diff #21249) | I am not sure whether it is safe to call onNewIntent directly once we register it in reactContext since there may be several additional layers of abstraction between registering and calling onNewIntent that provide additional logics. But extraction to a separate private method is always a good option, so I will proceed this way. |
| native/android/app/src/main/java/app/comm/android/notifications/CommAndroidNotificationsEventEmitter.java | ||
|---|---|---|
| 69 ↗ | (On Diff #21365) | I don't think the function's name is correct now. We take a message from any intent, so it isn't necessarily initial |