Make dashboard video links reliable public shares#48
Open
bmdavis419 wants to merge 2 commits into
Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughIntroduces ChangesDashboard Access Control & Share UX
Sequence Diagram(s)sequenceDiagram
participant User
participant DashboardLayout
participant resolveDashboardAccess
participant ConvexWorkspace
participant ClerkAuth
User->>DashboardLayout: navigate to dashboard route
DashboardLayout->>ClerkAuth: useAuth() / useConvexAuth()
ClerkAuth-->>DashboardLayout: {isLoaded, userId, isAuthenticated}
DashboardLayout->>ConvexWorkspace: resolveContext(rawProjectId, rawVideoId) [conditional]
ConvexWorkspace-->>DashboardLayout: workspaceContext | null
DashboardLayout->>resolveDashboardAccess: resolve(clerkState, convexAuthState, workspaceContext, publicId)
resolveDashboardAccess-->>DashboardLayout: access.kind (loading | dashboard | redirect-public | redirect-sign-in | not-found | auth-unavailable)
alt access.kind === "dashboard"
DashboardLayout-->>User: render dashboard content
else access.kind === "redirect-public"
DashboardLayout-->>User: window.location.replace(watchPath)
else access.kind === "redirect-sign-in"
DashboardLayout-->>User: window.location.replace(/sign-in?redirect_url=...)
else access.kind === "loading"
DashboardLayout-->>User: "Checking access..." screen
else access.kind === "not-found" | "auth-unavailable"
DashboardLayout-->>User: error screen with recovery actions
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 1
🧹 Nitpick comments (2)
src/components/ShareDialog.tsx (1)
271-272: ⚡ Quick winAlign new feedback surfaces with the required palette.
Line 271 uses
bg-[#e8e8e0]and Line 297 usesbg-white; both conflict with the defined cream background system for this UI.Suggested fix
- <p className="border-2 border-[`#1a1a1a`] bg-[`#e8e8e0`] px-3 py-2 text-sm text-[`#1a1a1a`]"> + <p className="border-2 border-[`#1a1a1a`] bg-[`#f0f0e8`] px-3 py-2 text-sm text-[`#1a1a1a`]"> ... - className="mt-2 bg-white font-mono text-xs font-normal" + className="mt-2 bg-[`#f0f0e8`] font-mono text-xs font-normal"As per coding guidelines, "Use warm cream (
#f0f0e8) for backgrounds".Also applies to: 297-297
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ShareDialog.tsx` around lines 271 - 272, Update the background color styling in the ShareDialog component to match the defined warm cream palette. Replace the background color in the paragraph element at line 271 (currently using `bg-[`#e8e8e0`]`) and at line 297 (currently using `bg-white`) with the correct warm cream color `bg-[`#f0f0e8`]` as specified in the coding guidelines for UI backgrounds.Source: Coding guidelines
app/routes/dashboard/-project.tsx (1)
908-911: ⚡ Quick winUpdate share toast styling to match UI rules.
Line 908 introduces a strong shadow, and Line 919 uses
bg-white; both diverge from the declared primary UI styling rules for this codebase.Suggested fix
- "border-2 px-3 py-2 text-sm font-bold shadow-[4px_4px_0px_0px_var(--shadow-color)]", + "border-2 px-3 py-2 text-sm font-bold", ... - className="bg-white font-mono text-xs font-normal" + className="bg-[`#f0f0e8`] font-mono text-xs font-normal"As per coding guidelines, "Do not use gradients or shadows in primary UI elements (except subtle where functional)" and "Use warm cream (
#f0f0e8) for backgrounds".Also applies to: 919-919
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/routes/dashboard/-project.tsx` around lines 908 - 911, The shareToast styling violates UI guidelines by using a strong shadow effect and bg-white background. Remove the shadow-[4px_4px_0px_0px_var(--shadow-color)] class from the toast className to comply with the "no strong shadows in primary UI elements" rule, and replace any bg-white background class with bg-[`#f0f0e8`] to use the designated warm cream color throughout the share toast component, including the styling at line 919.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/ShareDialog.tsx`:
- Around line 67-69: The canManageSharing variable does not properly handle null
values for the video object. Currently, the condition checks video !== undefined
which returns true when video is null (since null !== undefined), potentially
allowing share management controls to be enabled when video is in an invalid
null state. Fix this by changing the condition to properly check that video is
not null, such as using video != null or video !== null instead of video !==
undefined, ensuring manage sharing actions are only enabled when a valid video
object exists and the user is not a viewer.
---
Nitpick comments:
In `@app/routes/dashboard/-project.tsx`:
- Around line 908-911: The shareToast styling violates UI guidelines by using a
strong shadow effect and bg-white background. Remove the
shadow-[4px_4px_0px_0px_var(--shadow-color)] class from the toast className to
comply with the "no strong shadows in primary UI elements" rule, and replace any
bg-white background class with bg-[`#f0f0e8`] to use the designated warm cream
color throughout the share toast component, including the styling at line 919.
In `@src/components/ShareDialog.tsx`:
- Around line 271-272: Update the background color styling in the ShareDialog
component to match the defined warm cream palette. Replace the background color
in the paragraph element at line 271 (currently using `bg-[`#e8e8e0`]`) and at
line 297 (currently using `bg-white`) with the correct warm cream color
`bg-[`#f0f0e8`]` as specified in the coding guidelines for UI backgrounds.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 297f2604-dba6-457d-926d-7033d322ad21
📒 Files selected for processing (12)
app/routes/dashboard/-layout.tsxapp/routes/dashboard/-project.tsxconvex/publicWatch.vitest.tsconvex/videos.tsconvex/workspace.tssrc/components/ShareDialog.tsxsrc/lib/clipboard.test.tssrc/lib/clipboard.tssrc/lib/dashboardAccess.test.tssrc/lib/dashboardAccess.tssrc/lib/requestEpoch.test.tssrc/lib/requestEpoch.ts
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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
User impact
Videos remain public by default. Copying a public dashboard video URL now works as a share link: team members stay in the dashboard, while everyone else is sent to the watch page. Private videos are never auto-published or exposed.
Validation
bun run checkgit diff --checkNo dev server or build command was run, per repository instructions.
Note
Make dashboard video links redirect unauthenticated users to public watch pages
resolveDashboardAccessto determine routing outcome (loading, dashboard, public redirect, sign-in redirect, not-found, auth-unavailable) from auth and workspace context state, replacing ad-hoc redirect logic inDashboardLayout.ProjectPageandShareDialognow copies a public watch URL (viawatchPath) when the video is public, with success/error feedback and a manual-copy fallback input on clipboard failure.workspace.resolveContextnow accepts raw string IDs and normalizes them, returning null for malformed input instead of throwing.videos.getPublicIdByVideoIdnow returns apublicIdfor public videos in any status (uploading, processing, ready, failed), not onlyready.copyTextToClipboardutility withnavigator.clipboardandexecCommandfallback, and acreateRequestEpochhelper to guard against stale async results.Macroscope summarized 16039fd.
Summary by CodeRabbit
New Features
Bug Fixes