Push Notifications #73
Conversation
|
Thank you Matt! My schedule has been quite busy as of late, but I'll be sure to review this soon! |
firebase.json
Outdated
There was a problem hiding this comment.
I believe this file contains our private keys Edit: turns out I was mistaken: it looks to have the appId in common with credentials.dart but fortunately not the API keys (and even then, based on firebase/flutterfire#7617 we're perhaps already being a bit overzealous with our use of the private submodule 🤷)
| @@ -6,6 +8,7 @@ class SettingsScreen extends StatelessWidget { | |||
|
|
|||
| @override | |||
| Widget build(BuildContext context) { | |||
|
|
|||
There was a problem hiding this comment.
(pretty sure the auto-formatter would remove this line)
| @@ -77,6 +83,7 @@ sealed class ThcUser { | |||
| final String? id; | |||
| final String? email; | |||
| final bool registered; | |||
| final bool? notify; | |||
There was a problem hiding this comment.
Storing a bool? value is a smart move 👍
| @@ -41,7 +41,8 @@ enum LocalStorage { | |||
| themeMode, | |||
| navBarSelection, | |||
| adminWatchLive, | |||
| adminStream; | |||
| adminStream, | |||
| notify; | |||
There was a problem hiding this comment.
I think it makes a lot of sense for stuff like display & notification settings to be saved in local storage.
With that in mind, I wonder if we could get rid of the class member in user.dart and make it work just using the LocalStorage.notify() value 🤔
There was a problem hiding this comment.
After glancing at the message from the most recent commit, it looks like there's a decently good reason for the setup being the way it is.
I think we're good to land this one, after 1 very important change…
Enables push notifications for upcoming livestreams that the user has signed up for.
I don’t believe there currently is a way through the app to sign up for a livestream, so for now you would have to add yourself to the
signupsub collection underscheduled_streamsmanually(screenshot)Then in the app settings, you can enable/disable notifications. (screenshot)
It’s currently set up to send a notification an hour before the livestream starts, but we can change that as needed.
There is more work that needs to be done on push notifications, but this should give us a good starting point I think.
All feedback welcome, thanks!