improvements: mask as read when opening notifications, show custom domain, and more#1042
improvements: mask as read when opening notifications, show custom domain, and more#1042
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.
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.tsxapps/web/app/(org)/dashboard/layout.tsxapps/web/app/(org)/dashboard/_components/Navbar/Top.tsxapps/web/app/(org)/dashboard/_components/Navbar/CapAIDialog.tsxapps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsxapps/web/app/(org)/dashboard/Contexts.tsxapps/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.tsxapps/web/app/(org)/dashboard/layout.tsxapps/web/app/(org)/dashboard/_components/Navbar/Top.tsxapps/web/app/(org)/dashboard/_components/Navbar/CapAIDialog.tsxapps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsxapps/web/app/(org)/dashboard/Contexts.tsxapps/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.tsxapps/web/app/(org)/dashboard/layout.tsxapps/web/app/(org)/dashboard/_components/Navbar/Top.tsxapps/web/app/(org)/dashboard/_components/Navbar/CapAIDialog.tsxapps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsxapps/web/app/(org)/dashboard/Contexts.tsxapps/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.tsxapps/web/app/(org)/dashboard/layout.tsxapps/web/app/(org)/dashboard/_components/Navbar/Top.tsxapps/web/app/(org)/dashboard/_components/Navbar/CapAIDialog.tsxapps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsxapps/web/app/(org)/dashboard/Contexts.tsxapps/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