fix: reply notifs sometimes destroyed too early #25086
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
codebytere
force-pushed
the
fix-notifications-getting-lost
branch
from
2fc675f
to
1d25210
Compare
MarshallOfSound
approved these changes
Aug 21, 2020
nornagon
reviewed
Aug 21, 2020
|
@nornagon same behavior as a non-reply notification which is not directly interacted with - it disappears from view on its own. |
codebytere
force-pushed
the
fix-notifications-getting-lost
branch
from
1d25210
to
1609a7d
Compare
codebytere
force-pushed
the
fix-notifications-getting-lost
branch
from
7cc50ae
to
c1770ef
Compare
zcbenz
approved these changes
Aug 23, 2020
|
Release Notes Persisted
|
This was referenced Aug 24, 2020
|
I have automatically backported this PR to "10-x-y", please check out #25100 |
|
I have automatically backported this PR to "9-x-y", please check out #25101 |
|
I have automatically backported this PR to "8-x-y", please check out #25102 |
MarshallOfSound
added a commit
that referenced
this pull request
Sep 1, 2020
This reverts commit bea6c9e.
zcbenz
pushed a commit
that referenced
this pull request
Sep 2, 2020
MarshallOfSound
added a commit
that referenced
this pull request
Oct 23, 2020
MarshallOfSound
added a commit
that referenced
this pull request
Oct 23, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of Change
Closes #25062.
Fixes an issue where notifications with a reply button could potentially be destroyed too early when a user clicked on the notification body before replying, which meant that when they did finally reply the reply callback would never be called. This happened because we were destroying the notification on any activation type, and it's possible for reply-type notifications to be activated more than once. This fixes that by ensuring that reply-type notifications are only destroyed when the activation type is
NSUserNotificationActivationTypeReplied.Tested with https://gist.github.com/60a2cff9d13903bf538ebfc3c8134f0f.
cc @MarshallOfSound @zcbenz
Checklist
npm testpassesRelease Notes
Notes: Fixed an issue where notifications with a reply button could potentially be destroyed too early when a user clicked on the notification body before replying.