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

Cancel notifications on opening notifications tab #450

Merged
merged 1 commit into from Nov 13, 2017

Conversation

Projects
None yet
2 participants
@charlag
Collaborator

charlag commented Nov 12, 2017

This closes #422.

@charlag charlag requested a review from connyduck Nov 12, 2017

@connyduck

This comment has been minimized.

Show comment
Hide comment
@connyduck

connyduck Nov 12, 2017

Member

There is already a clearNotifications method in BaseActivity, I think it is best to make it static, put it in NotificationMaker (rename NotificationMaker to NotificationManager) and reuse that one method everywhere notifications get cancelled.
Also, does the NotificationClearBroadcastReceiver get called when a notification is cancelled programmatically? If so clearNotifications doesn't need to change the SharedPreferences.

Member

connyduck commented Nov 12, 2017

There is already a clearNotifications method in BaseActivity, I think it is best to make it static, put it in NotificationMaker (rename NotificationMaker to NotificationManager) and reuse that one method everywhere notifications get cancelled.
Also, does the NotificationClearBroadcastReceiver get called when a notification is cancelled programmatically? If so clearNotifications doesn't need to change the SharedPreferences.

@charlag

This comment has been minimized.

Show comment
Hide comment
@charlag

charlag Nov 12, 2017

Collaborator

@connyduck oh, thanks, will fix it!
I'm not sure, should check.

Collaborator

charlag commented Nov 12, 2017

@connyduck oh, thanks, will fix it!
I'm not sure, should check.

@charlag

This comment has been minimized.

Show comment
Hide comment
@charlag

charlag Nov 12, 2017

Collaborator

I've updated it.
I've also checked NotificationClearBroadcastReceiver and it's not called when I cancel notifications programmatically.

Collaborator

charlag commented Nov 12, 2017

I've updated it.
I've also checked NotificationClearBroadcastReceiver and it's not called when I cancel notifications programmatically.

@connyduck connyduck merged commit ddc4954 into tuskyapp:master Nov 13, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment