Skip to content

Commit 2ae6f79

Browse files
committed
ios: Fix bug where tapping a notification didn't navigate to conversation
For no apparent reason, the library we've been using for iOS notifications (@react-native-community/push-notification-ios) sends remote notification response events, including the simple response of tapping on a remote notification, with the name 'localNotification' instead of the name they document, which is 'notification': https://github.com/react-native-push-notification/ios/blob/v1.10.1/README.md#how-to-determine-push-notification-user-click (For what their 'notificaton' event actually means, see a recent commit where we removed our handler for it.) Rather than giving in and just listening for 'localNotification', set up our own native module to forward the event that iOS sends to the app. This took some time, but it makes the logic more transparent and removes some dependence on that library, which are nice outcomes. There was some custom `RCTConvert` and JavaScript munging to read through in the library's codepath, but thankfully the part that affects what we actually use is small: just a call to `RCTJSONClean`. Fixes: zulip#5679
1 parent 70a7e90 commit 2ae6f79

File tree

5 files changed

+95
-12
lines changed

5 files changed

+95
-12
lines changed

ios/ZulipMobile/AppDelegate.mm

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@
1111

1212
#import <React/RCTAppSetupUtils.h>
1313

14+
#import "ExpoModulesCore-Swift.h"
15+
#import "ZulipMobile-Swift.h"
16+
1417
#if RCT_NEW_ARCH_ENABLED
1518
#import <React/CoreModulesPlugins.h>
1619
#import <React/RCTCxxBridgeDelegate.h>
@@ -119,8 +122,9 @@ - (void)userNotificationCenter:(UNUserNotificationCenter *)center
119122
didReceiveNotificationResponse:(UNNotificationResponse *)response
120123
withCompletionHandler:(void (^)(void))completionHandler
121124
{
122-
[RNCPushNotificationIOS didReceiveNotificationResponse:response];
123-
completionHandler();
125+
[ZLPNotificationsEvents userNotificationCenter:center
126+
didReceive:response
127+
withCompletionHandler:completionHandler];
124128
}
125129

126130
// Called when a notification is delivered to a foreground app.

ios/ZulipMobile/ZLPNotifications.swift

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,47 @@
11
import Foundation
22
import UIKit
33
import React.RCTBridgeModule
4+
import React.RCTConvert
5+
import React.RCTEventEmitter
6+
7+
@objc(ZLPNotificationsEvents)
8+
class ZLPNotificationsEvents: RCTEventEmitter {
9+
static var currentInstance: ZLPNotificationsEvents? = nil
10+
11+
override func startObserving() -> Void {
12+
super.startObserving()
13+
ZLPNotificationsEvents.currentInstance = self
14+
}
15+
16+
override func stopObserving() -> Void {
17+
ZLPNotificationsEvents.currentInstance = nil
18+
super.stopObserving()
19+
}
20+
21+
@objc
22+
override func supportedEvents() -> [String] {
23+
return ["response"]
24+
}
25+
26+
@objc
27+
class func userNotificationCenter(
28+
_ center: UNUserNotificationCenter,
29+
didReceive response: UNNotificationResponse,
30+
withCompletionHandler completionHandler: () -> Void
31+
) {
32+
currentInstance?.sendEvent(
33+
withName: "response",
34+
35+
// The RCTJSONClean was copied over from
36+
// @react-native-community/push-notification-ios; possibly we don't
37+
// need it.
38+
body: RCTJSONClean(
39+
response.notification.request.content.userInfo
40+
)
41+
)
42+
completionHandler()
43+
}
44+
}
445

