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

Add option to snooze until next arrival/departure #2390

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tomato4
Copy link

@tomato4 tomato4 commented Jun 23, 2023

This change will allow users to snooze notifications for location-based tasks (Issue #2013).

Since the geofence record stays even after triggering, it's only needed to dismiss the notification. If the user arrives again at the given location, the task notification will trigger again.

Possible improvements:

  • Only show "Next arrival/departure" option when the task has actually location set
    • This would require fetching the task model in SnoozeDialog
  • Only show this option when AndroidGeofenceTransitionIntentService actually triggered the notification
    • This would require passing reason/source of the notification trigger which is not implemented

Let me know if you think these two improvements should be implemented...

@abaker
Copy link
Member

abaker commented Jul 16, 2023

Hey, sorry for the late response!

So conceptually this would be a little different than the existing 'Snooze' functionality. When you snooze a task you will receive a new notification when the snooze period lapses, and any other reminders (time or location) that occur before the snooze period are ignored

This PR would effectively be the same thing as dismissing the notification, and any time based notifications would be free to fire. Maybe there should be a 'Dismiss' button in the notification dialog, although there could be some confusion with this (is it dismissing the task notification, or dismissing the notification dialog?). What do you think?

I wouldn't be opposed to actually snoozing until the next time you visit a location, but it should probably behave like other snooze options. In other words it should block reminders until you visit the location again, and show a snooze status in the edit screen so the snooze can be cancelled. Unfortunately this would be quite a bit more involved

@tomato4
Copy link
Author

tomato4 commented Jul 17, 2023

Hey, no worries.

So if the user sets both time and location-based reminders and the location one is triggered and snoozed, should the time-based reminder be ignored? Just making sure if this is the right concept...

If so I will implement it in the future (probably in August - currently I'm on vacation with a slow laptop)

Maybe there should be a 'Dismiss' button in the notification dialog, although there could be some confusion with this (is it dismissing the task notification, or dismissing the notification dialog?). What do you think?

I would remove this option from the snooze dialog. Conceptually it does make sense to not have this feature because it forces the user to do something with the notification - either snooze or complete the task. If the user does not want that, he can disable persistent notifications in the options.
So this option only made sense for location snooze, but that will have specific behavior.

When you snooze a task you will receive a new notification when the snooze period lapses, and any other reminders (time or location) that occur before the snooze period are ignored

Are you sure about the ignoring location-based reminder while the reminder is snoozed? I looked into the code and didn't find any cancellation of the location-based reminder (geofence)... I can't test it now in Android Studio, but I will try it with the phone later just to be sure it really works that way. :)

@tomato4 tomato4 marked this pull request as draft July 17, 2023 18:53
@2hj6zb9d5
Copy link

#2274

Copy link

@2hj6zb9d5 2hj6zb9d5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link

@2hj6zb9d5 2hj6zb9d5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@2hj6zb9d5
Copy link

#2442

Copy link

@2hj6zb9d5 2hj6zb9d5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link

@2hj6zb9d5 2hj6zb9d5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants