[Dashboard] Add notifications system with unread tracking #7302
Conversation
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
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Contributor
## Walkthrough
A new notification system was introduced, replacing the previous implementation. This includes a new API module, React components for notification display, and a React hook for managing notification state. The old notification button components and related logic were removed from team and account headers, streamlining notification handling to use the new system based on account ID.
## Changes
| File(s) / Path(s) | Change Summary |
|--------------------------------------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------|
| apps/dashboard/src/@/api/notifications.ts | Added new API module for notifications with typed functions and consistent error handling. |
| apps/dashboard/src/@/components/blocks/notifications/notification-button.tsx | Added new `NotificationsButton` React component with bell icon, unread indicator, drawer/popover UI. |
| apps/dashboard/src/@/components/blocks/notifications/notification-entry.tsx | Added new `NotificationEntry` component rendering notification details with mark-as-read action. |
| apps/dashboard/src/@/components/blocks/notifications/notification-list.tsx | Added new `NotificationList` component with Inbox and Archive tabs, infinite loading, and empty states. |
| apps/dashboard/src/@/components/blocks/notifications/state/manager.ts | Added new `useNotifications` hook managing unread/archived notifications, count, and mark-as-read mutation. |
| apps/dashboard/src/app/(app)/account/components/AccountHeader.tsx | Removed old notification fetch/mark imports and props. |
| apps/dashboard/src/app/(app)/account/components/AccountHeaderUI.stories.tsx | Removed notification-related props from stories. |
| apps/dashboard/src/app/(app)/account/components/AccountHeaderUI.tsx | Replaced old notification button with new component; removed related props and types. |
| apps/dashboard/src/app/(app)/components/Header/SecondaryNav/SecondaryNav.tsx | Replaced old notification button with new component; removed related props and types. |
| apps/dashboard/src/app/(app)/team/components/NotificationButton/NotificationButton.stories.tsx | **Deleted**: Storybook stories for old notification button. |
| apps/dashboard/src/app/(app)/team/components/NotificationButton/NotificationButton.tsx | **Deleted**: Old notification button UI and types. |
| apps/dashboard/src/app/(app)/team/components/NotificationButton/fetch-notifications.ts | **Deleted**: Old notification fetching and marking logic using IndexedDB. |
| apps/dashboard/src/app/(app)/team/components/TeamHeader/TeamHeaderUI.stories.tsx | Removed notification-related stub props from stories. |
| apps/dashboard/src/app/(app)/team/components/TeamHeader/TeamHeaderUI.tsx | Replaced old notification button with new component; removed related props and types. |
| apps/dashboard/src/app/(app)/team/components/TeamHeader/team-header-logged-in.client.tsx | Removed old notification fetch/mark imports and props. |
| apps/dashboard/package.json | Removed dependency "idb-keyval"; added dependency "vaul" for drawer UI components. |
| apps/dashboard/src/@/components/ui/drawer.tsx | Added new drawer UI components built on `vaul` primitives for use in notifications UI. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant User
participant NotificationsButton
participant useNotifications (Hook)
participant Notifications API
User->>NotificationsButton: Click bell icon
NotificationsButton->>useNotifications: Fetch unread, archived notifications, count (accountId)
useNotifications->>Notifications API: getUnreadNotifications / getArchivedNotifications / getUnreadNotificationsCount
Notifications API-->>useNotifications: Return notifications data
useNotifications-->>NotificationsButton: Provide notification state
NotificationsButton->>User: Display drawer or popover with notification list
User->>NotificationsButton: Mark notification as read
NotificationsButton->>useNotifications: markNotificationAsRead(notificationId)
useNotifications->>Notifications API: markNotificationAsRead
Notifications API-->>useNotifications: Confirm update
useNotifications-->>NotificationsButton: Update notification stateSuggested reviewers
|
Member
Author
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
jnsdls
added
the
DO NOT MERGE
Graphite App
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7302 +/- ##
=======================================
Coverage 55.57% 55.57%
=======================================
Files 909 909
Lines 58673 58673
Branches 4158 4158
=======================================
Hits 32607 32607
Misses 25959 25959
Partials 107 107
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (8)
apps/dashboard/src/@/components/blocks/notifications/notification-button.tsx (1)
22-23: Improve the implementation comment and consider refactoring.The comment "kinda jank but it works" suggests the unread count logic could be improved. Consider either:
- Improving the implementation to be more robust
- Adding a more descriptive comment explaining why this approach is necessary
- {/* kinda jank but it works: always take the last page and check the unread count of IT */} + {/* Display notification indicator when there are unread notifications */}apps/dashboard/src/@/components/blocks/notifications/notification-entry.tsx (1)
45-53: Consider adding accessibility improvements to the mark as read button.The icon-only button lacks an accessible label for screen readers.
<Button variant="ghost" size="icon" onClick={() => onMarkAsRead(notification.id)} className="text-muted-foreground hover:text-foreground" + aria-label="Mark notification as read" > <Trash2Icon className="h-4 w-4" /> </Button>apps/dashboard/src/@/components/blocks/notifications/notification-list.tsx (1)
59-60: Consider making the max height configurable.The hardcoded max height of 600px might not be ideal for all screen sizes. Consider making this configurable or responsive.
- <div className="max-h-[600px] w-full overflow-y-auto"> + <div className="max-h-[min(600px,80vh)] w-full overflow-y-auto">apps/dashboard/src/@/components/blocks/notifications/state/manager.ts (3)
78-79: Improve type safety for pageParam.The type assertion pattern used for
pageParamcould be simplified and made more type-safe.- const cursor = (pageParam ?? undefined) as string | undefined; + const cursor = pageParam as string | undefined;Since
pageParamis already typed throughinitialPageParam, the nullish coalescing and type assertion are redundant.Also applies to: 93-94
135-135: Extract magic numbers into named constants.The stale time values should be extracted into named constants for better maintainability.
At the top of the file, add:
+const STALE_TIME = { + UNREAD_COUNT: 60_000, // 1 minute + PRODUCT_UPDATES: 60_000 * 15, // 15 minutes +} as const;Then update the usages:
- staleTime: 60_000, // 1min + staleTime: STALE_TIME.UNREAD_COUNT,- staleTime: 60_000 * 15, // 15min + staleTime: STALE_TIME.PRODUCT_UPDATES,
127-129: Consider using a more robust error handling approach.Using
console.errorin production code might not be ideal. Consider using a proper error tracking service or at least a more centralized error handling approach.- cleanupProductUpdates(res.data).catch((err) => { - console.error("Failed to cleanup product updates", err); - }); + cleanupProductUpdates(res.data).catch((err) => { + // Log to error tracking service if available + if (process.env.NODE_ENV === "development") { + console.error("Failed to cleanup product updates", err); + } + });apps/dashboard/src/@/api/notifications.ts (2)
128-144: Clarify the dual behavior of markNotificationAsRead function.The function's behavior changes significantly based on whether
notificationIdis provided, but this isn't clear from the function signature alone.Consider splitting into two separate functions for clarity:
-export async function markNotificationAsRead(notificationId?: string) { +export async function markNotificationAsRead(notificationId: string) { + const authToken = await getAuthToken(); + if (!authToken) { + throw new Error("No auth token found"); + } + const url = new URL( + "/v1/dashboard-notifications/mark-as-read", + NEXT_PUBLIC_THIRDWEB_API_HOST, + ); + const response = await fetch(url, { + method: "PUT", + headers: { + Authorization: `Bearer ${authToken}`, + "Content-Type": "application/json", + }, + body: JSON.stringify({ notificationId }), + }); + // ... rest of function +} + +export async function markAllNotificationsAsRead() { const authToken = await getAuthToken(); if (!authToken) { throw new Error("No auth token found"); } const url = new URL( "/v1/dashboard-notifications/mark-as-read", NEXT_PUBLIC_THIRDWEB_API_HOST, ); const response = await fetch(url, { method: "PUT", headers: { Authorization: `Bearer ${authToken}`, "Content-Type": "application/json", }, - // if notificationId is provided, mark it as read, otherwise mark all as read - body: JSON.stringify(notificationId ? { notificationId } : {}), + body: JSON.stringify({}), }); - if (!response.ok) { - const body = await response.text(); - return { - status: "error", - reason: "unknown", - body, - } as const; - } - return { - status: "success", - } as const; + // ... rest of function }
41-47: Enhance error handling with more specific error types.All functions return generic "unknown" error reasons. Consider providing more specific error categorization based on HTTP status codes.
- return { - status: "error", - reason: "unknown", - body, - } as const; + let reason: string; + switch (response.status) { + case 401: + reason = "unauthorized"; + break; + case 403: + reason = "forbidden"; + break; + case 404: + reason = "not_found"; + break; + case 429: + reason = "rate_limited"; + break; + case 500: + reason = "server_error"; + break; + default: + reason = "unknown"; + } + return { + status: "error", + reason, + body, + statusCode: response.status, + } as const;Also applies to: 77-83, 109-115, 146-152, 179-184
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (15)
apps/dashboard/src/@/api/notifications.ts(1 hunks)apps/dashboard/src/@/components/blocks/notifications/notification-button.tsx(1 hunks)apps/dashboard/src/@/components/blocks/notifications/notification-entry.tsx(1 hunks)apps/dashboard/src/@/components/blocks/notifications/notification-list.tsx(1 hunks)apps/dashboard/src/@/components/blocks/notifications/state/manager.ts(1 hunks)apps/dashboard/src/app/(app)/account/components/AccountHeader.tsx(0 hunks)apps/dashboard/src/app/(app)/account/components/AccountHeaderUI.stories.tsx(0 hunks)apps/dashboard/src/app/(app)/account/components/AccountHeaderUI.tsx(2 hunks)apps/dashboard/src/app/(app)/components/Header/SecondaryNav/SecondaryNav.tsx(2 hunks)apps/dashboard/src/app/(app)/team/components/NotificationButton/NotificationButton.stories.tsx(0 hunks)apps/dashboard/src/app/(app)/team/components/NotificationButton/NotificationButton.tsx(0 hunks)apps/dashboard/src/app/(app)/team/components/NotificationButton/fetch-notifications.ts(0 hunks)apps/dashboard/src/app/(app)/team/components/TeamHeader/TeamHeaderUI.stories.tsx(0 hunks)apps/dashboard/src/app/(app)/team/components/TeamHeader/TeamHeaderUI.tsx(2 hunks)apps/dashboard/src/app/(app)/team/components/TeamHeader/team-header-logged-in.client.tsx(1 hunks)
💤 Files with no reviewable changes (6)
- apps/dashboard/src/app/(app)/account/components/AccountHeader.tsx
- apps/dashboard/src/app/(app)/account/components/AccountHeaderUI.stories.tsx
- apps/dashboard/src/app/(app)/team/components/TeamHeader/TeamHeaderUI.stories.tsx
- apps/dashboard/src/app/(app)/team/components/NotificationButton/NotificationButton.stories.tsx
- apps/dashboard/src/app/(app)/team/components/NotificationButton/fetch-notifications.ts
- apps/dashboard/src/app/(app)/team/components/NotificationButton/NotificationButton.tsx
🧰 Additional context used
🧬 Code Graph Analysis (6)
apps/dashboard/src/app/(app)/account/components/AccountHeaderUI.tsx (1)
apps/dashboard/src/@/components/blocks/notifications/notification-button.tsx (1)
NotificationsButton(14-36)
apps/dashboard/src/@/components/blocks/notifications/notification-list.tsx (3)
apps/dashboard/src/@/components/blocks/notifications/state/manager.ts (1)
useNotifications(52-269)apps/dashboard/src/@/components/ui/tabs.tsx (1)
TabButtons(85-159)apps/dashboard/src/@/components/blocks/notifications/notification-entry.tsx (1)
NotificationEntry(13-57)
apps/dashboard/src/app/(app)/components/Header/SecondaryNav/SecondaryNav.tsx (1)
apps/dashboard/src/@/components/blocks/notifications/notification-button.tsx (1)
NotificationsButton(14-36)
apps/dashboard/src/@/components/blocks/notifications/notification-entry.tsx (2)
apps/dashboard/src/@/api/notifications.ts (1)
Notification(7-16)apps/dashboard/src/@/components/ui/button.tsx (1)
Button(85-85)
apps/dashboard/src/@/components/blocks/notifications/state/manager.ts (1)
apps/dashboard/src/@/api/notifications.ts (8)
Notification(7-16)NotificationsApiResponse(18-21)getUnreadNotifications(23-56)getArchivedNotifications(58-92)getUnreadNotificationsCount(94-126)fetchProductUpdates(170-191)markNotificationAsRead(128-157)ProductUpdateItem(163-168)
apps/dashboard/src/app/(app)/team/components/TeamHeader/TeamHeaderUI.tsx (1)
apps/dashboard/src/@/components/blocks/notifications/notification-button.tsx (1)
NotificationsButton(14-36)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: Unit Tests
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: Lint Packages
- GitHub Check: Size
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: Build Packages
- GitHub Check: Analyze (javascript)
🔇 Additional comments (9)
apps/dashboard/src/app/(app)/team/components/TeamHeader/team-header-logged-in.client.tsx (1)
13-13: LGTM! Notification props properly removed.The removal of notification-related imports and props from the team header aligns with the centralized notification system refactor.
apps/dashboard/src/@/components/blocks/notifications/notification-button.tsx (1)
14-36: Well-designed component with simplified props interface.The new
NotificationsButtoncomponent successfully centralizes notification state management through theuseNotificationshook and simplifies the component interface to just require anaccountId.apps/dashboard/src/app/(app)/components/Header/SecondaryNav/SecondaryNav.tsx (2)
5-5: Correct import update for new notification system.The import change from the old notification component to the new
NotificationsButtonaligns with the refactor.
20-20: Simplified component usage improves maintainability.The new
NotificationsButtononly requires theaccountIdprop, eliminating the need for async callback functions and simplifying the component interface.apps/dashboard/src/app/(app)/account/components/AccountHeaderUI.tsx (2)
8-8: Correct import for new notification system.Import of the new
NotificationsButtoncomponent is consistent with the refactor pattern.
119-119:✅ Verification successful
Verify notification coverage across UI variants.
The
NotificationsButtonis added to the mobile UI. Ensure that the desktop UI also has notification access (likely through theSecondaryNavcomponent).
🏁 Script executed:
#!/bin/bash # Verify that both mobile and desktop UIs have notification functionality echo "Checking desktop UI notification coverage..." rg -A 10 -B 5 "SecondaryNav" apps/dashboard/src/app/\(app\)/account/components/AccountHeaderUI.tsx echo "Checking SecondaryNav includes notifications..." rg -A 5 -B 5 "NotificationsButton" apps/dashboard/src/app/\(app\)/components/Header/SecondaryNav/SecondaryNav.tsxLength of output: 2496
Notification coverage verified across UI variants
– In AccountHeaderUI.tsx (mobile view):
<NotificationsButton accountId={props.account.id} />is present.
– In SecondaryNav.tsx (desktop view):<NotificationsButton accountId={props.account.id} />is already rendered.No further changes needed.
apps/dashboard/src/app/(app)/team/components/TeamHeader/TeamHeaderUI.tsx (1)
9-9: Clean implementation of the new notifications system.The replacement of the old notification button with the new
NotificationsButtoncomponent simplifies the props interface by requiring only theaccountId. The import path correctly uses the alias pattern.Also applies to: 198-198
apps/dashboard/src/@/api/notifications.ts (2)
1-6: LGTM! Server-side setup is correctly configured.The file properly uses server-only directives and imports the necessary dependencies for server-side API operations.
7-22: Well-defined type structures.The type definitions are comprehensive and properly structured for the notification system. The optional
nextCursorinNotificationsApiResponsecorrectly supports pagination.
apps/dashboard/src/@/components/blocks/notifications/notification-entry.tsx
Outdated
Show resolved
Hide resolved
apps/dashboard/src/@/components/blocks/notifications/notification-list.tsx
Outdated
Show resolved
Hide resolved
apps/dashboard/src/@/components/blocks/notifications/notification-list.tsx
Outdated
Show resolved
Hide resolved
apps/dashboard/src/@/components/blocks/notifications/state/manager.ts
Outdated
Show resolved
Hide resolved
Contributor
size-limit report 📦
|
jnsdls
force-pushed
the
_Dashboard_Add_notifications_system_with_unread_tracking
branch
from
59b6357 to
fa3c433
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
jnsdls
force-pushed
the
_Dashboard_Add_notifications_system_with_unread_tracking
branch
from
fa3c433 to
0ac15b4
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
apps/dashboard/src/@/components/blocks/notifications/state/manager.ts (3)
129-131: Consider extracting side effects from query function.The cleanup operation inside the query function is an anti-pattern as query functions should be pure. Consider moving this to a separate effect or mutation.
- // as a side effect, clean up the read notifications - cleanupProductUpdates(res.data).catch((err) => { - console.error("Failed to cleanup product updates", err); - });Move the cleanup to a separate effect:
+// Add this effect after the query definition +useEffect(() => { + if (updatesQuery.isSuccess && updatesQuery.data) { + const currentUpdates = updatesQuery.data.map(d => ({ ...d, isRead: false })); + cleanupProductUpdates(currentUpdates).catch((err) => { + console.error("Failed to cleanup product updates", err); + }); + } +}, [updatesQuery.isSuccess, updatesQuery.data]);
277-277: Track the temporary IndexedDB implementation.The comment indicates this is a temporary solution that will be replaced by a proper API. Consider creating a tracking issue to ensure this technical debt is addressed.
Would you like me to generate a GitHub issue template to track the replacement of the IndexedDB implementation with a proper API?
88-88: Consider making refetch intervals configurable.The hardcoded 1-minute refetch intervals might not be optimal for all use cases and could impact performance or user experience.
+interface UseNotificationsOptions { + refetchInterval?: number; +} + -export function useNotifications(accountId: string) { +export function useNotifications( + accountId: string, + options: UseNotificationsOptions = {} +) { + const { refetchInterval = 60_000 } = options;Then use
refetchIntervalin the query configurations instead of the hardcoded values.Also applies to: 104-104, 116-116
apps/dashboard/src/@/components/blocks/notifications/notification-list.tsx (1)
14-14: Consider more specific prop typing for better maintainability.Using the entire return type of
useNotificationsmakes the component tightly coupled and brittle to changes in the hook interface.+interface NotificationListProps { + unreadNotifications: Notification[]; + archivedNotifications: Notification[]; + updates: (ProductUpdateItem & { isRead: boolean })[]; + totalUnreadCount: number; + unreadUpdatesCount: number; + unreadNotificationsCount: number; + isLoadingUnread: boolean; + isLoadingArchived: boolean; + isLoadingUpdates: boolean; + hasMoreUnread: boolean; + hasMoreArchived: boolean; + isFetchingMoreUnread: boolean; + isFetchingMoreArchived: boolean; + loadMoreUnread: () => void; + loadMoreArchived: () => void; + markAsRead: (id: string) => void; + markAllAsRead: () => void; + markUpdateAsRead: (id: string) => void; +} + -export function NotificationList(props: ReturnType<typeof useNotifications>) { +export function NotificationList(props: NotificationListProps) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (15)
apps/dashboard/src/@/api/notifications.ts(1 hunks)apps/dashboard/src/@/components/blocks/notifications/notification-button.tsx(1 hunks)apps/dashboard/src/@/components/blocks/notifications/notification-entry.tsx(1 hunks)apps/dashboard/src/@/components/blocks/notifications/notification-list.tsx(1 hunks)apps/dashboard/src/@/components/blocks/notifications/state/manager.ts(1 hunks)apps/dashboard/src/app/(app)/account/components/AccountHeader.tsx(0 hunks)apps/dashboard/src/app/(app)/account/components/AccountHeaderUI.stories.tsx(0 hunks)apps/dashboard/src/app/(app)/account/components/AccountHeaderUI.tsx(2 hunks)apps/dashboard/src/app/(app)/components/Header/SecondaryNav/SecondaryNav.tsx(2 hunks)apps/dashboard/src/app/(app)/team/components/NotificationButton/NotificationButton.stories.tsx(0 hunks)apps/dashboard/src/app/(app)/team/components/NotificationButton/NotificationButton.tsx(0 hunks)apps/dashboard/src/app/(app)/team/components/NotificationButton/fetch-notifications.ts(0 hunks)apps/dashboard/src/app/(app)/team/components/TeamHeader/TeamHeaderUI.stories.tsx(0 hunks)apps/dashboard/src/app/(app)/team/components/TeamHeader/TeamHeaderUI.tsx(2 hunks)apps/dashboard/src/app/(app)/team/components/TeamHeader/team-header-logged-in.client.tsx(1 hunks)
💤 Files with no reviewable changes (6)
- apps/dashboard/src/app/(app)/account/components/AccountHeaderUI.stories.tsx
- apps/dashboard/src/app/(app)/account/components/AccountHeader.tsx
- apps/dashboard/src/app/(app)/team/components/TeamHeader/TeamHeaderUI.stories.tsx
- apps/dashboard/src/app/(app)/team/components/NotificationButton/NotificationButton.stories.tsx
- apps/dashboard/src/app/(app)/team/components/NotificationButton/NotificationButton.tsx
- apps/dashboard/src/app/(app)/team/components/NotificationButton/fetch-notifications.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- apps/dashboard/src/app/(app)/team/components/TeamHeader/team-header-logged-in.client.tsx
- apps/dashboard/src/app/(app)/team/components/TeamHeader/TeamHeaderUI.tsx
- apps/dashboard/src/app/(app)/account/components/AccountHeaderUI.tsx
- apps/dashboard/src/app/(app)/components/Header/SecondaryNav/SecondaryNav.tsx
- apps/dashboard/src/@/components/blocks/notifications/notification-button.tsx
- apps/dashboard/src/@/components/blocks/notifications/notification-entry.tsx
- apps/dashboard/src/@/api/notifications.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/dashboard/src/@/components/blocks/notifications/state/manager.ts (1)
apps/dashboard/src/@/api/notifications.ts (8)
Notification(7-16)NotificationsApiResponse(18-21)getUnreadNotifications(23-56)getArchivedNotifications(58-92)getUnreadNotificationsCount(94-126)fetchProductUpdates(170-191)markNotificationAsRead(128-157)ProductUpdateItem(163-168)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Size
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
apps/dashboard/src/@/components/blocks/notifications/notification-list.tsx (2)
245-245: Good implementation of intersection observer optimization.The
observer.unobserve(entry.target)call prevents duplicate trigger events, which is a solid optimization for the infinite scrolling behavior.
16-22: Well-designed initial tab selection logic.The priority-based tab selection (Inbox → Updates → Archive) based on unread counts provides a good user experience by showing the most relevant content first.
apps/dashboard/src/@/components/blocks/notifications/notification-list.tsx
Outdated
Show resolved
Hide resolved
jnsdls
removed
the
DO NOT MERGE
MananTank
approved these changes
Jun 9, 2025
jnsdls
force-pushed
the
_Dashboard_Add_notifications_system_with_unread_tracking
branch
from
d1e0c1d to
5bb9fd0
Compare
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Contributor
Merge activity
|
<!-- This is an auto-generated comment: release notes by coderabbit.ai -->
## Summary by CodeRabbit
- **New Features**
- Introduced a new notifications system with a bell icon button displaying unread indicators.
- Added a notifications panel with Inbox and Archive tabs supporting infinite scrolling and marking notifications as read.
- Implemented a centralized notification state manager with optimized fetching, caching, and mutation handling.
- Added a UI drawer component for mobile notification display with smooth open/close interactions.
- **Refactor**
- Replaced legacy notification button and fetching logic with the new unified Notifications button using only account ID.
- Removed legacy notification callbacks and components from header and navigation for simplified notification handling.
- **Chores**
- Removed deprecated notification files and Storybook stories.
- Updated dependencies by replacing "idb-keyval" with "vaul" for UI drawer functionality.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
<!-- start pr-codex -->
---
## PR-Codex overview
This PR focuses on removing the `NotificationButton` component and its related files, replacing it with a new `NotificationsButton` component that integrates notifications more effectively within the application.
### Detailed summary
- Deleted `NotificationButton.tsx`, `fetch-notifications.ts`, and `NotificationButton.stories.tsx`.
- Added `NotificationsButton` component with enhanced functionality for displaying notifications.
- Updated various components (`AccountHeaderUI`, `SecondaryNav`, `TeamHeaderUI`) to use `NotificationsButton`.
- Removed references to `getInboxNotifications` and `markNotificationAsRead` from multiple components.
- Introduced new notification management logic and UI components in `notification-list.tsx` and `notification-entry.tsx`.
- Updated dependencies in `package.json` to include the new `vaul` library for UI elements.
> ✨ Ask PR-Codex anything about this PR by commenting with `/codex {your question}`
<!-- end pr-codex -->
graphite-app
bot
force-pushed
the
_Dashboard_Add_notifications_system_with_unread_tracking
branch
from
5bb9fd0 to
e742929
Compare
graphite-app
bot
deleted the
_Dashboard_Add_notifications_system_with_unread_tracking
branch
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.
Summary by CodeRabbit
New Features
Refactor
Chores
PR-Codex overview
This PR focuses on removing the
NotificationButtoncomponent and its related files, replacing it with a newNotificationsButtoncomponent that enhances notification handling. Additionally, it updates the notification fetching logic and refactors related UI components.Detailed summary
NotificationButton.tsx,fetch-notifications.ts, and related story files.NotificationsButtoncomponent for improved notification UI.SecondaryNav,AccountHeader, andTeamHeadercomponents to useNotificationsButton.manager.tsfor better state management.notifications.tsfor fetching and marking notifications.