Skip to content

feat(notifications): add notification center with filters and bulk actions#5

Merged
kooksee merged 1 commit into
mainfrom
feat/notification-center
Jun 22, 2026
Merged

feat(notifications): add notification center with filters and bulk actions#5
kooksee merged 1 commit into
mainfrom
feat/notification-center

Conversation

@kooksee

@kooksee kooksee commented Jun 22, 2026

Copy link
Copy Markdown

Summary

  • Add Settings → Notifications as the full notification history page (replaces redirect to Scheduled Tasks)
  • Share notification list UI between sidebar bell panel and full page with All / Unread and type filters
  • Add bulk actions: Mark all read and Clear read (notification.deleteRead mutation)
  • Add navigation entry points from workspace settings sidebar and global command palette

Test plan

  • Desktop package smoke test (LOCAL_ONLY=true npm run package)
  • Sidebar bell panel: filters, View all, mark read, delete, navigation
  • Settings → Notifications: full history, filters, Mark all read, Clear read
  • Command palette: Open notification center
  • npm run test -- notifications.test in packages/ui

Made with Cursor

…tions

Introduce Settings → Notifications as the full notification history view,
shared list/filter components for the sidebar panel, clear-read mutation,
and quick navigation entry points from settings and the command palette.

Co-authored-by: Cursor <cursoragent@cursor.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a dedicated notification center and enhances notification management. It adds a new notification.deleteRead mutation and service method to clear read notifications, refactors the sidebar notifications to use a new reusable NotificationList component with filtering capabilities, and sets up the /settings/notifications route and page. Additionally, helper functions and unit tests are introduced. Feedback is provided regarding the NotificationList component, where non-interactive notifications (already read and non-clickable) still show hover background highlights, which could confuse users; it is recommended to conditionally apply hover styles only when notifications are interactive.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +120 to +144
const clickable = isNotificationClickable(notification);

return (
<div
key={notification.id}
className={cn(
'group/item relative rounded-md',
isPage ? 'border bg-card' : 'border-transparent',
notification.readAt
? 'opacity-70'
: isPage
? 'bg-accent/20'
: 'bg-sidebar-accent/40'
)}
>
<button
type="button"
className={cn(
'flex w-full flex-col gap-1 rounded-md px-3 py-3 pr-10 text-left',
isPage
? 'hover:bg-accent/40'
: 'hover:bg-sidebar-accent',
!clickable && 'cursor-default'
)}
onClick={() => onNotificationClick(notification)}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Non-clickable notifications that are already read are not interactive (clicking them does nothing). However, they still show hover background highlights (hover:bg-accent/40 or hover:bg-sidebar-accent), which can be confusing to users as it implies interactivity.

We should only apply the hover styles and pointer cursor if the notification is actually interactive (i.e., either unread or clickable).

            const clickable = isNotificationClickable(notification);
            const isInteractive = !notification.readAt || clickable;

            return (
              <div
                key={notification.id}
                className={cn(
                  'group/item relative rounded-md',
                  isPage ? 'border bg-card' : 'border-transparent',
                  notification.readAt
                    ? 'opacity-70'
                    : isPage
                      ? 'bg-accent/20'
                      : 'bg-sidebar-accent/40'
                )}
              >
                <button
                  type="button"
                  className={cn(
                    'flex w-full flex-col gap-1 rounded-md px-3 py-3 pr-10 text-left',
                    isInteractive
                      ? isPage
                        ? 'hover:bg-accent/40'
                        : 'hover:bg-sidebar-accent'
                      : 'cursor-default'
                  )}
                  onClick={isInteractive ? () => onNotificationClick(notification) : undefined}
                >

@kooksee kooksee merged commit 7bf8c4a into main Jun 22, 2026
1 check failed
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.

1 participant