Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ios [nfc]: Cut notifications library out of handling "initial notification" #5740

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

chrisbobbe
Copy link
Contributor

I'm thinking that for iOS notifications in zulip-flutter (zulip/zulip-flutter#122, etc.), it'll help to demonstrate a working implementation that doesn't depend on @react-native-community/push-notification-ios, since that library's code can be unnecessarily opaque, and also buggy.

#5676 took a first step toward that, and this takes another step. (The former was about tapping a notification when the app is already open; this PR is in the code for tapping a notification when the app is closed.)

@chrisbobbe
Copy link
Contributor Author

Tested on my iPhone 13 Pro running iOS 16.

…ation"

And handle it ourselves, through our dedicated iOS native module
ZLPNotificationsStatus.

(The "initial notification" is a notification that, by being tapped,
launched the app from cold. PR zulip#5676 was about notifications that
are tapped when the app is already open, and that PR fixed a known
bug.)

This, like zulip#5676, makes our iOS notification handling more
transparent. My hope is to smooth the path to custom iOS
notification code in zulip-flutter (zulip-flutter#122, etc.) by
demonstrating a working implementation that's simple and avoids
depending on the details of
@react-native-community/push-notification-ios.

Cutting the library out of this codepath makes one small difference
in the payload we pass to `fromAPNs` on the JS side: the `aps`
property is now present. (So, remove a comment saying it might be
absent). We've never looked at that property, so this is NFC. Unlike
in 2ae6f79 / zulip#5676, there wasn't a call to the library's custom
RCTConvert extensions to account for, as we did there with a
cautious call to RCTJSONClean.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant