-
Notifications
You must be signed in to change notification settings - Fork 810
improvements: mask as read when opening notifications, show custom domain, and more #1042
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Caution Review failedThe pull request is closed. WalkthroughAdds a persistent referClicked boolean to the dashboard context (cookie-backed), wires it from layout SSR cookies, consumes it in Top to control Refer button ping and click behavior, and adds a mark-as-read mutation in Top. Several Navbar and CapCard presentational/layout tweaks applied. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant L as layout.tsx (SSR)
participant DC as DashboardContexts
participant Ctx as DashboardContext.Provider
participant Top as Top.tsx (ReferButton)
rect #E6F0FF
note over L: Server render reads cookie
U->>L: Request dashboard
L->>L: Read cookie "referClicked"
L->>DC: Render with prop referClicked
DC->>Ctx: Initialize referClickedState
end
rect #F0FFF0
note over Top: Client interactions
Top->>Ctx: useContext() { referClickedState, setReferClickedStateHandler }
Top-->>U: Show Refer ping when referClickedState === false
U->>Top: Click Refer
Top->>Ctx: setReferClickedStateHandler(true)
Ctx->>Ctx: Update state + write cookie "referClicked=true"
Top-->>U: Ping hidden
end
sequenceDiagram
autonumber
actor U as User
participant Top as Top.tsx
participant RQ as React Query
participant API as markAsRead()
U->>Top: Click bell / press Enter/Space
alt hasNewNotifications
Top->>RQ: mutate(markAsRead)
RQ->>API: POST markAsRead
API-->>RQ: success
RQ->>RQ: invalidateQuery("notifications")
else noNew
note over Top: Skip mutation
end
Top->>Top: Toggle notifications panel
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/app/(org)/dashboard/_components/Navbar/CapAIDialog.tsx (1)
1-1
: Rename file to kebab-case and update importsRepo guideline requires kebab-case for .ts/.tsx filenames. Rename
apps/web/app/(org)/dashboard/_components/Navbar/CapAIDialog.tsx → apps/web/app/(org)/dashboard/_components/Navbar/cap-ai-dialog.tsx
and update imports (found: apps/web/app/(org)/dashboard/_components/Navbar/CapAIBox.tsx — change import CapAIDialog from "./CapAIDialog" to "./cap-ai-dialog").
🧹 Nitpick comments (8)
apps/web/app/(org)/dashboard/_components/Navbar/CapAIDialog.tsx (2)
53-53
: Avoid redundant color inheritance on list itemsYou’ve set text-gray-12 on the parent
, so child text inherits it. No need to restate it on inner spans.
68-68
: Remove duplicate text color on spanDrop the extra text-gray-12; it’s inherited from the
.
- <span className="text-gray-12">{feature}</span> + <span>{feature}</span>apps/web/app/(org)/dashboard/_components/Navbar/CapAIBox.tsx (1)
27-31
: Avoid calc-based width; prefer Tailwind spacing utilities.Using w-[calc(100%-12px)] can cause layout jitter. Use w-full with horizontal padding for consistent spacing.
- className="hidden p-3 mb-6 w-[calc(100%-12px)] mx-auto rounded-xl border transition-colors cursor-pointer md:block hover:bg-gray-2 h-fit border-gray-3" + className="hidden p-3 mb-6 w-full px-1.5 rounded-xl border transition-colors cursor-pointer md:block hover:bg-gray-2 h-fit border-gray-3"apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx (1)
407-407
: Z-index now overtakes the upload overlay; confirm interaction intent.z-[51] sits above the upload overlay (z-50). If the overlay should block interactions, disable pointer events on the selector while uploading.
- className={clsx( - "absolute top-2 left-2 z-[51] duration-200", + className={clsx( + "absolute top-2 left-2 z-[51] duration-200", + uploadProgress && "pointer-events-none",apps/web/app/(org)/dashboard/_components/Navbar/Top.tsx (3)
49-51
: Remove unused value from context destructure.activeOrganization is not used.
- const { activeSpace, anyNewNotifications, activeOrganization } = + const { activeSpace, anyNewNotifications } = useDashboardContext();
82-92
: Prefer precise cache updates over broad invalidation (TanStack Query v5).Update notifications in cache and any badge/boolean query directly to avoid refetch and UI lag.
- const markAllAsread = useMutation({ - mutationFn: () => markAsRead(), - onSuccess: () => { - queryClient.invalidateQueries({ - queryKey: ["notifications"], - }); - }, - onError: (error) => { - console.error("Error marking notifications as read:", error); - }, - }); + const markAllAsRead = useMutation({ + mutationFn: () => markAsRead(), + onSuccess: () => { + // 1) Optimistically mark all as read in list queries + queryClient.setQueriesData({ queryKey: ["notifications"] }, (prev: any) => { + if (!prev) return prev; + // Adjust to your shape; example assumes array with readAt + return Array.isArray(prev) + ? prev.map((n) => ({ ...n, readAt: new Date().toISOString() })) + : prev; + }); + // 2) If you cache badge/boolean, zero it out precisely + queryClient.setQueryData(["notifications-badge"], 0); + queryClient.setQueryData(["notifications-any-new"], false); + }, + onError: (error) => { + console.error("Error marking notifications as read:", error); + }, + });Also rename markAllAsread → markAllAsRead for readability.
131-135
: Debounce duplicate mark-as-read calls on rapid toggle.Guard to avoid firing the mutation twice on quick click/keyboard events.
+ const hasMarkedRef = useRef(false); ... - if (anyNewNotifications) { - markAllAsread.mutate(); - } + if (anyNewNotifications && !hasMarkedRef.current) { + hasMarkedRef.current = true; + markAllAsRead.mutate(); + }Reset hasMarkedRef when fresh new notifications arrive (e.g., via a query subscription).
Also applies to: 139-143
apps/web/app/(org)/dashboard/_components/Navbar/Items.tsx (1)
162-173
: Invalid Tailwind class: use leading-none (or leading-tight) instead of leading-0.leading-0 isn’t a Tailwind utility and won’t apply.
- <p className="text-sm truncate leading-0 text-gray-12"> + <p className="text-sm truncate leading-none text-gray-12"> ... - <p className="w-full text-[11px] flex-1 duration-200 truncate leading-0 text-gray-11"> + <p className="w-full text-[11px] flex-1 duration-200 truncate leading-none text-gray-11">Also applies to: 196-201
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/web/app/(org)/dashboard/Contexts.tsx
(5 hunks)apps/web/app/(org)/dashboard/_components/Navbar/CapAIBox.tsx
(1 hunks)apps/web/app/(org)/dashboard/_components/Navbar/CapAIDialog.tsx
(2 hunks)apps/web/app/(org)/dashboard/_components/Navbar/Items.tsx
(4 hunks)apps/web/app/(org)/dashboard/_components/Navbar/Top.tsx
(6 hunks)apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx
(2 hunks)apps/web/app/(org)/dashboard/layout.tsx
(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/**/*.{ts,tsx}
: Use TanStack Query v5 for client-side server state and data fetching in the web app
Mutations should call Server Actions and perform precise cache updates with setQueryData/setQueriesData, avoiding broad invalidations
Prefer Server Components for initial data and pass initialData to client components for React Query hydration
Files:
apps/web/app/(org)/dashboard/_components/Navbar/CapAIBox.tsx
apps/web/app/(org)/dashboard/layout.tsx
apps/web/app/(org)/dashboard/_components/Navbar/Top.tsx
apps/web/app/(org)/dashboard/_components/Navbar/CapAIDialog.tsx
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx
apps/web/app/(org)/dashboard/Contexts.tsx
apps/web/app/(org)/dashboard/_components/Navbar/Items.tsx
{apps/web,packages/ui}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
{apps/web,packages/ui}/**/*.{ts,tsx}
: Use Tailwind CSS exclusively for styling in the web app and shared React UI components
Component naming: React components in PascalCase; hooks in camelCase starting with 'use'
Files:
apps/web/app/(org)/dashboard/_components/Navbar/CapAIBox.tsx
apps/web/app/(org)/dashboard/layout.tsx
apps/web/app/(org)/dashboard/_components/Navbar/Top.tsx
apps/web/app/(org)/dashboard/_components/Navbar/CapAIDialog.tsx
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx
apps/web/app/(org)/dashboard/Contexts.tsx
apps/web/app/(org)/dashboard/_components/Navbar/Items.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; prefer shared types from packages
**/*.{ts,tsx}
: Use Biome to format/lint TypeScript with a 2-space indent
TypeScript file names should be kebab-case (e.g., user-menu.tsx)
Files:
apps/web/app/(org)/dashboard/_components/Navbar/CapAIBox.tsx
apps/web/app/(org)/dashboard/layout.tsx
apps/web/app/(org)/dashboard/_components/Navbar/Top.tsx
apps/web/app/(org)/dashboard/_components/Navbar/CapAIDialog.tsx
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx
apps/web/app/(org)/dashboard/Contexts.tsx
apps/web/app/(org)/dashboard/_components/Navbar/Items.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
React/Solid components should be named using PascalCase
Files:
apps/web/app/(org)/dashboard/_components/Navbar/CapAIBox.tsx
apps/web/app/(org)/dashboard/layout.tsx
apps/web/app/(org)/dashboard/_components/Navbar/Top.tsx
apps/web/app/(org)/dashboard/_components/Navbar/CapAIDialog.tsx
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx
apps/web/app/(org)/dashboard/Contexts.tsx
apps/web/app/(org)/dashboard/_components/Navbar/Items.tsx
🧬 Code graph analysis (1)
apps/web/app/(org)/dashboard/_components/Navbar/Top.tsx (2)
apps/web/app/(org)/dashboard/Contexts.tsx (2)
useDashboardContext
(44-44)useTheme
(42-42)apps/web/actions/notifications/mark-as-read.ts (1)
markAsRead
(9-34)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (4)
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx (1)
470-472
: LGTM: clearer failure text sizing.apps/web/app/(org)/dashboard/_components/Navbar/Items.tsx (1)
111-113
: Layout updates look good.Responsive padding/stacking changes read well in collapsed/expanded states.
Also applies to: 117-129, 145-151
apps/web/app/(org)/dashboard/Contexts.tsx (1)
26-28
: Context/cookie plumbing for referClicked is sound.State initialization from server cookie and persistence via js-cookie are correct. Nice, minimal API.
Consider updating any “badge/indicator” query or context boolean when Top.tsx marks notifications as read to avoid stale UI until a full refresh. If you have such a query, expose a setter parallel to referClicked for symmetry.
Also applies to: 56-58, 76-77, 141-146, 166-168
apps/web/app/(org)/dashboard/layout.tsx (1)
68-83
: Cookie key 'referClicked' matches — no change required.
layout.tsx reads cookies().get("referClicked") and Contexts.tsx writes Cookies.set("referClicked", referClicked ? "true" : "false"), so the key and boolean handling are consistent.
This PR:
Summary by CodeRabbit
New Features
UI/Style