Skip to content

Notifications: keep the bell badge live while the panel is closed - #112080

Closed
arthur791004 wants to merge 7 commits into
trunkfrom
add/notifications-live-badge
Closed

Notifications: keep the bell badge live while the panel is closed#112080
arthur791004 wants to merge 7 commits into
trunkfrom
add/notifications-live-badge

Conversation

@arthur791004

@arthur791004 arthur791004 commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Fixes DOTMSD-1208

Proposed Changes

  • Keep the unread notifications bell badge live while the panel is closed, on the surfaces that mount the panel only when open (redesigned masterbar, Dashboard, Reader header).
  • Add a small, panel-independent subscriber that reuses the app's existing note stream — a real-time websocket push with a polling fallback — and reports the count of notes newer than the server's last-seen time.
  • The masterbar shows the exact live count; Dashboard and Reader use it as a yes/no indicator. While the panel is open it still owns the count; the subscriber only runs while closed.

Why are these changes being made?

The notifications app subscribes to the note stream and computes the unread count, but it only runs while the panel is open. The Dropdown surfaces unmount it on close, so the bell froze at its last value until the user reopened the panel — notifications arriving while it was closed weren't reflected. This keeps the badge live by reusing the same note-stream and count logic, decoupled from the panel UI.

Considerations

  • Reuses the existing detection. The panel already decides "new note → flip the badge" from the note stream plus a notifications poll, counting notes newer than the last-seen time; this mirrors that. An earlier iteration polled the user's has_unseen_notes flag, but nothing else polls it and it's only a boolean, so this matches the established path and keeps the exact count.
  • A standalone subscriber rather than the existing client. The panel's client is tightly coupled to its Redux store, pagination, and note bodies, so reusing it wholesale isn't practical; this shares only the underlying requests.
  • A plain subscribe/unsubscribe function, not a hook. One consumer is a class component, and the Dashboard can't statically import from the notifications app — a function works for all three where a hook wouldn't.
  • Standalone widget untouched. It keeps the app mounted and already runs the note stream, so its badge stays live without this.

Testing Instructions

  • On a surface with the new notifications app (Dashboard, Reader, or the masterbar with notifications/redesign), leave the panel closed and trigger a notification from another account (like/comment).
  • Confirm the bell updates without opening the panel — near-instant via the websocket, or within ~30s if websockets are unavailable — and that the masterbar shows the exact count.
  • Open the panel and confirm the count/seen state is correct, and that closing keeps it live.

Pre-merge Checklist

  • Has the general commit checklist been followed? (PCYsg-hS-p2)
  • Have you written new tests for your changes?
  • Have you tested the feature in Simple (P9HQHe-k8-p2), Atomic (P9HQHe-jW-p2), and self-hosted Jetpack sites (PCYsg-g6b-p2)?
  • Have you checked for TypeScript, React or other console errors?
  • For UI changes, have you tested the affected components in dark mode?
  • Have you tested accessibility for your changes? Ensure the feature remains usable with various user agents (e.g., browsers), interfaces (e.g., keyboard navigation), and assistive technologies (e.g., screen readers) (PCYsg-S3g-p2).
  • Have you used memoizing on expensive computations? More info in Memoizing with create-selector and Using memoizing selectors and Our Approach to Data
  • Have we added the "[Status] String Freeze" label as soon as any new strings were ready for translation (p4TIVU-5Jq-p2)?
    • For UI changes, have we tested the change in various languages (for example, ES, PT, FR, or DE)? The length of text and words vary significantly between languages.
  • For changes affecting Jetpack: Have we added the "[Status] Needs Privacy Updates" label if this pull request changes what data or activity we track or use (p4TIVU-aUh-p2)?

@arthur791004 arthur791004 self-assigned this Jun 29, 2026
@github-actions

Copy link
Copy Markdown

Looks like one of the E2E tests has failed.

You can fix them following these steps:

  1. Check out this branch locally:
    gh pr checkout 112080
  2. Start Claude Code in the repo:
    claude
  3. Run the /fix-e2e-tests skill, passing this PR number:
    /fix-e2e-tests 112080
    

@arthur791004
arthur791004 marked this pull request as ready for review June 29, 2026 08:57
@arthur791004
arthur791004 requested review from a team as code owners June 29, 2026 08:57
@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jun 29, 2026
Comment thread client/notifications/index.jsx Outdated
locale={ localeSlug }
actionHandlers={ this.actionHandlers }
closePanel={ this.closePanel }
setUnseenCount={ this.props.setUnseenCount }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[fix here] Issue: Prop name mismatch — this passes setUnseenCount but RedesignedNotifications destructures updateUnseenCount (line 115). The callback will be undefined at runtime, so the websocket push will throw a TypeError when it tries to call updateUnseenCount( count ).

Suggestion: Rename the prop to match the destructured name:

Suggested change
setUnseenCount={ this.props.setUnseenCount }
updateUnseenCount={ this.props.setUnseenCount }

Source: general correctness — the prop name at the call site must match the destructured name in the receiving component.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch — fixed in 13391b0. Renamed the call-site prop to updateUnseenCount to match the destructured name, so the note-stream callback resolves.