546
@objc(ZLPNotificationsStatus)
647
class ZLPNotificationsStatus: NSObject {

ios/ZulipMobile/ZLPNotificationsBridge.m

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
11
#import "React/RCTBridgeModule.h"
2+
#import "React/RCTEventEmitter.h"
23

3-
// Register the ZLPNotificationsStatus implementation with React Native
4-
// needed because ZLPNotificationsStatus is in Swift:
4+
// Register our Swift modules with React Native:
55
// https://reactnative.dev/docs/0.68/native-modules-ios#exporting-swift
6+
7+
@interface RCT_EXTERN_MODULE(ZLPNotificationsEvents, RCTEventEmitter)
8+
@end
9+
610
@interface RCT_EXTERN_MODULE(ZLPNotificationsStatus, NSObject)
711

812
RCT_EXTERN_METHOD(areNotificationsAuthorized:

src/notification/NotificationListener.js

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,23 @@
11
/* @flow strict-local */
2-
import { DeviceEventEmitter, Platform } from 'react-native';
2+
import { DeviceEventEmitter, Platform, NativeModules, NativeEventEmitter } from 'react-native';
33
import type { PushNotificationEventName } from '@react-native-community/push-notification-ios';
44
import PushNotificationIOS from '@react-native-community/push-notification-ios';
5+
import invariant from 'invariant';
56

67
import type { JSONableDict } from '../utils/jsonable';
78
import type { GlobalDispatch } from '../types';
89
import { androidGetToken, handleDeviceToken } from './notifTokens';
910
import type { Notification } from './types';
1011
import * as logging from '../utils/logging';
12+
import { fromAPNs } from './extract';
1113
import { narrowToNotification } from './notifOpen';
1214

15+
// TODO: Could go in a separate file, with some thin wrapper perhaps.
16+
const iosNativeEventEmitter =
17+
Platform.OS === 'ios'
18+
? new NativeEventEmitter<{| +response: [JSONableDict] |}>(NativeModules.ZLPNotificationsEvents)
19+
: null;
20+
1321
/**
1422
* From ios/RNCPushNotificationIOS.m in @rnc/push-notification-ios at 1.2.2.
1523
*/
@@ -42,11 +50,26 @@ export default class NotificationListener {
4250
}
4351

4452
/** Private. */
45-
// prettier-ignore
4653
listenIOS(
4754
args:
48-
{| +name: PushNotificationEventName, +handler: (...empty) => void | Promise<void> |}
55+
| {| +name: PushNotificationEventName, +handler: (...empty) => void | Promise<void> |}
56+
| {| +name: 'response', +handler: JSONableDict => void |},
4957
) {
58+
invariant(
59+
iosNativeEventEmitter != null,
60+
'NotificationListener: expected `iosNativeEventEmitter` in listenIOS',
61+
);
62+
63+
if (args.name === 'response') {
64+
const { name, handler } = args;
65+
const sub = iosNativeEventEmitter.addListener(name, handler);
66+
this.unsubs.push(() => sub.remove());
67+
return;
68+
}
69+
70+
// TODO: Use iosNativeEventEmitter (as above) instead of
71+
// PushNotificationIOS (as below) for all iOS events
72+
5073
// In the native code, the PushNotificationEventName we pass here
5174
// is mapped to something else (see implementation):
5275
//
@@ -96,6 +119,16 @@ export default class NotificationListener {
96119
this.listenAndroid('notificationOpened', this.handleNotificationOpen);
97120
this.listenAndroid('remoteNotificationsRegistered', this.handleDeviceToken);
98121
} else {
122+
this.listenIOS({
123+
name: 'response',
124+
handler: payload => {
125+
const dataFromAPNs = fromAPNs(payload);
126+
if (!dataFromAPNs) {
127+
return;
128+
}
129+
this.handleNotificationOpen(dataFromAPNs);
130+
},
131+
});
99132
this.listenIOS({ name: 'register', handler: this.handleDeviceToken });
100133
this.listenIOS({ name: 'registrationError', handler: this.handleIOSRegistrationFailure });
101134
}

src/notification/extract.js

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,12 @@ export const fromAPNsImpl = (data: ?JSONableDict): Notification | void => {
3838
//
3939
// For the format this parses, see `ApnsPayload` in src/api/notificationTypes.js .
4040
//
41-
// Though what it actually receives is more like this:
41+
// Though in one case what it actually receives is more like this:
4242
// $Rest<ApnsPayload, {| aps: mixed |}>
43-
// because the `ApnsPayload` gets parsed by the `PushNotificationIOS`
44-
// library, and what it gives us through `getData` is everything but the
45-
// `aps` property.
43+
// That case is the "initial notification", a notification that launched
44+
// the app by being tapped, because the `PushNotificationIOS` library
45+
// parses the `ApnsPayload` and gives us (through `getData`) everything
46+
// but the `aps` property.
4647

4748
/** Helper function: fail. */
4849
const err = (style: string) =>
@@ -160,7 +161,7 @@ export const fromAPNsImpl = (data: ?JSONableDict): Notification | void => {
160161
*
161162
* @returns A `Notification` on success; `undefined` on failure.
162163
*/
163-
const fromAPNs = (data: ?JSONableDict): Notification | void => {
164+
export const fromAPNs = (data: ?JSONableDict): Notification | void => {
164165
try {
165166
return fromAPNsImpl(data);
166167
} catch (errorIllTyped) {

0 commit comments

Comments
 (0)