ui: show org name + image on share page #1152
Conversation
|
Warning Rate limit exceeded@ameer2468 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 25 minutes and 57 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
WalkthroughAdds optional organizationIconUrl and organizationName to video data and props; updates ShareHeader to render Next/Image or Avatar, adds a blur guard to skip no-op title updates, restructures header layout and status/time/action placement; removes Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Page as /s/[videoId]/page
participant Loader as DataLoader
participant Share as ShareHeader
participant Image as Next/Image
participant Avatar as Avatar
User->>Page: navigate to /s/[videoId]
Page->>Loader: fetch video + organization fields
Loader-->>Page: returns { ..., organizationIconUrl?, organizationName? }
Page->>Share: render ShareHeader(data)
alt organizationIconUrl present
Share->>Image: render organizationIconUrl
else
Share->>Avatar: render organizationName fallback
end
note right of Share: Title input interaction
User->>Share: edit title -> blur
alt title changed
Share->>Share: handleBlur updates title
else
Share-->>User: no update (guarded)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
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/s/[videoId]/page.tsx (1)
351-366: Fix TypeScript error: AddorganizationNameto the video type.The static analysis correctly flags that
organizationNameis missing from the video type definition. WhileorganizationIconUrlis included at line 364,organizationNameis omitted. This causes a type error at line 687 wherevideo.organizationNameis accessed.Apply this diff to add the missing property:
video: Omit< InferSelectModel<typeof videos>, "folderId" | "password" | "settings" > & { sharedOrganization: { organizationId: string } | null; hasPassword: boolean; ownerIsPro?: boolean; orgSettings?: OrganizationSettings | null; videoSettings?: OrganizationSettings | null; organizationIconUrl?: string | null; + organizationName?: string | null; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/web/app/(org)/dashboard/_components/Navbar/Items.tsx(1 hunks)apps/web/app/s/[videoId]/_components/ShareHeader.tsx(7 hunks)apps/web/app/s/[videoId]/page.tsx(4 hunks)apps/web/components/forms/NewOrganization.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TanStack Query v5 for all client-side server state and data fetching in the web app
Web mutations should call Server Actions directly and perform targeted cache updates with setQueryData/setQueriesData rather than broad invalidations
Client code should use useEffectQuery/useEffectMutation and useRpcClient from apps/web/lib/EffectRuntime.ts; do not create ManagedRuntime inside components
Files:
apps/web/app/(org)/dashboard/_components/Navbar/Items.tsxapps/web/app/s/[videoId]/_components/ShareHeader.tsxapps/web/app/s/[videoId]/page.tsxapps/web/components/forms/NewOrganization.tsx
apps/web/app/**/*.{tsx,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer Server Components for initial data in the Next.js App Router and pass initialData to client components
Files:
apps/web/app/(org)/dashboard/_components/Navbar/Items.tsxapps/web/app/s/[videoId]/_components/ShareHeader.tsxapps/web/app/s/[videoId]/page.tsx
**/*.{ts,tsx,js,jsx,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not add inline, block, or docstring comments in any language; code must be self-explanatory
Files:
apps/web/app/(org)/dashboard/_components/Navbar/Items.tsxapps/web/app/s/[videoId]/_components/ShareHeader.tsxapps/web/app/s/[videoId]/page.tsxapps/web/components/forms/NewOrganization.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; leverage shared types from packages
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
apps/web/app/(org)/dashboard/_components/Navbar/Items.tsxapps/web/app/s/[videoId]/_components/ShareHeader.tsxapps/web/app/s/[videoId]/page.tsxapps/web/components/forms/NewOrganization.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
apps/web/app/(org)/dashboard/_components/Navbar/Items.tsxapps/web/app/s/[videoId]/_components/ShareHeader.tsxapps/web/app/s/[videoId]/page.tsxapps/web/components/forms/NewOrganization.tsx
apps/web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
On the client, always use
useEffectQueryoruseEffectMutationfrom@/lib/EffectRuntime; never callEffectRuntime.run*directly in components.
Files:
apps/web/app/(org)/dashboard/_components/Navbar/Items.tsxapps/web/app/s/[videoId]/_components/ShareHeader.tsxapps/web/app/s/[videoId]/page.tsxapps/web/components/forms/NewOrganization.tsx
🧬 Code graph analysis (2)
apps/web/app/s/[videoId]/_components/ShareHeader.tsx (1)
packages/database/schema.ts (1)
videos(256-311)
apps/web/app/s/[videoId]/page.tsx (1)
packages/database/schema.ts (1)
organizations(152-180)
🪛 GitHub Check: Typecheck
apps/web/app/s/[videoId]/page.tsx
[failure] 687-687:
Property 'organizationName' does not exist on type 'Omit<{ metadata: VideoMetadata | null; name: string; id: string & Brand<"VideoId">; ownerId: string; settings: { disableSummary?: boolean | undefined; disableCaptions?: boolean | undefined; disableChapters?: boolean | undefined; disableReactions?: boolean | undefined; disableTranscript?: boolean | undefined; disable...'.
⏰ 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 (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (1)
apps/web/app/s/[videoId]/_components/ShareHeader.tsx (1)
79-81: LGTM!The guard prevents unnecessary title updates when the value hasn't changed, avoiding redundant API calls.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/app/s/[videoId]/page.tsx(4 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TanStack Query v5 for all client-side server state and data fetching in the web app
Web mutations should call Server Actions directly and perform targeted cache updates with setQueryData/setQueriesData rather than broad invalidations
Client code should use useEffectQuery/useEffectMutation and useRpcClient from apps/web/lib/EffectRuntime.ts; do not create ManagedRuntime inside components
Files:
apps/web/app/s/[videoId]/page.tsx
apps/web/app/**/*.{tsx,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer Server Components for initial data in the Next.js App Router and pass initialData to client components
Files:
apps/web/app/s/[videoId]/page.tsx
**/*.{ts,tsx,js,jsx,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not add inline, block, or docstring comments in any language; code must be self-explanatory
Files:
apps/web/app/s/[videoId]/page.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; leverage shared types from packages
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
apps/web/app/s/[videoId]/page.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
apps/web/app/s/[videoId]/page.tsx
apps/web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
On the client, always use
useEffectQueryoruseEffectMutationfrom@/lib/EffectRuntime; never callEffectRuntime.run*directly in components.
Files:
apps/web/app/s/[videoId]/page.tsx
🧬 Code graph analysis (1)
apps/web/app/s/[videoId]/page.tsx (1)
packages/database/schema.ts (1)
organizations(152-180)
⏰ 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 (2)
apps/web/app/s/[videoId]/page.tsx (2)
108-109: Type definitions are correct but require proper data fetching.The type definitions for
organizationIconUrlandorganizationNameare syntactically correct. However, ensure these fields are properly fetched from the database (see comment on lines 687-688).Also applies to: 364-365
484-485: Selection is correct but only executes conditionally.The selection of
organizationIconUrlandorganizationNameis correct. However, this query only executes when transcription is not COMPLETE or PROCESSING (line 452), meaning these fields won't be available for videos with completed transcription.
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 (2)
apps/web/app/s/[videoId]/_components/ShareHeader.tsx (2)
136-138: Fix owner check to avoid false negatives in upgrade banner.Use the computed isOwner for consistency with the toString comparison.
- const showUpgradeBanner = - user && data.ownerId === user.id && !userIsPro(user); + const showUpgradeBanner = isOwner && !userIsPro(user);Also applies to: 67-68
152-171: Use semantic button for share status (keyboard accessible).Interactive
is not focusable and breaks keyboard navigation.
- <p - className={clsx(baseClassName, "cursor-pointer hover:text-gray-12")} - onClick={() => setIsSharingDialogOpen(true)} - > - Not shared{" "} - <FontAwesomeIcon className="ml-2 size-2.5" icon={faChevronDown} /> - </p> + <button + type="button" + className={clsx(baseClassName, "cursor-pointer hover:text-gray-12")} + onClick={() => setIsSharingDialogOpen(true)} + > + Not shared{" "} + <FontAwesomeIcon className="ml-2 size-2.5" icon={faChevronDown} /> + </button>- <p - className={clsx(baseClassName, "cursor-pointer hover:text-gray-12")} - onClick={() => setIsSharingDialogOpen(true)} - > - Shared{" "} - <FontAwesomeIcon className="ml-1 size-2.5" icon={faChevronDown} /> - </p> + <button + type="button" + className={clsx(baseClassName, "cursor-pointer hover:text-gray-12")} + onClick={() => setIsSharingDialogOpen(true)} + > + Shared{" "} + <FontAwesomeIcon className="ml-1 size-2.5" icon={faChevronDown} /> + </button>
🧹 Nitpick comments (5)
apps/web/app/s/[videoId]/_components/ShareHeader.tsx (5)
225-227: Fallback and truncate organization name to avoid null/overflow.- <p className="text-sm font-medium text-gray-12"> - {data.organizationName} - </p> + <p + className="text-sm font-medium text-gray-12 max-w-[200px] truncate" + title={data.organizationName ?? "Organization"} + > + {data.organizationName ?? "Organization"} + </p>
269-277: Clear clipboard timer to avoid state updates after unmount.Store timeout id in a ref and clean it up.
- onClick={() => { - navigator.clipboard.writeText(getVideoLink()); - setLinkCopied(true); - setTimeout(() => { - setLinkCopied(false); - }, 2000); - }} + onClick={() => { + navigator.clipboard.writeText(getVideoLink()); + setLinkCopied(true); + if (copyTimer.current) clearTimeout(copyTimer.current); + copyTimer.current = window.setTimeout(() => { + setLinkCopied(false); + }, 2000); + }}Add these outside changes:
// import line import { useEffect, useRef, useState } from "react"; // near other state const copyTimer = useRef<number>(); // cleanup useEffect(() => { return () => { if (copyTimer.current) clearTimeout(copyTimer.current); }; }, []);
82-84: Avoid full refresh; prefer targeted updates.After editTitle, update local/UI state or targeted caches instead of refresh(), per project guidelines.
As per coding guidelines
117-133: Optional: normalize display link (strip protocol for consistency).- return `${webUrl}/s/${data.id}`; + return `${webUrl.replace(/^https?:\/\//, "")}/s/${data.id}`;
210-218: Improve organization icon alt text and specify sizes
- Change
altto{data.organizationName ?? "Organization"}and addsizes="36px"on the<Image />.apps/web/next.config.mjsalready permits all hosts viaremotePatterns.hostname = "**".
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/app/s/[videoId]/_components/ShareHeader.tsx(6 hunks)apps/web/app/s/[videoId]/page.tsx(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/app/s/[videoId]/page.tsx
🧰 Additional context used
📓 Path-based instructions (6)
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TanStack Query v5 for all client-side server state and data fetching in the web app
Web mutations should call Server Actions directly and perform targeted cache updates with setQueryData/setQueriesData rather than broad invalidations
Client code should use useEffectQuery/useEffectMutation and useRpcClient from apps/web/lib/EffectRuntime.ts; do not create ManagedRuntime inside components
Files:
apps/web/app/s/[videoId]/_components/ShareHeader.tsx
apps/web/app/**/*.{tsx,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer Server Components for initial data in the Next.js App Router and pass initialData to client components
Files:
apps/web/app/s/[videoId]/_components/ShareHeader.tsx
**/*.{ts,tsx,js,jsx,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not add inline, block, or docstring comments in any language; code must be self-explanatory
Files:
apps/web/app/s/[videoId]/_components/ShareHeader.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; leverage shared types from packages
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
apps/web/app/s/[videoId]/_components/ShareHeader.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
apps/web/app/s/[videoId]/_components/ShareHeader.tsx
apps/web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
On the client, always use
useEffectQueryoruseEffectMutationfrom@/lib/EffectRuntime; never callEffectRuntime.run*directly in components.
Files:
apps/web/app/s/[videoId]/_components/ShareHeader.tsx
🧬 Code graph analysis (1)
apps/web/app/s/[videoId]/_components/ShareHeader.tsx (1)
packages/database/schema.ts (1)
videos(256-311)
⏰ 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 (1)
apps/web/app/s/[videoId]/_components/ShareHeader.tsx (1)
219-224: Avatar fallback looks good.
This PR:
Summary by CodeRabbit
New Features
Bug Fixes
Style