Conversation
…cused DM and thread-reply desktop notifications are suppressed for the currently selected channel, but the check only compared channel ids — it ignored whether the window was actually focused. Leaving a DM conversation open and minimizing the app silently dropped every notification for that conversation, the one the user is most likely waiting on. Gate the suppression on isAppFocused(): a channel selected in a blurred or hidden window is not being viewed, so notifications for it fire again. The notifyForActiveChannel opt-in still bypasses suppression entirely. The decision is extracted into a pure isSuppressedAsActiveChannel() predicate shared by both the DM and thread-reply paths, with regression tests. Signed-off-by: mdschoff <mdschoff@gmail.com>
🔐 Codex Security Review
|
This file contains hidden or 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
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.
Problem
DM and thread-reply desktop notifications are suppressed for the currently selected channel, but the check only compares channel ids — it never asks whether the app window is actually focused. Leaving a DM conversation selected and minimizing (or backgrounding) the app silently drops every notification for that conversation — the one the user is most likely waiting on.
useLiveChannelUpdates.tspreviously:How it was implemented
isSuppressedAsActiveChannel()indesktop/src/features/channels/activeChannelSuppression.ts: a channel only counts as "being viewed" when it is the active channel and the app window is focused, using the existingisAppFocused()utility fromshared/lib/useDocumentVisible.ts.useLiveChannelUpdates.ts(the DM path and the thread-reply desktop-notification path) now share the predicate, evaluated at event-delivery time.notifyForActiveChannel(notify-while-viewing) opt-in still bypasses suppression entirely, and behavior while the app is focused is unchanged.activeChannelSuppression.test.mjs, including the minimized-window case.How to test manually
No UI changes — behavior-only, so no screenshots.
Verification
desktop: pnpm test— 5,804 pass (5 new)desktop: pnpm typecheck,pnpm check,pnpm build— cleanRelated
Searched open PRs and issues for duplicates — none found for this specific bug. Closest neighbors: #6276 (mentions don't notify while unfocused — different mechanism, the home-feed poll pausing) and the notify-while-viewing setting added in #753. Follow-up candidate deferred to a future PR: fixing #6276 itself.