Enforce recording limit #964
Conversation
WalkthroughAdds Instant Mode free-limit enforcement (5 minutes) with UI warnings and upgraded plan prompts across desktop UI and overlay. Introduces macOS WindowEvent::Focused handling in Tauri to hide TargetSelectOverlay when Upgrade gains focus and adjust activation policy. Web share pages receive styling updates and an owner-only upgrade banner with inline copy feedback. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as Recording UI (Instant Mode)
participant Auth as authStore
participant Timer as Elapsed Timer
participant Mut as toggleRecording
participant Win as Upgrade Window
User->>UI: Click Start
UI->>Auth: Check plan (upgraded?)
alt Not upgraded & Instant
UI->>Timer: Start tracking (adjustedTime)
UI-->>User: Show 5‑min tooltip/warnings
loop Each tick
Timer->>UI: elapsed time
UI->>UI: remainingRecordingTime()
alt Remaining <= 0 and not aborted
UI->>Mut: Stop recording
UI->>UI: Set aborted flag
else Continue
UI-->>User: Show remaining time
end
end
User-->>Win: Click Upgrade
Win->>Win: commands.showWindow("Upgrade")
else Upgraded or non‑Instant
UI->>Timer: Start/Stop normal
UI-->>User: Show elapsed time
end
sequenceDiagram
autonumber
participant OS as macOS
participant App as Tauri App
participant W as WebviewWindow(s)
participant Policy as Activation Policy
OS-->>App: WindowEvent::Focused(window, focused)
App->>App: Parse window label -> window_id
alt focused == true
alt window_id == Upgrade
App->>W: Iterate webview windows
W-->>App: Identify TargetSelectOverlay labeled windows
App->>W: Hide those overlays
end
alt activates_dock(window_id)
App->>Policy: Set to Regular
end
else focused == false
Note over App: No new behavior
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing Touches
🧪 Generate unit tests
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: 2
🧹 Nitpick comments (17)
apps/web/app/s/[videoId]/_components/tabs/Activity/Comments.tsx (1)
216-219: Typo in Omit key: placholder → placeholderThis typo defeats the intent to block overriding CommentInput’s placeholder via commentInputProps.
Apply:
- "user" | "placholder" | "buttonLabel" + "user" | "placeholder" | "buttonLabel"apps/desktop/src-tauri/src/lib.rs (1)
2220-2239: Gate overlay-hiding to focus gained to avoid unnecessary hidesCurrently runs on both focus and blur; gate under focused==true.
Apply:
- WindowEvent::Focused(focused) => { - let window_id = CapWindowId::from_str(label); - - if matches!(window_id, Ok(CapWindowId::Upgrade)) { - for (label, window) in app.webview_windows() { - if let Ok(id) = CapWindowId::from_str(&label) - && matches!(id, CapWindowId::TargetSelectOverlay { .. }) - { - let _ = window.hide(); - } - } - } - - if *focused { - if let Ok(window_id) = window_id - && window_id.activates_dock() - { - app.set_activation_policy(tauri::ActivationPolicy::Regular) - .ok(); - } - } - } + WindowEvent::Focused(focused) => { + let window_id = CapWindowId::from_str(label); + if *focused { + if matches!(window_id, Ok(CapWindowId::Upgrade)) { + for (label, window) in app.webview_windows() { + if let Ok(id) = CapWindowId::from_str(&label) + && matches!(id, CapWindowId::TargetSelectOverlay { .. }) + { + let _ = window.hide(); + } + } + } + if let Ok(window_id) = window_id + && window_id.activates_dock() + { + app.set_activation_policy(tauri::ActivationPolicy::Regular).ok(); + } + } + }apps/web/app/s/[videoId]/Share.tsx (2)
248-259: Simplify absolute child sizing; inset already constrains width/height
inset-3makes the extraw-[calc(100%-1.5rem)] h-[calc(100%-1.5rem)]redundant. This trims CSS and avoids double-calculation.-<div className="absolute inset-3 w-[calc(100%-1.5rem)] h-[calc(100%-1.5rem)] overflow-hidden rounded-xl"> +<div className="absolute inset-3 overflow-hidden rounded-xl">
340-375: AI card styling LGTM; also replace the remainingnew-card-stylein the skeleton for consistencyNice move to explicit Tailwind classes here. For consistency (and per “Tailwind only” guideline), also update the skeleton wrapper to drop
new-card-style.Apply at Line 308:
-<div className="p-4 animate-pulse new-card-style"> +<div className="p-4 animate-pulse bg-white rounded-2xl border border-gray-3">apps/web/app/s/[videoId]/_components/tabs/Activity/CommentInput.tsx (2)
54-63: UI polish looks good; add an aria-label to the textareaSmall a11y boost: add an explicit label for screen readers instead of relying on placeholder.
<textarea ref={inputRef} value={content} disabled={disabled} onChange={(e) => setContent(e.target.value)} onKeyDown={handleKeyDown} placeholder={placeholder || "Leave a comment..."} + aria-label={placeholder || "Leave a comment"} className="w-full placeholder:text-gray-8 text-sm leading-[22px] text-gray-12 bg-transparent focus:outline-none" />
65-71: Disable the submit button when the component itself is disabledPrevents clickable “Reply” when
disabledis true (textarea is disabled, but button is not).- disabled={!content} + disabled={!content || disabled}apps/web/app/s/[videoId]/_components/ShareHeader.tsx (2)
172-188: Add ARIA semantics to the sticky bannerHelps announce the upgrade notice to assistive tech.
-<div className="flex sticky flex-col sm:flex-row inset-x-0 top-0 z-10 gap-4 justify-center items-center px-3 py-2 mx-auto w-[calc(100%-20px)] max-w-fit rounded-b-xl border bg-gray-4 border-gray-6"> +<div role="region" aria-label="Upgrade banner" className="flex sticky flex-col sm:flex-row inset-x-0 top-0 z-10 gap-4 justify-center items-center px-3 py-2 mx-auto w-[calc(100%-20px)] max-w-fit rounded-b-xl border bg-gray-4 border-gray-6">
57-58: Clean up copy timeout and add error handling
- Add useRef to the React import.
- Store the timeout ID in copyTimeoutRef and clear it on unmount.
- Refactor the onClick handler to async/await the clipboard API, wrap in try/catch, and clear any existing timeout before setting a new one.
--- apps/web/app/s/[videoId]/_components/ShareHeader.tsx @@ -import { useEffect, useState } from "react"; +import { useEffect, useState, useRef } from "react"; @@ -const [linkCopied, setLinkCopied] = useState(false); +const [linkCopied, setLinkCopied] = useState(false); +const copyTimeoutRef = useRef<number | null>(null); + +useEffect(() => { + return () => { + if (copyTimeoutRef.current) { + clearTimeout(copyTimeoutRef.current); + } + }; +}, []); @@ -<Button - variant="white" - onClick={() => { - navigator.clipboard.writeText(getVideoLink()); - setLinkCopied(true); - setTimeout(() => { - setLinkCopied(false); - }, 2000); - }} -> +<Button + variant="white" + onClick={async () => { + try { + await navigator.clipboard.writeText(getVideoLink()); + setLinkCopied(true); + if (copyTimeoutRef.current) { + clearTimeout(copyTimeoutRef.current); + } + copyTimeoutRef.current = window.setTimeout(() => { + setLinkCopied(false); + }, 2000); + } catch { + toast.error("Failed to copy link. Please try again."); + } + }} +>Applies to lines 14 (import), 57–58 (state), and 243–258 (copy handler).
apps/web/app/s/[videoId]/page.tsx (2)
314-316: Remove unused idx; Effect.map provides a single argumentEffect.map only passes the mapped value; the second param will always be undefined and may trigger noUnusedParameters. Drop idx.
- Effect.map((data, idx) => ( + Effect.map((data) => (
710-717: Add rel to external target=_blank linkPrevent reverse tabnabbing by adding rel="noopener noreferrer".
- <a + <a target="_blank" href={`/?ref=video_${video.id}`} + rel="noopener noreferrer" className="flex justify-center items-center px-4 py-2 mx-auto mb-2 space-x-2 bg-white rounded-full border border-gray-5 w-fit" >apps/desktop/src/routes/in-progress-recording.tsx (2)
36-37: Extract limit to a shared constantConsider moving MAX_RECORDING_FOR_FREE to a shared constants module so the cap stays consistent across routes/components.
162-171: Tighten cutoff and simplify remaining-time clampCurrent logic can overshoot by one tick. Use >= for the cutoff and clamp remaining time with Math.max.
- if ( - isMaxRecordingLimitEnabled() && - adjustedTime() > MAX_RECORDING_FOR_FREE && - !aborted - ) { + if ( + isMaxRecordingLimitEnabled() && + adjustedTime() >= MAX_RECORDING_FOR_FREE && + !aborted + ) { aborted = true; stopRecording.mutate(); }- const remainingRecordingTime = () => { - if (MAX_RECORDING_FOR_FREE < adjustedTime()) return 0; - return MAX_RECORDING_FOR_FREE - adjustedTime(); - }; + const remainingRecordingTime = () => + Math.max(0, MAX_RECORDING_FOR_FREE - adjustedTime());Also applies to: 172-183, 184-188, 229-234
apps/desktop/src/routes/target-select-overlay.tsx (2)
44-49: Unify options source in InnerInner still uses createOptionsQuery while children use useRecordingOptions. Prefer the context hook in Inner too to avoid duplicate state sources.
-function Inner() { +function Inner() { const [params] = useSearchParams<{ displayId: DisplayId }>(); - const { rawOptions, setOptions } = createOptionsQuery(); + const { rawOptions, setOptions } = useRecordingOptions();
838-856: Avoid duplicate auth queries in warningShowCapFreeWarning calls authStore.createQuery again. Optionally pass upgraded as a prop from parent (or read from a shared context) to cut redundant queries.
apps/desktop/src/routes/(window-chrome)/(main).tsx (3)
496-501: Add button type and track the Upgrade CTA clickPrevents default submit behavior and gives you a funnel metric for this entry point.
- <button - class="underline" - onClick={() => commands.showWindow("Upgrade")} - > + <button + type="button" + class="underline" + onClick={() => { + trackEvent("upgrade_cta_clicked", { source: "instant_limit_tooltip" }); + commands.showWindow("Upgrade"); + }} + >
506-511: Disable tooltip while recordingAvoids showing an upgrade promo when the user is already recording.
- disabled={ - !( - rawOptions.mode === "instant" && - auth.data?.plan?.upgraded === false - ) - } + disabled={ + !( + rawOptions.mode === "instant" && + auth.data?.plan?.upgraded === false + ) || isRecording() + }
493-504: De-hardcode the 5-min limit and standardize copyUse a shared constant to keep UI text in sync with enforcement logic and avoid “min/mins” drift.
- content={ - <> - Instant Mode recordings are limited - <br /> to 5 mins,{" "} + content={ + <> + Instant Mode recordings are limited + <br /> to {MAX_RECORDING_FOR_FREE_MINUTES} min,{" "} <button class="underline" onClick={() => commands.showWindow("Upgrade")} > Upgrade to Pro </button> </> }- {rawOptions.mode === "instant" && - auth.data?.plan?.upgraded === false - ? "Start 5 min recording" - : "Start recording"} + {rawOptions.mode === "instant" && + auth.data?.plan?.upgraded === false + ? `Start ${MAX_RECORDING_FOR_FREE_MINUTES} min recording` + : "Start recording"}Add a shared constant (example):
// apps/desktop/src/constants.ts export const MAX_RECORDING_FOR_FREE_MINUTES = 5 as const;And import it here:
import { MAX_RECORDING_FOR_FREE_MINUTES } from "~/constants";If you'd like, I can hoist the existing MAX_RECORDING_FOR_FREE from in-progress-recording.tsx into this constants module and wire both call sites to it.
Also applies to: 543-546
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
apps/desktop/src-tauri/src/lib.rs(1 hunks)apps/desktop/src/routes/(window-chrome)/(main).tsx(1 hunks)apps/desktop/src/routes/in-progress-recording.tsx(5 hunks)apps/desktop/src/routes/target-select-overlay.tsx(6 hunks)apps/web/app/s/[videoId]/Share.tsx(2 hunks)apps/web/app/s/[videoId]/_components/ShareHeader.tsx(6 hunks)apps/web/app/s/[videoId]/_components/Sidebar.tsx(1 hunks)apps/web/app/s/[videoId]/_components/Toolbar.tsx(2 hunks)apps/web/app/s/[videoId]/_components/tabs/Activity/CommentInput.tsx(1 hunks)apps/web/app/s/[videoId]/_components/tabs/Activity/Comments.tsx(1 hunks)apps/web/app/s/[videoId]/page.tsx(3 hunks)packages/ui/src/components/Button.tsx(1 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 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/s/[videoId]/_components/tabs/Activity/CommentInput.tsxapps/web/app/s/[videoId]/_components/Toolbar.tsxapps/web/app/s/[videoId]/_components/tabs/Activity/Comments.tsxapps/web/app/s/[videoId]/Share.tsxapps/web/app/s/[videoId]/_components/Sidebar.tsxapps/web/app/s/[videoId]/page.tsxapps/web/app/s/[videoId]/_components/ShareHeader.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/s/[videoId]/_components/tabs/Activity/CommentInput.tsxapps/web/app/s/[videoId]/_components/Toolbar.tsxapps/web/app/s/[videoId]/_components/tabs/Activity/Comments.tsxapps/web/app/s/[videoId]/Share.tsxpackages/ui/src/components/Button.tsxapps/web/app/s/[videoId]/_components/Sidebar.tsxapps/web/app/s/[videoId]/page.tsxapps/web/app/s/[videoId]/_components/ShareHeader.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; prefer shared types from packages
Files:
apps/web/app/s/[videoId]/_components/tabs/Activity/CommentInput.tsxapps/web/app/s/[videoId]/_components/Toolbar.tsxapps/web/app/s/[videoId]/_components/tabs/Activity/Comments.tsxapps/web/app/s/[videoId]/Share.tsxapps/desktop/src/routes/in-progress-recording.tsxpackages/ui/src/components/Button.tsxapps/web/app/s/[videoId]/_components/Sidebar.tsxapps/web/app/s/[videoId]/page.tsxapps/desktop/src/routes/target-select-overlay.tsxapps/web/app/s/[videoId]/_components/ShareHeader.tsxapps/desktop/src/routes/(window-chrome)/(main).tsx
apps/desktop/src-tauri/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Use tauri_specta on the Rust side for IPC (typed commands/events) and emit events via generated types
Files:
apps/desktop/src-tauri/src/lib.rs
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/desktop/**/*.{ts,tsx}: In the desktop app, rely on unplugin-icons auto-imports; do not manually import icon modules
Use generated tauri_specta commands/events (commands, events) in the desktop frontend; listen to generated events directly
Use @tanstack/solid-query for server state in the desktop app
Files:
apps/desktop/src/routes/in-progress-recording.tsxapps/desktop/src/routes/target-select-overlay.tsxapps/desktop/src/routes/(window-chrome)/(main).tsx
{apps/desktop,packages/ui-solid}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Component naming (Solid): components in PascalCase; hooks/utilities in camelCase starting with 'use' where applicable
Files:
apps/desktop/src/routes/in-progress-recording.tsxapps/desktop/src/routes/target-select-overlay.tsxapps/desktop/src/routes/(window-chrome)/(main).tsx
⏰ 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 (14)
apps/web/app/s/[videoId]/_components/tabs/Activity/Comments.tsx (1)
231-231: Styling palette alignment — LGTMBorder/background updates match the new gray scale across the share view.
packages/ui/src/components/Button.tsx (1)
24-24: White variant border color tweak — LGTMSwitch to border-gray-5 is consistent with the broader card/border refresh.
apps/web/app/s/[videoId]/_components/Toolbar.tsx (1)
192-192: Toolbar border and separator color refresh — LGTMMatches the new card + divider palette; no behavior changes.
Also applies to: 275-275
apps/web/app/s/[videoId]/_components/Sidebar.tsx (2)
158-158: Card styling update — LGTMbg-white + rounded-2xl + border-gray-5 aligns with the new container style.
Also applies to: 160-160
161-170: Ignore corrupted/garbled characters comment – Sidebar.tsx contains no non-ASCII or garbled content.Likely an incorrect or invalid review comment.
apps/web/app/s/[videoId]/_components/ShareHeader.tsx (2)
11-11: Imports LGTMIcon additions are correct and used below.
262-262: Explicit button type is goodPrevents accidental form submissions when placed inside forms.
apps/web/app/s/[videoId]/page.tsx (1)
678-709: Layout/padding tweak looks goodContainer padding change aligns with the new share-page styling.
apps/desktop/src/routes/in-progress-recording.tsx (1)
162-171: Confirm intended behavior while auth is loadingUntil auth.data resolves, the limit isn’t enforced. That’s okay if intentional; otherwise treat “unknown” as not-upgraded.
apps/desktop/src/routes/target-select-overlay.tsx (5)
34-37: Good: centralizing options via contextRecordingOptionsProvider/useRecordingOptions introduction is sound.
176-181: Upgrade nudge placement looks rightCap-free warning is injected in all relevant flows (display, window, area).
Also applies to: 246-249, 681-684
327-329: Delta change is fine; clamping covers negativesDropping Math.max on drag deltas is safe given downstream clamping in setBounds.
705-706: Correct: useRecordingOptions in RecordingControlsSwitching RecordingControls to the options context keeps state consistent.
846-851: Verify ‘Upgrade’ window registration
Ensure the literal “Upgrade” passed tocommands.showWindow("Upgrade")in apps/desktop/src/routes/target-select-overlay.tsx (lines 846–851) exactly matches the id used when registering or creating that window in the main process.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
apps/web/app/s/[videoId]/_components/tabs/Activity/Comments.tsx (1)
216-219: Fix typo in Omit key: 'placholder' → 'placeholder'.Type-level bug currently doesn’t omit the prop.
- componentProps?: Omit< - ComponentProps<typeof CommentInput>, - "user" | "placholder" | "buttonLabel" - >; + componentProps?: Omit< + ComponentProps<typeof CommentInput>, + "user" | "placeholder" | "buttonLabel" + >;apps/web/app/s/[videoId]/_components/Sidebar.tsx (1)
157-170: Corrupted/gibberish content in render; this will break build.There’s binary garbage after the button className block; likely a paste/encoding issue. This must be removed.
Proposed restoration for the mapped tab button:
- className={classNames( - "flex-1 px-5 py-3 text-sm font-medium relative transition-colors duration-200", - "hover:bg-gray-1", - activeTab === tab.id ? "bg-gray-3" : "", - )} - > 8#s�... + className={classNames( + "flex-1 px-5 py-3 text-sm font-medium relative transition-colors duration-200", + "hover:bg-gray-1", + activeTab === tab.id ? "bg-gray-3" : "", + )} + > + {tab.label} + </button>Please ensure the closing for the tabs container remains balanced after this fix.
apps/desktop/src/routes/in-progress-recording.tsx (1)
64-75: Bug: aborted never resets after restart; subsequent recordings won’t re-stop at 5 minAfter hitting the limit once,
abortedstays true across restarts within the same view; the effect will no-op and not enforce the limit again.Apply:
} else if (payload.variant === "Started") { + aborted = false; setState({ variant: "recording" }); setStart(Date.now()); }await commands.restartRecording(); + aborted = false; setState({ variant: "recording" }); setTime(Date.now());Also applies to: 122-136, 172-183
apps/web/app/s/[videoId]/page.tsx (1)
698-705: Add rel="noopener noreferrer" for target="_blank" linkPrevents reverse tabnabbing and isolates the opener context.
- <a - target="_blank" - href={`/?ref=video_${video.id}`} + <a + target="_blank" + rel="noopener noreferrer" + href={`/?ref=video_${video.id}`}apps/desktop/src/routes/target-select-overlay.tsx (1)
26-26: UseuseRecordingOptionsinstead of directcreateOptionsQuery
In apps/desktop/src/routes/target-select-overlay.tsx (around line 53), remove the stale import and replace the hook call:- import { createOptionsQuery } from "~/utils/queries"; + // remove this import - const { rawOptions, setOptions } = createOptionsQuery(); + const { rawOptions, setOptions } = useRecordingOptions();This ensures the component shares the context-backed state and avoids desync.
♻️ Duplicate comments (3)
apps/desktop/src/routes/in-progress-recording.tsx (1)
16-16: Unify entitlement gating to licenseQuery and default to conservative while loadingElsewhere (main.tsx) you already have createLicenseQuery; here you’re mixing auth.plan.upgraded. This risks inconsistent UX (e.g., commercial users limited) and bypass while auth loads.
Apply:
-import { authStore } from "~/store"; +// no auth gating here; use licenseQuery for a single source of truth-import { - createCurrentRecordingQuery, - createOptionsQuery, -} from "~/utils/queries"; +import { + createCurrentRecordingQuery, + createOptionsQuery, + createLicenseQuery, +} from "~/utils/queries";-const auth = authStore.createQuery(); +const license = createLicenseQuery();-const isMaxRecordingLimitEnabled = () => { - // Only enforce the limit on instant mode. - // We enforce it on studio mode when exporting. - return ( - optionsQuery.rawOptions.mode === "instant" && - // If the data is loaded and the user is not upgraded - auth.data?.plan?.upgraded === false - ); -}; +const isMaxRecordingLimitEnabled = () => { + // Only enforce the limit on instant mode. + // Studio mode enforcement happens on export. + if (optionsQuery.rawOptions.mode !== "instant") return false; + // Be conservative while loading to avoid bypass. + if (license.isLoading) return true; + return !["pro", "commercial"].includes(license.data?.type ?? "personal"); +};Also applies to: 52-53, 162-171, 229-234
apps/desktop/src/routes/(window-chrome)/(main).tsx (1)
490-512: Use licenseQuery for Instant-mode 5‑min gating (avoid auth.plan.upgraded)Keep entitlement checks consistent and include a conservative loading default so the tooltip/label behavior matches enforcement.
- disabled={ - !( - rawOptions.mode === "instant" && - auth.data?.plan?.upgraded === false - ) - } + disabled={ + !( + rawOptions.mode === "instant" && + (license.isLoading ? true : license.data?.type === "personal") + ) + }- {rawOptions.mode === "instant" && - auth.data?.plan?.upgraded === false + {rawOptions.mode === "instant" && + (license.isLoading ? true : license.data?.type === "personal") ? "Start 5 min recording" : "Start recording"}Also applies to: 543-546
apps/web/app/s/[videoId]/_components/ShareHeader.tsx (1)
63-64: Normalize owner check and reuse computed flags (prevents banner not rendering).Unify on the already-normalized owner check and the computed isUserPro. Also coerce isOwner to a real boolean. This addresses the prior review about ID type mismatch and avoids a second userIsPro() call.
-const isOwner = user && user.id.toString() === data.ownerId; +const isOwner = Boolean(user && user.id.toString() === data.ownerId); const isUserPro = userIsPro(user); -const showUpgradeBanner = - user && data.ownerId === user.id && !userIsPro(user); +const showUpgradeBanner = Boolean(isOwner && !isUserPro);Also applies to: 129-131
🧹 Nitpick comments (11)
packages/ui/src/components/Button.tsx (1)
69-74: Remove any casts in polymorphic Button.Current casts hide type safety when href/asChild are used.
Apply a minimal typing improvement:
- const Comp = href ? "a" : asChild ? Slot : ("button" as any); + const Comp: React.ElementType = href ? "a" : asChild ? Slot : "button"; - ref={ref as any} + ref={ref}If stricter polymorphic typing is desired, I can follow up with a typed Polymorphic version.
apps/web/app/s/[videoId]/_components/Toolbar.tsx (2)
236-239: Avoid double getTimestamp() computation.Minor nit: compute once to prevent duplicate reads.
- {videoElement && getTimestamp() > 0 - ? `Comment at ${getTimestamp().toFixed(2)}` + {videoElement && (() => { const ts = getTimestamp(); return ts > 0 + ? `Comment at ${ts.toFixed(2)}` : "Comment"}
33-46: Prefer MutationObserver over polling for element availability.Polling every 100ms is okay, but an observer is cleaner and avoids timers.
I can provide a small MutationObserver snippet if you want to switch.
apps/web/app/s/[videoId]/_components/tabs/Activity/CommentInput.tsx (1)
65-71: Disable submit based on trimmed content.Currently whitespace-only content enables the button but is rejected on submit.
- disabled={!content} + disabled={!content.trim()}If desired, mirror this check in handleSubmit to keep logic aligned.
apps/desktop/src-tauri/src/lib.rs (1)
2220-2240: Upgrade focus behavior: good; consider restoring overlays on blur.Hiding TargetSelectOverlay when Upgrade gains focus is correct. If the Upgrade window stays open and loses focus (without being destroyed), overlays remain hidden; consider showing them back on blur.
#[cfg(target_os = "macos")] WindowEvent::Focused(focused) => { let window_id = CapWindowId::from_str(label); if matches!(window_id, Ok(CapWindowId::Upgrade)) { for (label, window) in app.webview_windows() { if let Ok(id) = CapWindowId::from_str(&label) && matches!(id, CapWindowId::TargetSelectOverlay { .. }) { - let _ = window.hide(); + if *focused { + let _ = window.hide(); + } else { + let _ = window.show(); + } } } } if *focused { if let Ok(window_id) = window_id && window_id.activates_dock() { app.set_activation_policy(tauri::ActivationPolicy::Regular).ok(); } } }Alternatively, rely solely on Destroyed to reshow if that’s the intended UX; in that case, ignore this.
apps/desktop/src/routes/in-progress-recording.tsx (1)
174-181: Tighten boundary checks to stop exactly at 5:00 and keep UI consistentUse >= for the cutoff and <= for the remaining-time zeroing.
- adjustedTime() > MAX_RECORDING_FOR_FREE && + adjustedTime() >= MAX_RECORDING_FOR_FREE &&- if (MAX_RECORDING_FOR_FREE < adjustedTime()) return 0; + if (adjustedTime() >= MAX_RECORDING_FOR_FREE) return 0;Also applies to: 185-187
apps/web/app/s/[videoId]/_components/ShareHeader.tsx (2)
245-257: Avoid dangling timers when unmounting (minor leak) and handle copy failure.Store/clear the timeout on unmount and surface clipboard write errors.
-import { useEffect, useState } from "react"; +import { useEffect, useRef, useState } from "react";-const [linkCopied, setLinkCopied] = useState(false); +const [linkCopied, setLinkCopied] = useState(false); +const copyTimerRef = useRef<number | null>(null); + +useEffect(() => { + return () => { + if (copyTimerRef.current) window.clearTimeout(copyTimerRef.current); + }; +}, []);- navigator.clipboard.writeText(getVideoLink()); - setLinkCopied(true); - setTimeout(() => { - setLinkCopied(false); - }, 2000); + navigator.clipboard.writeText(getVideoLink()) + .then(() => { + setLinkCopied(true); + if (copyTimerRef.current) window.clearTimeout(copyTimerRef.current); + copyTimerRef.current = window.setTimeout(() => { + setLinkCopied(false); + }, 2000); + }) + .catch(() => toast.error("Failed to copy link"));Also applies to: 14-14, 57-58
254-257: Prefer Tailwind for animations; drop custom css class.Replace the custom svgpathanimation with a Tailwind animation to align with styling guidelines.
- <Check className="ml-2 w-4 h-4 svgpathanimation" /> + <Check className="ml-2 h-4 w-4 motion-safe:animate-pulse" />apps/desktop/src/routes/target-select-overlay.tsx (3)
43-49: Name the default-exported component (PascalCase) per guidelines.Improves devtools traces and follows component naming rules.
-export default function () { +export default function TargetSelectOverlay() { return ( <RecordingOptionsProvider> <Inner /> </RecordingOptionsProvider> ); }
180-181: Let the warning read from context; remove prop drilling.Have ShowCapFreeWarning derive isInstantMode from useRecordingOptions(), and simplify callsites.
-<ShowCapFreeWarning isInstantMode={rawOptions.mode === "instant"} /> +<ShowCapFreeWarning />-<ShowCapFreeWarning - isInstantMode={rawOptions.mode === "instant"} -/> +<ShowCapFreeWarning />-<ShowCapFreeWarning - isInstantMode={rawOptions.mode === "instant"} -/> +<ShowCapFreeWarning />-function ShowCapFreeWarning(props: { isInstantMode: boolean }) { - const auth = authStore.createQuery(); - - return ( - <Suspense> - <Show when={props.isInstantMode && auth.data?.plan?.upgraded === false}> - <p class="text-sm text-center max-w-64"> - Instant Mode recordings are limited to 5 mins,{" "} - <button - class="underline" - onClick={() => commands.showWindow("Upgrade")} - > - Upgrade to Pro - </button> - </p> - </Show> - </Suspense> - ); -} +function ShowCapFreeWarning() { + const auth = authStore.createQuery(); + const { rawOptions } = useRecordingOptions(); + return ( + <Suspense> + <Show when={rawOptions.mode === "instant" && auth.data?.plan?.upgraded === false}> + <p class="max-w-64 text-center text-sm"> + Instant Mode recordings are limited to 5 minutes,{" "} + <button class="underline" onClick={() => commands.showWindow("Upgrade")}> + Upgrade to Pro + </button> + </p> + </Show> + </Suspense> + ); +}Also applies to: 246-248, 681-683, 838-856
845-852: Copy nit: “5 minutes” reads better than “5 mins”.-Instant Mode recordings are limited to 5 mins, +Instant Mode recordings are limited to 5 minutes,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
apps/desktop/src-tauri/src/lib.rs(1 hunks)apps/desktop/src/routes/(window-chrome)/(main).tsx(1 hunks)apps/desktop/src/routes/in-progress-recording.tsx(5 hunks)apps/desktop/src/routes/target-select-overlay.tsx(6 hunks)apps/web/app/s/[videoId]/Share.tsx(2 hunks)apps/web/app/s/[videoId]/_components/ShareHeader.tsx(6 hunks)apps/web/app/s/[videoId]/_components/Sidebar.tsx(1 hunks)apps/web/app/s/[videoId]/_components/Toolbar.tsx(2 hunks)apps/web/app/s/[videoId]/_components/tabs/Activity/CommentInput.tsx(1 hunks)apps/web/app/s/[videoId]/_components/tabs/Activity/Comments.tsx(1 hunks)apps/web/app/s/[videoId]/page.tsx(3 hunks)packages/ui/src/components/Button.tsx(1 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 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/s/[videoId]/_components/tabs/Activity/Comments.tsxapps/web/app/s/[videoId]/_components/tabs/Activity/CommentInput.tsxapps/web/app/s/[videoId]/_components/Toolbar.tsxapps/web/app/s/[videoId]/page.tsxapps/web/app/s/[videoId]/Share.tsxapps/web/app/s/[videoId]/_components/Sidebar.tsxapps/web/app/s/[videoId]/_components/ShareHeader.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/s/[videoId]/_components/tabs/Activity/Comments.tsxapps/web/app/s/[videoId]/_components/tabs/Activity/CommentInput.tsxapps/web/app/s/[videoId]/_components/Toolbar.tsxapps/web/app/s/[videoId]/page.tsxpackages/ui/src/components/Button.tsxapps/web/app/s/[videoId]/Share.tsxapps/web/app/s/[videoId]/_components/Sidebar.tsxapps/web/app/s/[videoId]/_components/ShareHeader.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; prefer shared types from packages
Files:
apps/web/app/s/[videoId]/_components/tabs/Activity/Comments.tsxapps/web/app/s/[videoId]/_components/tabs/Activity/CommentInput.tsxapps/web/app/s/[videoId]/_components/Toolbar.tsxapps/web/app/s/[videoId]/page.tsxapps/desktop/src/routes/in-progress-recording.tsxpackages/ui/src/components/Button.tsxapps/web/app/s/[videoId]/Share.tsxapps/desktop/src/routes/(window-chrome)/(main).tsxapps/web/app/s/[videoId]/_components/Sidebar.tsxapps/desktop/src/routes/target-select-overlay.tsxapps/web/app/s/[videoId]/_components/ShareHeader.tsx
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/desktop/**/*.{ts,tsx}: In the desktop app, rely on unplugin-icons auto-imports; do not manually import icon modules
Use generated tauri_specta commands/events (commands, events) in the desktop frontend; listen to generated events directly
Use @tanstack/solid-query for server state in the desktop app
Files:
apps/desktop/src/routes/in-progress-recording.tsxapps/desktop/src/routes/(window-chrome)/(main).tsxapps/desktop/src/routes/target-select-overlay.tsx
{apps/desktop,packages/ui-solid}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Component naming (Solid): components in PascalCase; hooks/utilities in camelCase starting with 'use' where applicable
Files:
apps/desktop/src/routes/in-progress-recording.tsxapps/desktop/src/routes/(window-chrome)/(main).tsxapps/desktop/src/routes/target-select-overlay.tsx
apps/desktop/src-tauri/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Use tauri_specta on the Rust side for IPC (typed commands/events) and emit events via generated types
Files:
apps/desktop/src-tauri/src/lib.rs
⏰ 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 (12)
packages/ui/src/components/Button.tsx (1)
24-24: Border color tweak looks good.Aligns with the new gray scale; no API changes.
apps/web/app/s/[videoId]/_components/Toolbar.tsx (2)
192-193: Toolbar container border update is consistent.The added border/border-gray-5 matches the new card pattern.
275-275: Separator color update is consistent.bg-gray-5 aligns with the updated palette.
apps/web/app/s/[videoId]/_components/tabs/Activity/CommentInput.tsx (2)
54-55: Border color change LGTM.Matches the rest of the PR’s border-gray-5 updates.
61-63: Keep placeholder copy consistent; styling looks good.The “...” and placeholder color update are fine.
apps/web/app/s/[videoId]/_components/tabs/Activity/Comments.tsx (1)
231-248: Bottom bar palette tweak looks good.border-gray-5 + bg-gray-2 is consistent with the card updates.
apps/web/app/s/[videoId]/_components/Sidebar.tsx (1)
157-161: Card container/border updates are consistent.bg-white + rounded-2xl + border-gray-5 aligns with the new design language.
apps/web/app/s/[videoId]/Share.tsx (3)
245-256: Video container card styling LGTM.The move to a bordered white card with internal inset preserves layout and simplifies spacing.
337-371: AI summary/chapters card styling LGTM.White card + border-gray-3 reads well against bg-gray-2 pages.
6-15: Versions align across monorepo
All React-using packages (apps/web, packages/database, ui, utils) pin React ^19.1.0, and apps/web is on @tanstack/react-query ^5.75.4—no mismatches detected.apps/desktop/src/routes/in-progress-recording.tsx (1)
36-37: Good: single constant for free limitCentralizing the 5-minute threshold is clean and makes future changes easy.
apps/web/app/s/[videoId]/page.tsx (1)
308-308: Style updates look consistent with the new token paletteContainer/bg/border changes align with the broader Share page refresh.
Also applies to: 666-666, 701-701
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/desktop/src/routes/(window-chrome)/new-main/SystemAudio.tsx (1)
16-16: Fix typo in className: curosr-default -> cursor-defaultThe misspelling prevents the intended cursor style.
Apply this diff:
- class="flex flex-row gap-2 items-center px-2 w-full h-9 rounded-lg transition-colors curosr-default disabled:opacity-70 bg-gray-3 disabled:text-gray-11 KSelect" + class="flex flex-row gap-2 items-center px-2 w-full h-9 rounded-lg transition-colors cursor-default disabled:opacity-70 bg-gray-3 disabled:text-gray-11 KSelect"apps/desktop/src/routes/(window-chrome)/new-main/TargetTypeButton.tsx (1)
9-10: Don’t forward non-DOM props; add aria-disabledselected, Component, name, and disabled are being spread onto a div. Destructure and spread the rest; also expose aria-disabled for accessibility.
Apply this diff:
-function TargetTypeButton( - props: { - selected: boolean; - Component: Component<ComponentProps<"svg">>; - name: string; - disabled?: boolean; - } & ComponentProps<"div">, -) { +function TargetTypeButton({ + selected, + Component, + name, + disabled, + ...rest +}: { + selected: boolean; + Component: Component<ComponentProps<"svg">>; + name: string; + disabled?: boolean; +} & ComponentProps<"div">) { return ( <div - {...props} + {...rest} + aria-disabled={disabled} class={cx( "flex-1 text-center hover:bg-gray-4 bg-gray-3 flex flex-col ring-offset-gray-1 ring-offset-2 items-center justify-end gap-2 py-1.5 rounded-lg transition-all", - props.selected + selected ? "bg-gray-3 text-white ring-blue-9 ring-1" : "ring-transparent ring-0", - props.disabled ? "opacity-70 pointer-events-none" : "", + disabled ? "opacity-70 pointer-events-none" : "", )} > - <props.Component + <Component class={cx( "size-6 transition-colors", - props.selected ? "text-gray-12" : "text-gray-9", + selected ? "text-gray-12" : "text-gray-9", )} /> - <p class="text-xs text-gray-12">{props.name}</p> + <p class="text-xs text-gray-12">{name}</p> </div> ); }Also applies to: 14-21, 23-30
apps/desktop/src/routes/(window-chrome)/new-main/index.tsx (1)
395-403: DRY the “recording guard” for onClickYou repeat the early-return check in three handlers. Wrap once and reuse to reduce duplication.
Apply this diff to each onClick:
- onClick={() => { - //if recording early return - if (isRecording()) return; - setOptions("targetMode", (v) => - v === "display" ? null : "display", - ); - }} + onClick={withRecordingGuard(() => { + setOptions("targetMode", (v) => (v === "display" ? null : "display")); + })}- onClick={() => { - if (isRecording()) return; - setOptions("targetMode", (v) => (v === "window" ? null : "window")); - }} + onClick={withRecordingGuard(() => { + setOptions("targetMode", (v) => (v === "window" ? null : "window")); + })}- onClick={() => { - if (isRecording()) return; - setOptions("targetMode", (v) => (v === "area" ? null : "area")); - }} + onClick={withRecordingGuard(() => { + setOptions("targetMode", (v) => (v === "area" ? null : "area")); + })}Add this helper near isRecording():
const withRecordingGuard = (fn: () => void) => () => { if (isRecording()) return; fn(); };Also applies to: 408-413, 418-423
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/desktop/src/routes/(window-chrome)/new-main/CameraSelect.tsx(1 hunks)apps/desktop/src/routes/(window-chrome)/new-main/MicrophoneSelect.tsx(1 hunks)apps/desktop/src/routes/(window-chrome)/new-main/SystemAudio.tsx(1 hunks)apps/desktop/src/routes/(window-chrome)/new-main/TargetTypeButton.tsx(2 hunks)apps/desktop/src/routes/(window-chrome)/new-main/index.tsx(3 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/desktop/**/*.{ts,tsx}: In the desktop app, rely on unplugin-icons auto-imports; do not manually import icon modules
Use generated tauri_specta commands/events (commands, events) in the desktop frontend; listen to generated events directly
Use @tanstack/solid-query for server state in the desktop app
Files:
apps/desktop/src/routes/(window-chrome)/new-main/SystemAudio.tsxapps/desktop/src/routes/(window-chrome)/new-main/MicrophoneSelect.tsxapps/desktop/src/routes/(window-chrome)/new-main/CameraSelect.tsxapps/desktop/src/routes/(window-chrome)/new-main/index.tsxapps/desktop/src/routes/(window-chrome)/new-main/TargetTypeButton.tsx
{apps/desktop,packages/ui-solid}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Component naming (Solid): components in PascalCase; hooks/utilities in camelCase starting with 'use' where applicable
Files:
apps/desktop/src/routes/(window-chrome)/new-main/SystemAudio.tsxapps/desktop/src/routes/(window-chrome)/new-main/MicrophoneSelect.tsxapps/desktop/src/routes/(window-chrome)/new-main/CameraSelect.tsxapps/desktop/src/routes/(window-chrome)/new-main/index.tsxapps/desktop/src/routes/(window-chrome)/new-main/TargetTypeButton.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/desktop/src/routes/(window-chrome)/new-main/SystemAudio.tsxapps/desktop/src/routes/(window-chrome)/new-main/MicrophoneSelect.tsxapps/desktop/src/routes/(window-chrome)/new-main/CameraSelect.tsxapps/desktop/src/routes/(window-chrome)/new-main/index.tsxapps/desktop/src/routes/(window-chrome)/new-main/TargetTypeButton.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
React/Solid components should be named using PascalCase
Files:
apps/desktop/src/routes/(window-chrome)/new-main/SystemAudio.tsxapps/desktop/src/routes/(window-chrome)/new-main/MicrophoneSelect.tsxapps/desktop/src/routes/(window-chrome)/new-main/CameraSelect.tsxapps/desktop/src/routes/(window-chrome)/new-main/index.tsxapps/desktop/src/routes/(window-chrome)/new-main/TargetTypeButton.tsx
🧬 Code graph analysis (1)
apps/desktop/src/routes/(window-chrome)/new-main/index.tsx (1)
apps/desktop/src/utils/queries.ts (1)
createCurrentRecordingQuery(120-126)
⏰ 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 (6)
apps/desktop/src/routes/(window-chrome)/new-main/SystemAudio.tsx (1)
16-16: Consistent disabled styling addedAdding disabled:opacity-70 aligns this control with Camera/Microphone selectors. Looks good.
apps/desktop/src/routes/(window-chrome)/new-main/MicrophoneSelect.tsx (1)
69-69: Consistent disabled visual statedisabled:opacity-70 matches the rest of the UI and clearly communicates non-interactivity.
apps/desktop/src/routes/(window-chrome)/new-main/CameraSelect.tsx (1)
41-41: Consistent disabled visual stateThe added disabled:opacity-70 keeps styling uniform across selectors. Good change.
apps/desktop/src/routes/(window-chrome)/new-main/TargetTypeButton.tsx (1)
20-21: Good disabled UX guardOpacity + pointer-events-none prevents accidental interactions while recording.
apps/desktop/src/routes/(window-chrome)/new-main/index.tsx (2)
33-34: Good: use shared createCurrentRecordingQuery()Centralizing recording state via the shared query keeps concerns consistent across components.
116-118: Simple, reactive isRecording() helperClear and idiomatic for Solid Query state access.
Prevent studio upload that's more than 5 minutesAlready worksSummary by CodeRabbit
New Features
Style