-
Notifications
You must be signed in to change notification settings - Fork 18
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
Add background workers functionality and notification background support #645
Conversation
Co-authored-by: Sirze01 <Sirze01@users.noreply.github.com>
Co-authored-by: Sirze01 <Sirze01@users.noreply.github.com>
… shows when at least the app opens
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apart from testing on IOS it seems to be going well. Please address and submit some feedback on the comments I made.
uni/lib/controller/backgroundWorkers/notifications/tuition_notification.dart
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it it is good to go. Any news on the IOS side of things? @thePeras
…os run in the main isolate
It appears to working as expected! The only thing lasting is to discuss if the notifications are to be defaulted enable or not, as it can be annoying to be asked to accept notifications launching the app for the first time. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a very complex issue, so congratulations on the results! However, the code is still far from ready regarding readability and tidiness. I have left some suggestions; on top of that, please review all the changes and format the code properly.
I'll test this more extensively when you get the chance to improve on this, please let me know then.
uni/lib/controller/background_workers/notifications/tuition_notification.dart
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please format all files properly with dart format to remove blank lines. I have also left some quick suggestions.
uni/lib/controller/background_workers/notifications/tuition_notification.dart
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm
Initial work for displaying notifications when even UNI is closed
Add workerCallback (iOS and android support)
Add NotificationManager (android support only for now, because I need a macbook to complete background worker support).
Review checklist
whatsnew/whatsnew-pt-PT
changelog.md
with the change