-
Notifications
You must be signed in to change notification settings - Fork 319
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
Prevent NavigationNotification update after unregistered #1118
Conversation
@danesfeder this would only fix the first crash reported in #1116 right? |
I've just 👀 #1116 (comment) Thanks @danesfeder 🙏 |
ef8ecec
to
ba02ce6
Compare
Tracking in #1119 |
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.
Added a minor comment (not blocking the PR).
Thanks @danesfeder
@@ -8,6 +8,7 @@ | |||
class NavigationNotificationProvider { | |||
|
|||
private NavigationNotification navigationNotification; | |||
private boolean isUpdating = true; |
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.
NIT - what about shouldUpdate
instead? It sounds better to me. By default, shouldUpdate
? Yes - true
After shutting down, shouldUpdate
? No - false
What do you think?
ba02ce6
to
2b233bc
Compare
2b233bc
to
3a70d7e
Compare
Closes #1116 Regression from #1066
Added a boolean to prevent notification updates after the notification has been shut down.