notif: Support iOS! #409
Merged
notif: Support iOS! #409
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is one of the steps required for showing push notifications, as helpfully laid out here: https://firebase.flutter.dev/docs/messaging/apple-integration/
This is one of the steps required for showing push notifications (when the app isn't already running, anyway), as helpfully laid out here: https://firebase.flutter.dev/docs/messaging/apple-integration/
Fixes: zulip#321
gnprice
added
a-iOS
chrisbobbe
approved these changes
Nov 22, 2023
| /// These values are similar to [kFirebaseOptionsAndroid] but are for iOS, | ||
| /// and they let us initialize the Firebase library so that we can do that. | ||
| /// | ||
| /// TODO: Cut out Firebase for APNs and use a thinner platform-API binding. |
There was a problem hiding this comment.
/// TODO: Cut out Firebase for APNs and use a thinner platform-API binding.Oh good, I think this might be wise to do eventually. 🙂
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #321.
There's one notable limitation in the functionality here, which I've filed as:
Also the permissions prompt comes annoyingly early:
We can fix those in the Beta 2 period.
I've manually tested this with my own Zulip dev server (running main), and it works. Testing notifications with a dev version of the client is kind of a pain, unfortunately (even after the simplifications I made to it this month):
https://github.com/zulip/zulip-mobile/blob/main/docs/howto/push-notifications.md#ios
so for review, given the impending Beta 1, I think it's best to skip that for now — for manual testing we can just rely on what I've done already, and on the next alpha release.
Screenshot of the notifications in Notification Center:
(There were also messages with content "4", "2", and "0". They didn't produce notifications because of #408 -- the app was open in the foreground at the time.)
(The icon needs to be replaced; that's #390.)