Comment thread apps/notifications/src/app/unseen-notifications.ts
arthur791004 and others added 7 commits July 9, 2026 14:02
The Dropdown consumers (redesigned masterbar, Dashboard, Reader) mount
the notifications app only while the panel is open, so the unread bell
badge froze once closed — the app's RestClient stopped polling.

Add a panel-independent poller in the notifications app
(subscribeUnseenNotifications) that polls the user's has_unseen_notes
flag and reports changes. It's a plain subscribe/unsubscribe function,
not a hook, so it works from the masterbar class component and can be
dynamically imported by the Dashboard (which disallows static imports
from the app). Each consumer wires it to its own badge while closed; the
open panel keeps driving the exact count via APP_RENDER_NOTES.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Use api-core's fetchUser() in the background poller instead of a
hand-rolled /me request, reusing the canonical /me fetcher and the typed
User. Drops the wpcom argument since the fetcher uses the shared client.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace the /me has_unseen_notes poll with a slim subscriber that reuses
the panel's existing note-stream mechanism: a pinghub websocket push
(real-time) with a /notifications poll fallback, deriving the unseen
count from notes newer than the server's last-seen time. This gives the
closed badge a real-time, exact count instead of a 60s boolean, matching
how the open panel already decides the count. Drops the api-core dep.

Masterbar shows the exact live count; Dashboard/Reader map it to their
boolean indicator.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
We only need to know whether there are unseen notes (and a small badge
count), so fetch 10 instead of 100.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The call site passed setUnseenCount but RedesignedNotifications reads
updateUnseenCount, so the live-count callback was undefined and the
note-stream push threw. Match the prop name.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@arthur791004
arthur791004 force-pushed the add/notifications-live-badge branch from 13391b0 to a56de88 Compare July 9, 2026 07:44
@matticbot

Copy link
Copy Markdown
Contributor

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

App Entrypoints (~109 bytes added 📈 [gzipped])

Details
name                    parsed_size           gzip_size
entry-dashboard-dotcom       +345 B  (+0.0%)     +109 B  (+0.0%)
entry-dashboard-ciab         +345 B  (+0.0%)     +109 B  (+0.0%)
entry-dashboard-a4a          +345 B  (+0.0%)     +109 B  (+0.0%)
entry-subscriptions           +76 B  (+0.0%)      +23 B  (+0.0%)
entry-stepper                 +76 B  (+0.0%)      +23 B  (+0.0%)
entry-reauth-required         +76 B  (+0.0%)      +23 B  (+0.0%)
entry-main                    +76 B  (+0.0%)      +23 B  (+0.0%)
entry-login                   +76 B  (+0.0%)      +23 B  (+0.0%)
entry-domains-landing         +76 B  (+0.0%)      +23 B  (+0.0%)
entry-browsehappy             +76 B  (+0.0%)      +23 B  (+0.0%)

Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used.

Sections (~86 bytes added 📈 [gzipped])

Details
name                parsed_size           gzip_size
staging-site             +269 B  (+0.0%)      +86 B  (+0.0%)
sites-dashboard          +269 B  (+0.0%)      +86 B  (+0.0%)
site-settings            +269 B  (+0.0%)      +86 B  (+0.0%)
site-performance         +269 B  (+0.0%)      +86 B  (+0.0%)
site-monitoring          +269 B  (+0.0%)      +86 B  (+0.0%)
site-logs                +269 B  (+0.0%)      +86 B  (+0.0%)
plans                    +269 B  (+0.0%)      +86 B  (+0.0%)
overview                 +269 B  (+0.0%)      +86 B  (+0.0%)
hosting                  +269 B  (+0.0%)      +86 B  (+0.0%)
github-deployments       +269 B  (+0.0%)      +86 B  (+0.0%)
domains                  +269 B  (+0.0%)      +86 B  (+0.0%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

Async-loaded Components (~559 bytes added 📈 [gzipped])

Details
name                                                                   parsed_size           gzip_size
async-load-calypso-notifications                                           +1117 B  (+2.6%)     +473 B  (+3.5%)
async-load-calypso-my-sites-customer-home-celebrate-site-launch-modal       +269 B  (+0.0%)      +86 B  (+0.0%)

React components that are loaded lazily, when a certain part of UI is displayed for the first time.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

let pollTimer: ReturnType< typeof setTimeout > | undefined;
let cooldownTimer: ReturnType< typeof setTimeout > | undefined;

const fetchCount = () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the PR! What worries me is that this PR is adding duplicated fetch mechanism on top of what the existing RestClient already provides. I'm trying a different alternative and will let you know...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I haven't forgotten about this, will post something later today or tomorrow.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No worries 😉

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you check this alternative approach without duplicating the fetch mechanism? #113086

@arthur791004

Copy link
Copy Markdown
Contributor Author

Closing it in favor of #113086

@arthur791004
arthur791004 deleted the add/notifications-live-badge branch August 3, 2026 07:10
@github-actions github-actions Bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Aug 3, 2026
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.

4 participants