-
-
Notifications
You must be signed in to change notification settings - Fork 63
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
Revamped notification service #118
base: main
Are you sure you want to change the base?
Revamped notification service #118
Conversation
@@ -91,4 +91,7 @@ flutter { | |||
|
|||
dependencies { | |||
implementation "org.jetbrains.kotlin:kotlin-stdlib-jdk7:$kotlin_version" | |||
implementation 'androidx.window:window:1.0.0' | |||
implementation 'androidx.window:window-java:1.0.0' | |||
coreLibraryDesugaring 'com.android.tools:desugar_jdk_libs:1.2.2' |
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.
@ishanvaghani why are these needed?
Then, shouldn't we also add coreLibraryDesugaringEnabled true
as it says here: https://www.geeksforgeeks.org/desugaring-in-android/
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.
Have missed it. Adding
); | ||
} | ||
|
||
void cancelTodayNotification() async { |
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.
@ishanvaghani This is not working properly, this method is executed when adding a video for today, but the notification shows up (when it should be cancelled).
Future<void> rescheduleNotification(DateTime day) async { | ||
final TimeOfDay timeOfDay = getScheduledTime(); | ||
await scheduleNotification(timeOfDay.hour, timeOfDay.minute, day); | ||
int getNotificationId() { |
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.
Is there any reason to have a random id?
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.
We are scheduling 2 weeks notifications and storing this in shared pref and canceling notification based on notification id. So for unique identification using random id.
Hi @ishanvaghani , also, after changing to a profile and back to default profile, it schedule the notification for the day in a hour that has already passed (example: it is 5pm, the notification time in the settings is 10am, I changed the profiles and it scheduled for today at 10am.) |
@bianca7dev I have pushed one fix can you check notification is still coming after uploading video? secondly also check for changing profile still schedules invalid notification. |
Description
Revamped Notification Service
Checklist
Before you create this PR, please confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]
). This will ensure a smooth and quick review process.CONTRIBUTORS.md
file, if it wasn't already present.CHANGELOG.md
.