Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughReplaces Bun tooling with pnpm/vp and integrates vite-plus (including staged/lint config), adds VS Code workspace settings and a pre-commit hook, updates CSS/font imports and Google Fonts links, removes the ESLint config, and applies widespread formatting adjustments across TS/React/Convex code and docs. Changes
Sequence Diagram(s)(silently omitted) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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: 4
🧹 Nitpick comments (2)
app/app.css (1)
421-562: Consider migrating hardcoded color utilities to semantic Tailwind classes.These attribute selectors mapping hardcoded hex values to CSS variables are a clever dark-mode workaround, but they're fragile—any change to the hex values in component code will silently break the mapping. This pattern also increases CSS bundle size significantly.
When time permits, consider refactoring components to use semantic Tailwind classes (e.g.,
bg-background,text-foreground) directly instead of hardcoded values. This would eliminate the need for these overrides.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/app.css` around lines 421 - 562, The stylesheet uses fragile attribute selectors like [class~="bg-[`#f0f0e8`]"], [class~="text-[`#1a1a1a`]"], and [class~="border-[`#cccccc`]"] to map hex utilities to CSS variables; replace usages of those hardcoded utilities in components with semantic Tailwind classes (e.g., bg-background, text-foreground, border-subtle) and remove the corresponding attribute-selector rules (all entries mapping bg[#...], text[#...], border[#...], hover: and group-hover: variants). If you need an incremental path, add semantic aliases to your Tailwind config (safelist or custom utilities) so components can switch to names like bg-surface-muted / text-foreground-muted, update components referencing bg-[#...]/text-[#...]/border-[#...] to the new semantic names, and once components are migrated, delete the matching attribute-selector blocks from the CSS.docs/deployment.md (1)
17-19: Call out thatCONVEX_DEPLOY_KEYshould be Production-only.
build:vercelshells out toconvex deploy, so exposing the production key to Preview would make preview builds deploy Convex too. A short note here would prevent an easy misconfiguration.Suggested wording
Required Vercel environment variable: -- `CONVEX_DEPLOY_KEY` (create a production deploy key in Convex and add it in Vercel project settings) +- `CONVEX_DEPLOY_KEY` (create a production deploy key in Convex and add it to the **Production** environment in Vercel project settings; do not expose it to Preview unless you intentionally want preview builds to run `convex deploy`)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/deployment.md` around lines 17 - 19, Add a clear note that the environment variable CONVEX_DEPLOY_KEY is a production-only secret because the npm script build:vercel runs the shell command convex deploy; update docs/deployment.md around the existing "Required Vercel environment variable" section to explicitly state that CONVEX_DEPLOY_KEY must only be set for Production (not Preview) in Vercel project settings to avoid preview builds invoking convex deploy.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.vite-hooks/pre-commit:
- Line 1: Add a shebang as the very first line of the pre-commit hook so the
script runs reliably across environments: insert a standard interpreter shebang
(for example using env to locate bash or sh) above the existing "vp staged"
line, and then ensure the hook file is executable (chmod +x) so Git can run it.
In `@app/app.css`:
- Around line 185-188: The CSS uses a quoted capitalized generic font family
"Georgia" in the --font-serif custom property which violates Stylelint's
value-keyword-case; update the --font-serif declaration (the --font-serif
variable) to use the lowercase generic family georgia without quotes so it reads
--font-serif: "Instrument Serif", georgia, serif; ensuring the generic keyword
is lowercase and unquoted.
- Line 5: Remove the unnecessary quotes around the single-word font name in the
font-family declarations: replace font-family: "Geist" with font-family: Geist
in all `@font-face` blocks that use "Geist" (the occurrences shown as font-family:
"Geist"); do not remove quotes for multi-word names like "Geist Mono"—leave
those quoted. Ensure all identical declarations (the repeated font-family:
"Geist") are updated consistently.
In `@app/routes/__root.tsx`:
- Around line 30-35: The preconnect for "https://fonts.googleapis.com" is
missing crossOrigin like the "https://fonts.gstatic.com" entry; update the
preconnect link object for fonts.googleapis.com to include crossOrigin:
"anonymous" for consistency, and (unless block behavior is intentional) change
the stylesheet href query from display=block to display=swap to avoid
FOIT—locate the link objects with rel: "preconnect" and the stylesheet href
containing "fonts.googleapis.com/css2?family=Instrument+Serif" in the array and
apply these two changes.
---
Nitpick comments:
In `@app/app.css`:
- Around line 421-562: The stylesheet uses fragile attribute selectors like
[class~="bg-[`#f0f0e8`]"], [class~="text-[`#1a1a1a`]"], and
[class~="border-[`#cccccc`]"] to map hex utilities to CSS variables; replace
usages of those hardcoded utilities in components with semantic Tailwind classes
(e.g., bg-background, text-foreground, border-subtle) and remove the
corresponding attribute-selector rules (all entries mapping bg[#...],
text[#...], border[#...], hover: and group-hover: variants). If you need an
incremental path, add semantic aliases to your Tailwind config (safelist or
custom utilities) so components can switch to names like bg-surface-muted /
text-foreground-muted, update components referencing
bg-[#...]/text-[#...]/border-[#...] to the new semantic names, and once
components are migrated, delete the matching attribute-selector blocks from the
CSS.
In `@docs/deployment.md`:
- Around line 17-19: Add a clear note that the environment variable
CONVEX_DEPLOY_KEY is a production-only secret because the npm script
build:vercel runs the shell command convex deploy; update docs/deployment.md
around the existing "Required Vercel environment variable" section to explicitly
state that CONVEX_DEPLOY_KEY must only be set for Production (not Preview) in
Vercel project settings to avoid preview builds invoking convex deploy.
🪄 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: 48d6cd77-7336-4ade-8f59-781d4c4cdc07
⛔ Files ignored due to path filters (3)
bun.lockis excluded by!**/*.lockconvex/_generated/api.d.tsis excluded by!**/_generated/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
.vite-hooks/pre-commit.vscode/extensions.json.vscode/settings.jsonapp/app.cssapp/routes/__root.tsxdocs/deployment.mddocs/setup.mdeslint.config.mjspackage.jsontsconfig.jsonvercel.jsonvite.config.ts
💤 Files with no reviewable changes (1)
- eslint.config.mjs
| @@ -0,0 +1 @@ | |||
| vp staged | |||
There was a problem hiding this comment.
Add a shebang to ensure proper script execution.
Pre-commit hooks need a shebang to be reliably executed across different environments. Without it, the hook may fail depending on how Git invokes it.
🔧 Proposed fix
+#!/bin/sh
vp staged📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| vp staged | |
| #!/bin/sh | |
| vp staged |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.vite-hooks/pre-commit at line 1, Add a shebang as the very first line of
the pre-commit hook so the script runs reliably across environments: insert a
standard interpreter shebang (for example using env to locate bash or sh) above
the existing "vp staged" line, and then ensure the hook file is executable
(chmod +x) so Git can run it.
| @font-face { | ||
| font-family: 'Geist'; | ||
| src: url('https://cdn.jsdelivr.net/npm/geist@1.2.0/dist/fonts/geist-sans/Geist-Regular.woff2') format('woff2'); | ||
| font-family: "Geist"; |
There was a problem hiding this comment.
Remove unnecessary quotes around font-family name.
Stylelint flags that "Geist" doesn't need quotes since it's a single-word font name without spaces or special characters. This applies to all @font-face declarations (lines 5, 14, 23, 32, 41, 50, 59).
🔧 Proposed fix (example for first occurrence)
`@font-face` {
- font-family: "Geist";
+ font-family: Geist;
src: url("https://cdn.jsdelivr.net/npm/geist@1.2.0/dist/fonts/geist-sans/Geist-Regular.woff2")Apply similar changes to:
- Line 14, 23, 32, 41:
"Geist"→Geist - Line 50, 59:
"Geist Mono"(keep quotes - has space)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| font-family: "Geist"; | |
| font-family: Geist; |
🧰 Tools
🪛 Stylelint (17.4.0)
[error] 5-5: Unexpected quotes around "Geist" (font-family-name-quotes)
(font-family-name-quotes)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/app.css` at line 5, Remove the unnecessary quotes around the single-word
font name in the font-family declarations: replace font-family: "Geist" with
font-family: Geist in all `@font-face` blocks that use "Geist" (the occurrences
shown as font-family: "Geist"); do not remove quotes for multi-word names like
"Geist Mono"—leave those quoted. Ensure all identical declarations (the repeated
font-family: "Geist") are updated consistently.
Comment on lines
+185
to
188
| --font-sans: "Geist", system-ui, -apple-system, sans-serif; | ||
| --font-mono: "Geist Mono", ui-monospace, monospace; | ||
| --font-serif: "Instrument Serif", Georgia, serif; | ||
| } |
There was a problem hiding this comment.
Fix casing for generic font family.
Per Stylelint's value-keyword-case rule, generic font family keywords should be lowercase. Change "Georgia" to georgia (unquoted) on line 187.
🔧 Proposed fix
--font-sans: "Geist", system-ui, -apple-system, sans-serif;
--font-mono: "Geist Mono", ui-monospace, monospace;
- --font-serif: "Instrument Serif", Georgia, serif;
+ --font-serif: "Instrument Serif", georgia, serif;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| --font-sans: "Geist", system-ui, -apple-system, sans-serif; | |
| --font-mono: "Geist Mono", ui-monospace, monospace; | |
| --font-serif: "Instrument Serif", Georgia, serif; | |
| } | |
| --font-sans: "Geist", system-ui, -apple-system, sans-serif; | |
| --font-mono: "Geist Mono", ui-monospace, monospace; | |
| --font-serif: "Instrument Serif", georgia, serif; | |
| } |
🧰 Tools
🪛 Biome (2.4.7)
[error] 182-188: Tailwind-specific syntax is disabled.
(parse)
🪛 Stylelint (17.4.0)
[error] 187-187: Expected "Georgia" to be "georgia" (value-keyword-case)
(value-keyword-case)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/app.css` around lines 185 - 188, The CSS uses a quoted capitalized
generic font family "Georgia" in the --font-serif custom property which violates
Stylelint's value-keyword-case; update the --font-serif declaration (the
--font-serif variable) to use the lowercase generic family georgia without
quotes so it reads --font-serif: "Instrument Serif", georgia, serif; ensuring
the generic keyword is lowercase and unquoted.
Comment on lines
+30
to
+35
| { rel: "preconnect", href: "https://fonts.googleapis.com" }, | ||
| { rel: "preconnect", href: "https://fonts.gstatic.com", crossOrigin: "anonymous" }, | ||
| { | ||
| rel: "stylesheet", | ||
| href: "https://fonts.googleapis.com/css2?family=Instrument+Serif:ital@0;1&display=block", | ||
| }, |
There was a problem hiding this comment.
Add crossOrigin to the googleapis.com preconnect for consistency.
The fonts.gstatic.com preconnect has crossOrigin: "anonymous" but fonts.googleapis.com does not. For proper CORS credentialing and connection reuse, both preconnects should have it.
Also consider using display=swap instead of display=block to avoid Flash of Invisible Text (FOIT) while fonts load—unless the block behavior is intentional for design consistency.
🔧 Proposed fix for crossOrigin
- { rel: "preconnect", href: "https://fonts.googleapis.com" },
+ { rel: "preconnect", href: "https://fonts.googleapis.com", crossOrigin: "anonymous" },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| { rel: "preconnect", href: "https://fonts.googleapis.com" }, | |
| { rel: "preconnect", href: "https://fonts.gstatic.com", crossOrigin: "anonymous" }, | |
| { | |
| rel: "stylesheet", | |
| href: "https://fonts.googleapis.com/css2?family=Instrument+Serif:ital@0;1&display=block", | |
| }, | |
| { rel: "preconnect", href: "https://fonts.googleapis.com", crossOrigin: "anonymous" }, | |
| { rel: "preconnect", href: "https://fonts.gstatic.com", crossOrigin: "anonymous" }, | |
| { | |
| rel: "stylesheet", | |
| href: "https://fonts.googleapis.com/css2?family=Instrument+Serif:ital@0;1&display=block", | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/routes/__root.tsx` around lines 30 - 35, The preconnect for
"https://fonts.googleapis.com" is missing crossOrigin like the
"https://fonts.gstatic.com" entry; update the preconnect link object for
fonts.googleapis.com to include crossOrigin: "anonymous" for consistency, and
(unless block behavior is intentional) change the stylesheet href query from
display=block to display=swap to avoid FOIT—locate the link objects with rel:
"preconnect" and the stylesheet href containing
"fonts.googleapis.com/css2?family=Instrument+Serif" in the array and apply these
two changes.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/setup.md`:
- Around line 5-6: Update the docs to clarify the prerequisite step by adding an
explicit instruction to run pnpm install before referencing the vp CLI;
specifically mention running "pnpm install" so that vite+ (and the vp command)
is installed and available prior to the subsequent "vp install" instruction, and
update the sentence referencing "Install vite+ first" to include this pnpm
install step for clarity.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Comment on lines
+5
to
+6
| Install vite+ first. Then... | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether prerequisite installation steps for `vp` are documented elsewhere.
rg -n -C2 'vite\+|vite-plus|vp install|install.*vp' docs README.mdRepository: pingdotgg/lawn
Length of output: 315
🏁 Script executed:
cat -n docs/setup.mdRepository: pingdotgg/lawn
Length of output: 1244
🏁 Script executed:
rg -n 'vp' docs/setup.md | head -20Repository: pingdotgg/lawn
Length of output: 170
🌐 Web query:
vite+ vp package manager install
💡 Result:
Vite (app scaffold) — install via your package manager
Use the Vite “create” CLI (it installs/runs without a separate global install):
- npm
npm create vite@latest
- pnpm
pnpm create vite@latest
- yarn
yarn create vite
- bun
(vite.dev)
bun create vite
VitePress (docs site) — install + init wizard
Install VitePress, then run the init wizard:
- Install
npm add -D vitepress@next # or: pnpm add -D vitepress@next # or: yarn add -D vitepress@next vue # or: bun add -D vitepress@next
- Initialize
(vitepress.dev)
npx vitepress init # or: pnpm vitepress init # or: yarn vitepress init # or: bun vitepress init
Citations:
- 1: https://vite.dev/guide/?utm_source=openai
- 2: https://vitepress.dev/guide/getting-started?utm_source=openai
🏁 Script executed:
fd -type f -name "package.json" -o -name "README*" | head -10Repository: pingdotgg/lawn
Length of output: 228
🏁 Script executed:
rg -n 'vp.*install|npm install.*vp|install vite\+' --type-listRepository: pingdotgg/lawn
Length of output: 447
🏁 Script executed:
# Search for vp bin or setup references
rg -n 'bin.*vp|vp.*bin|install.*command' | head -20Repository: pingdotgg/lawn
Length of output: 40
🏁 Script executed:
# Check if package.json has vp as a script
cat package.json 2>/dev/null | head -50Repository: pingdotgg/lawn
Length of output: 1882
🏁 Script executed:
# Check devDependencies and other dep sections in package.json
tail -100 package.json | grep -A 50 "devDependencies"Repository: pingdotgg/lawn
Length of output: 1217
🏁 Script executed:
# Search for "vp" in package.json to see if it's listed anywhere
rg -n '"vp"' package.jsonRepository: pingdotgg/lawn
Length of output: 40
🏁 Script executed:
# Check README for installation or setup instructions
cat README.md 2>/dev/null | head -100Repository: pingdotgg/lawn
Length of output: 871
🏁 Script executed:
# Search for any docs mentioning vp or vite+
fd -name "*.md" | xargs rg -l "vp|vite\+" | head -10Repository: pingdotgg/lawn
Length of output: 374
Clarify the initial setup step before using vp.
"Install vite+ first" is missing the crucial prerequisite. The documentation should explicitly instruct users to run pnpm install first, which installs vite-plus and makes the vp CLI available. Without this, the next line's vp install command will fail.
Add an explicit instruction:
📝 Suggested doc patch
Install vite+ first. Then...
+```bash
+pnpm install
+```
+
Install dependencies:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/setup.md` around lines 5 - 6, Update the docs to clarify the
prerequisite step by adding an explicit instruction to run pnpm install before
referencing the vp CLI; specifically mention running "pnpm install" so that
vite+ (and the vp command) is installed and available prior to the subsequent
"vp install" instruction, and update the sentence referencing "Install vite+
first" to include this pnpm install step for clarity.
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (4)
app/routes/-watch.tsx (2)
288-293: Consider deduplicating the submit button/form chunk.Both desktop and mobile composer sections use near-identical submit button props/content; extracting shared JSX would simplify future updates.
Also applies to: 406-411
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/routes/-watch.tsx` around lines 288 - 293, Extract the repeated submit button JSX into a small shared component (e.g., CommentSubmitButton or ComposerSubmitButton) that accepts props used by both instances: commentText, isSubmittingComment, onSubmit (or type/size/className if needed), and any children/label; replace the two near-identical inline Button usages with this new component and forward the disabled logic (disabled={!commentText.trim() || isSubmittingComment}) and other props (type="submit", size="sm", className="w-full") to keep behavior identical; update the form/compose areas to import/use the new component so future changes require editing only one place.
240-245: Consider extracting shared comment-content rendering.The desktop and mobile blocks now repeat the same comment text/time rendering. A small shared component/helper would reduce drift risk.
Also applies to: 357-362
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/routes/-watch.tsx` around lines 240 - 245, Extract the duplicated markup into a small React component (e.g., CommentContent or CommentText) that accepts the comment (or props text and _creationTime) and returns the two <p> elements using comment.text and formatRelativeTime(comment._creationTime) with the same classNames; then replace the repeated blocks in both desktop and mobile render paths with a single <CommentContent comment={comment} /> (or <CommentText text={comment.text} time={comment._creationTime} />) to remove duplication and keep rendering consistent.src/components/ui/card.tsx (1)
25-33: Pre-existing type mismatch inCardTitle.The ref type is
HTMLParagraphElementbut the component renders an<h3>element, and the props extendReact.HTMLAttributes<HTMLHeadingElement>. This inconsistency predates this PR but may cause type errors when passing a ref. Consider aligning types:🔧 Suggested fix
-const CardTitle = React.forwardRef<HTMLParagraphElement, React.HTMLAttributes<HTMLHeadingElement>>( +const CardTitle = React.forwardRef<HTMLHeadingElement, React.HTMLAttributes<HTMLHeadingElement>>(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/card.tsx` around lines 25 - 33, CardTitle's forwardRef type parameters are inconsistent: the ref is declared as HTMLParagraphElement while the component renders an <h3> and uses React.HTMLAttributes<HTMLHeadingElement>, which can cause type errors when consumers pass a ref. Fix by aligning the generic types for React.forwardRef so the ref type and props match the rendered element (e.g., use HTMLHeadingElement for the ref and keep React.HTMLAttributes<HTMLHeadingElement> for props, or switch to React.ComponentPropsWithRef<'h3'>); update the CardTitle forwardRef signature to use the chosen matching types so ref and props are consistent with the <h3> element.src/components/comments/CommentInput.tsx (1)
107-112: Remove redundant ternary in action-row className.Both branches return the same string, so the conditional can be simplified.
Suggested cleanup
- <div - className={ - variant === "seamless" - ? "absolute bottom-3 right-3 flex items-center gap-2" - : "absolute bottom-3 right-3 flex items-center gap-2" - } - > + <div className="absolute bottom-3 right-3 flex items-center gap-2">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/comments/CommentInput.tsx` around lines 107 - 112, In CommentInput.tsx remove the redundant ternary used for the action-row div className: replace the conditional expression that checks variant === "seamless" and returns the same string in both branches with a single constant string ("absolute bottom-3 right-3 flex items-center gap-2"); update the JSX in the CommentInput component where the div with className currently uses the ternary so it directly uses the static className value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/routes/dashboard/-project.tsx`:
- Around line 336-337: The Download action is shown for videos in "processing"
even though convex/videoActions.ts:getDownloadUrl only allows downloads when
video.status === "ready"; update the canDownload predicate used in both the grid
and list branches to require video.status === "ready" (e.g. canDownload =
Boolean(video.s3Key) && video.status === "ready") so the UI hides Download until
the video is actually ready.
- Around line 381-389: The menu trigger button is hidden via "opacity-0
group-hover:opacity-100" which prevents touch/keyboard users from
seeing/focusing it and the icon-only button lacks an accessible name; update the
DropdownMenu trigger usage (the DropdownMenuTrigger and its child button using
the MoreVertical icon) to be visible by removing the opacity-0/group-hover
classes (or make it always have sufficient opacity/visibility) and add an
accessible label to the icon-only button (e.g., aria-label="Open actions" or
include a screen-reader-only text) so screen readers and keyboard users can
identify it; apply the same visibility and labeling changes to the list-view
trigger instance referenced later (the other DropdownMenuTrigger/button block).
In `@app/routes/dashboard/-video.tsx`:
- Around line 267-303: The inline title editor lacks accessible names: add
descriptive accessible labels to the Input and the three icon Buttons so screen
readers announce their purpose; specifically, give the Input (used with
editedTitle) an aria-label like "Edit video title" or aria-labelledby pointing
to a visible label, and add aria-labels to the save Button (handleSaveTitle)
e.g. "Save title", the cancel Button (setIsEditingTitle(false)) e.g. "Cancel
rename", and the edit Button (startEditingTitle) e.g. "Rename video"; ensure
these attributes are applied to the Input and Button components used in the
isEditingTitle conditional so assistive tech can identify each control.
In `@convex/billing.ts`:
- Around line 193-197: subscriptionStatus can come from subscription?.status or
fallback to subscriptionState.team.billingStatus, but hasActiveSubscription is
currently derived only from subscription and can contradict subscriptionStatus;
update the logic so hasActiveSubscription uses the resolved subscriptionStatus
(or the same fallback order) instead of subscription-only data. Specifically,
compute subscriptionStatus using subscription?.status ??
subscriptionState.team.billingStatus ?? null and then set hasActiveSubscription
by evaluating that resolved status (e.g., === "active") or the same fallback
expression, ensuring consistency between subscriptionStatus and
hasActiveSubscription.
In `@src/components/comments/CommentInput.tsx`:
- Around line 77-80: The Enter-key handler that calls submitComment currently
doesn't prevent submission during IME composition; update the keydown handler
(the block checking e.key === "Enter" && !e.shiftKey && !e.metaKey && !e.ctrlKey
&& !e.altKey) to also check for IME composition by verifying
!e.nativeEvent.isComposing before calling e.preventDefault() and
submitComment(), and for broader compatibility add a fallback that treats
nativeEvent.keyCode === 229 as composing (or alternatively implement
onCompositionStart/onCompositionEnd to track a composing flag and check that
flag here); ensure you reference and update the same handler around the
submitComment invocation so composition prevents accidental submits.
In `@src/components/video-player/VideoPlayer.tsx`:
- Around line 829-832: In VideoPlayer.tsx the volume slider only calls
e.stopPropagation() inside the onChange handler (where setVideoVolume is
called), allowing parent pointer/click handlers to still fire; add onPointerDown
and onClick handlers on the same slider element that call e.stopPropagation()
(and prevent default if needed) to fully isolate interactions with the control
so parent listeners won't receive pointer or click events when the user
manipulates the slider.
---
Nitpick comments:
In `@app/routes/-watch.tsx`:
- Around line 288-293: Extract the repeated submit button JSX into a small
shared component (e.g., CommentSubmitButton or ComposerSubmitButton) that
accepts props used by both instances: commentText, isSubmittingComment, onSubmit
(or type/size/className if needed), and any children/label; replace the two
near-identical inline Button usages with this new component and forward the
disabled logic (disabled={!commentText.trim() || isSubmittingComment}) and other
props (type="submit", size="sm", className="w-full") to keep behavior identical;
update the form/compose areas to import/use the new component so future changes
require editing only one place.
- Around line 240-245: Extract the duplicated markup into a small React
component (e.g., CommentContent or CommentText) that accepts the comment (or
props text and _creationTime) and returns the two <p> elements using
comment.text and formatRelativeTime(comment._creationTime) with the same
classNames; then replace the repeated blocks in both desktop and mobile render
paths with a single <CommentContent comment={comment} /> (or <CommentText
text={comment.text} time={comment._creationTime} />) to remove duplication and
keep rendering consistent.
In `@src/components/comments/CommentInput.tsx`:
- Around line 107-112: In CommentInput.tsx remove the redundant ternary used for
the action-row div className: replace the conditional expression that checks
variant === "seamless" and returns the same string in both branches with a
single constant string ("absolute bottom-3 right-3 flex items-center gap-2");
update the JSX in the CommentInput component where the div with className
currently uses the ternary so it directly uses the static className value.
In `@src/components/ui/card.tsx`:
- Around line 25-33: CardTitle's forwardRef type parameters are inconsistent:
the ref is declared as HTMLParagraphElement while the component renders an <h3>
and uses React.HTMLAttributes<HTMLHeadingElement>, which can cause type errors
when consumers pass a ref. Fix by aligning the generic types for
React.forwardRef so the ref type and props match the rendered element (e.g., use
HTMLHeadingElement for the ref and keep React.HTMLAttributes<HTMLHeadingElement>
for props, or switch to React.ComponentPropsWithRef<'h3'>); update the CardTitle
forwardRef signature to use the chosen matching types so ref and props are
consistent with the <h3> element.
🪄 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: 9ed95175-281d-4b98-b396-5f812150c53a
⛔ Files ignored due to path filters (1)
convex/_generated/dataModel.d.tsis excluded by!**/_generated/**
📒 Files selected for processing (94)
.node-version.prettierignoreCLAUDE.mdapp/routes/-compare-frameio.tsxapp/routes/-compare-wipster.tsxapp/routes/-for-agencies.tsxapp/routes/-for-video-editors.tsxapp/routes/-home.tsxapp/routes/-invite.data.tsapp/routes/-invite.tsxapp/routes/-pricing.tsxapp/routes/-share.data.tsapp/routes/-share.tsxapp/routes/-watch.data.tsapp/routes/-watch.tsxapp/routes/auth/-layout.tsxapp/routes/dashboard/$teamSlug.$projectId.index.tsxapp/routes/dashboard/-index.data.tsapp/routes/dashboard/-layout.tsxapp/routes/dashboard/-project.data.tsapp/routes/dashboard/-project.tsxapp/routes/dashboard/-routeDataContracts.test.tsapp/routes/dashboard/-settings.data.tsapp/routes/dashboard/-settings.tsxapp/routes/dashboard/-team.data.tsapp/routes/dashboard/-team.tsxapp/routes/dashboard/-useVideoUploadManager.tsapp/routes/dashboard/-video.data.tsapp/routes/dashboard/-video.tsxapp/routes/dashboard/index.tsxapp/routes/mono.tsxapp/routes/sign-in.tsxapp/routes/sign-up.tsxconvex/README.mdconvex/auth.tsconvex/billing.tsconvex/billingHelpers.tsconvex/comments.tsconvex/mux.tsconvex/muxActions.tsconvex/projects.tsconvex/s3.tsconvex/schema.tsconvex/security.tsconvex/shareAccess.tsconvex/shareLinks.tsconvex/teams.tsconvex/videoActions.tsconvex/videoPresence.tsconvex/videos.tsconvex/workspace.tsdocs/philosophy.mdplans/fix-navigation.mdplans/remove-user-table.mdscripts/generate-og.tsxsrc/components/DashboardHeader.tsxsrc/components/MarketingFooter.tsxsrc/components/MarketingNav.tsxsrc/components/ShareDialog.tsxsrc/components/comments/CommentInput.tsxsrc/components/comments/CommentItem.tsxsrc/components/comments/CommentList.tsxsrc/components/presence/VideoWatchers.tsxsrc/components/teams/CreateTeamDialog.tsxsrc/components/teams/MemberInvite.tsxsrc/components/theme/ThemeToggle.tsxsrc/components/ui/NotFound.tsxsrc/components/ui/avatar.tsxsrc/components/ui/badge.tsxsrc/components/ui/button.tsxsrc/components/ui/card.tsxsrc/components/ui/dialog.tsxsrc/components/ui/dropdown-menu.tsxsrc/components/ui/input.tsxsrc/components/ui/progress.tsxsrc/components/ui/scroll-area.tsxsrc/components/ui/separator.tsxsrc/components/ui/tabs.tsxsrc/components/ui/textarea.tsxsrc/components/ui/tooltip.tsxsrc/components/upload/DropZone.tsxsrc/components/upload/UploadButton.tsxsrc/components/upload/UploadProgress.tsxsrc/components/video-player/VideoPlayer.tsxsrc/components/videos/VideoWorkflowStatusControl.tsxsrc/lib/convexRouteData.test.tssrc/lib/convexRouteData.tssrc/lib/dashboardUploadContext.tsxsrc/lib/download.tssrc/lib/seo.tssrc/lib/useRoutePrewarmIntent.tssrc/lib/useVideoPresence.tsvercel.jsonvite.config.ts
💤 Files with no reviewable changes (3)
- src/lib/download.ts
- plans/fix-navigation.md
- plans/remove-user-table.md
✅ Files skipped from review due to trivial changes (52)
- src/components/ui/progress.tsx
- app/routes/dashboard/-useVideoUploadManager.ts
- app/routes/-compare-wipster.tsx
- app/routes/dashboard/-index.data.ts
- src/components/ui/tabs.tsx
- src/lib/convexRouteData.ts
- src/components/ui/badge.tsx
- src/lib/useVideoPresence.ts
- src/lib/seo.ts
- src/components/teams/CreateTeamDialog.tsx
- convex/README.md
- src/lib/dashboardUploadContext.tsx
- src/components/theme/ThemeToggle.tsx
- convex/shareLinks.ts
- app/routes/sign-in.tsx
- convex/projects.ts
- .prettierignore
- app/routes/-invite.data.ts
- app/routes/-pricing.tsx
- app/routes/-share.tsx
- app/routes/-compare-frameio.tsx
- src/components/presence/VideoWatchers.tsx
- convex/muxActions.ts
- src/components/comments/CommentItem.tsx
- src/components/MarketingNav.tsx
- src/components/ui/tooltip.tsx
- convex/s3.ts
- src/components/DashboardHeader.tsx
- app/routes/dashboard/$teamSlug.$projectId.index.tsx
- app/routes/dashboard/index.tsx
- src/components/upload/UploadButton.tsx
- app/routes/dashboard/-project.data.ts
- src/components/teams/MemberInvite.tsx
- src/components/ShareDialog.tsx
- src/components/ui/input.tsx
- .node-version
- src/components/ui/avatar.tsx
- src/components/MarketingFooter.tsx
- app/routes/dashboard/-video.data.ts
- app/routes/dashboard/-team.data.ts
- app/routes/dashboard/-settings.tsx
- scripts/generate-og.tsx
- convex/workspace.ts
- src/components/ui/NotFound.tsx
- app/routes/-for-agencies.tsx
- app/routes/-home.tsx
- src/components/videos/VideoWorkflowStatusControl.tsx
- convex/auth.ts
- convex/teams.ts
- src/components/ui/button.tsx
- src/components/upload/DropZone.tsx
- app/routes/-invite.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- vercel.json
Comment on lines
+336
to
+337
| const canDownload = | ||
| Boolean(video.s3Key) && video.status !== "failed" && video.status !== "uploading"; |
There was a problem hiding this comment.
Keep the Download action hidden until the video is ready.
Lines 336-337 and Lines 475-476 still treat processing videos as downloadable, but convex/videoActions.ts Lines 480-482 reject getDownloadUrl unless video.status === "ready". Right now the menu can surface a Download action that always fails for videos still encoding.
💡 Suggested fix
- const canDownload =
- Boolean(video.s3Key) && video.status !== "failed" && video.status !== "uploading";
+ const canDownload = Boolean(video.s3Key) && video.status === "ready";Apply the same predicate in both the grid and list branches.
Also applies to: 475-476
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/routes/dashboard/-project.tsx` around lines 336 - 337, The Download
action is shown for videos in "processing" even though
convex/videoActions.ts:getDownloadUrl only allows downloads when video.status
=== "ready"; update the canDownload predicate used in both the grid and list
branches to require video.status === "ready" (e.g. canDownload =
Boolean(video.s3Key) && video.status === "ready") so the UI hides Download until
the video is actually ready.
Comment on lines
+381
to
+389
| <div className="absolute top-2 right-2 opacity-0 group-hover:opacity-100 transition-opacity"> | ||
| <DropdownMenu> | ||
| <DropdownMenuTrigger asChild onClick={(e) => e.stopPropagation()}> | ||
| <button | ||
| type="button" | ||
| className="inline-flex h-8 w-8 cursor-pointer items-center justify-center bg-black/60 hover:bg-black/80 text-white" | ||
| > | ||
| <Download className="mr-2 h-4 w-4" /> | ||
| Download | ||
| </DropdownMenuItem> | ||
| )} | ||
| <DropdownMenuItem | ||
| onClick={(e) => { | ||
| e.stopPropagation(); | ||
| void handleShareVideo(video); | ||
| }} | ||
| > | ||
| <LinkIcon className="mr-2 h-4 w-4" /> | ||
| Share | ||
| </DropdownMenuItem> | ||
| {canUpload && ( | ||
| <MoreVertical className="h-4 w-4" /> | ||
| </button> |
There was a problem hiding this comment.
Expose the action menu trigger without hover.
Lines 381-389 and Lines 556-564 hide the only menu trigger behind group-hover, so touch users never see it and keyboard users can tab onto an invisible button. The trigger also needs an accessible name because it's icon-only.
💡 Suggested fix
- <div className="absolute top-2 right-2 opacity-0 group-hover:opacity-100 transition-opacity">
+ <div className="absolute top-2 right-2 opacity-100 sm:opacity-0 sm:group-hover:opacity-100 sm:group-focus-within:opacity-100 transition-opacity">
...
- <button
- type="button"
- className="inline-flex h-8 w-8 cursor-pointer items-center justify-center bg-black/60 hover:bg-black/80 text-white"
- >
+ <button
+ type="button"
+ aria-label={`Open actions for ${video.title}`}
+ className="inline-flex h-8 w-8 cursor-pointer items-center justify-center bg-black/60 hover:bg-black/80 text-white"
+ >Apply the same visibility and labeling change to the list-view trigger as well.
Also applies to: 556-564
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/routes/dashboard/-project.tsx` around lines 381 - 389, The menu trigger
button is hidden via "opacity-0 group-hover:opacity-100" which prevents
touch/keyboard users from seeing/focusing it and the icon-only button lacks an
accessible name; update the DropdownMenu trigger usage (the DropdownMenuTrigger
and its child button using the MoreVertical icon) to be visible by removing the
opacity-0/group-hover classes (or make it always have sufficient
opacity/visibility) and add an accessible label to the icon-only button (e.g.,
aria-label="Open actions" or include a screen-reader-only text) so screen
readers and keyboard users can identify it; apply the same visibility and
labeling changes to the list-view trigger instance referenced later (the other
DropdownMenuTrigger/button block).
Comment on lines
+267
to
+303
| label: isEditingTitle ? ( | ||
| <div className="flex items-center gap-2"> | ||
| <Input | ||
| value={editedTitle} | ||
| onChange={(e) => setEditedTitle(e.target.value)} | ||
| className="w-40 sm:w-64 h-8 text-base font-black tracking-tighter uppercase font-mono" | ||
| autoFocus | ||
| onKeyDown={(e) => { | ||
| if (e.key === "Enter") handleSaveTitle(); | ||
| if (e.key === "Escape") setIsEditingTitle(false); | ||
| }} | ||
| /> | ||
| <Button size="icon" variant="ghost" className="h-8 w-8" onClick={handleSaveTitle}> | ||
| <Check className="h-4 w-4" /> | ||
| </Button> | ||
| <Button | ||
| size="icon" | ||
| variant="ghost" | ||
| className="h-6 w-6" | ||
| onClick={startEditingTitle} | ||
| className="h-8 w-8" | ||
| onClick={() => setIsEditingTitle(false)} | ||
| > | ||
| <Edit2 className="h-3 w-3" /> | ||
| <X className="h-4 w-4" /> | ||
| </Button> | ||
| )} | ||
| {video.status !== "ready" && ( | ||
| <Badge | ||
| variant={video.status === "failed" ? "destructive" : "secondary"} | ||
| > | ||
| {video.status === "uploading" && "Uploading"} | ||
| {video.status === "processing" && "Processing"} | ||
| {video.status === "failed" && "Failed"} | ||
| </Badge> | ||
| )} | ||
| </div> | ||
| ) | ||
| } | ||
| ]}> | ||
| </div> | ||
| ) : ( | ||
| <div className="flex items-center gap-2"> | ||
| <span className="truncate max-w-[150px] sm:max-w-[300px]">{video.title}</span> | ||
| {canEdit && ( | ||
| <Button | ||
| size="icon" | ||
| variant="ghost" | ||
| className="h-6 w-6" | ||
| onClick={startEditingTitle} | ||
| > | ||
| <Edit2 className="h-3 w-3" /> | ||
| </Button> | ||
| )} |
There was a problem hiding this comment.
Add accessible names to the inline title editor controls.
Lines 267-303 introduce a new rename flow, but the input and the save/cancel/edit icon buttons are all unnamed. Screen readers will announce anonymous controls here, which makes the title-edit path much harder to use.
💡 Suggested fix
<Input
+ aria-label="Video title"
value={editedTitle}
onChange={(e) => setEditedTitle(e.target.value)}
className="w-40 sm:w-64 h-8 text-base font-black tracking-tighter uppercase font-mono"
autoFocus
onKeyDown={(e) => {
if (e.key === "Enter") handleSaveTitle();
if (e.key === "Escape") setIsEditingTitle(false);
}}
/>
- <Button size="icon" variant="ghost" className="h-8 w-8" onClick={handleSaveTitle}>
+ <Button
+ type="button"
+ size="icon"
+ variant="ghost"
+ className="h-8 w-8"
+ aria-label="Save title"
+ onClick={handleSaveTitle}
+ >
<Check className="h-4 w-4" />
</Button>
<Button
+ type="button"
size="icon"
variant="ghost"
className="h-8 w-8"
+ aria-label="Cancel title editing"
onClick={() => setIsEditingTitle(false)}
>
<X className="h-4 w-4" />
</Button>
...
{canEdit && (
<Button
+ type="button"
size="icon"
variant="ghost"
className="h-6 w-6"
+ aria-label="Edit video title"
onClick={startEditingTitle}
>
<Edit2 className="h-3 w-3" />
</Button>
)}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| label: isEditingTitle ? ( | |
| <div className="flex items-center gap-2"> | |
| <Input | |
| value={editedTitle} | |
| onChange={(e) => setEditedTitle(e.target.value)} | |
| className="w-40 sm:w-64 h-8 text-base font-black tracking-tighter uppercase font-mono" | |
| autoFocus | |
| onKeyDown={(e) => { | |
| if (e.key === "Enter") handleSaveTitle(); | |
| if (e.key === "Escape") setIsEditingTitle(false); | |
| }} | |
| /> | |
| <Button size="icon" variant="ghost" className="h-8 w-8" onClick={handleSaveTitle}> | |
| <Check className="h-4 w-4" /> | |
| </Button> | |
| <Button | |
| size="icon" | |
| variant="ghost" | |
| className="h-6 w-6" | |
| onClick={startEditingTitle} | |
| className="h-8 w-8" | |
| onClick={() => setIsEditingTitle(false)} | |
| > | |
| <Edit2 className="h-3 w-3" /> | |
| <X className="h-4 w-4" /> | |
| </Button> | |
| )} | |
| {video.status !== "ready" && ( | |
| <Badge | |
| variant={video.status === "failed" ? "destructive" : "secondary"} | |
| > | |
| {video.status === "uploading" && "Uploading"} | |
| {video.status === "processing" && "Processing"} | |
| {video.status === "failed" && "Failed"} | |
| </Badge> | |
| )} | |
| </div> | |
| ) | |
| } | |
| ]}> | |
| </div> | |
| ) : ( | |
| <div className="flex items-center gap-2"> | |
| <span className="truncate max-w-[150px] sm:max-w-[300px]">{video.title}</span> | |
| {canEdit && ( | |
| <Button | |
| size="icon" | |
| variant="ghost" | |
| className="h-6 w-6" | |
| onClick={startEditingTitle} | |
| > | |
| <Edit2 className="h-3 w-3" /> | |
| </Button> | |
| )} | |
| label: isEditingTitle ? ( | |
| <div className="flex items-center gap-2"> | |
| <Input | |
| aria-label="Video title" | |
| value={editedTitle} | |
| onChange={(e) => setEditedTitle(e.target.value)} | |
| className="w-40 sm:w-64 h-8 text-base font-black tracking-tighter uppercase font-mono" | |
| autoFocus | |
| onKeyDown={(e) => { | |
| if (e.key === "Enter") handleSaveTitle(); | |
| if (e.key === "Escape") setIsEditingTitle(false); | |
| }} | |
| /> | |
| <Button | |
| type="button" | |
| size="icon" | |
| variant="ghost" | |
| className="h-8 w-8" | |
| aria-label="Save title" | |
| onClick={handleSaveTitle} | |
| > | |
| <Check className="h-4 w-4" /> | |
| </Button> | |
| <Button | |
| type="button" | |
| size="icon" | |
| variant="ghost" | |
| className="h-8 w-8" | |
| aria-label="Cancel title editing" | |
| onClick={() => setIsEditingTitle(false)} | |
| > | |
| <X className="h-4 w-4" /> | |
| </Button> | |
| </div> | |
| ) : ( | |
| <div className="flex items-center gap-2"> | |
| <span className="truncate max-w-[150px] sm:max-w-[300px]">{video.title}</span> | |
| {canEdit && ( | |
| <Button | |
| type="button" | |
| size="icon" | |
| variant="ghost" | |
| className="h-6 w-6" | |
| aria-label="Edit video title" | |
| onClick={startEditingTitle} | |
| > | |
| <Edit2 className="h-3 w-3" /> | |
| </Button> | |
| )} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/routes/dashboard/-video.tsx` around lines 267 - 303, The inline title
editor lacks accessible names: add descriptive accessible labels to the Input
and the three icon Buttons so screen readers announce their purpose;
specifically, give the Input (used with editedTitle) an aria-label like "Edit
video title" or aria-labelledby pointing to a visible label, and add aria-labels
to the save Button (handleSaveTitle) e.g. "Save title", the cancel Button
(setIsEditingTitle(false)) e.g. "Cancel rename", and the edit Button
(startEditingTitle) e.g. "Rename video"; ensure these attributes are applied to
the Input and Button components used in the isEditingTitle conditional so
assistive tech can identify each control.
Comment on lines
+193
to
+197
| subscriptionStatus: subscription?.status ?? subscriptionState.team.billingStatus ?? null, | ||
| stripeCustomerId: | ||
| subscriptionState.team.stripeCustomerId ?? | ||
| subscription?.stripeCustomerId ?? | ||
| null, | ||
| subscriptionState.team.stripeCustomerId ?? subscription?.stripeCustomerId ?? null, | ||
| stripeSubscriptionId: | ||
| subscription?.stripeSubscriptionId ?? | ||
| subscriptionState.team.stripeSubscriptionId ?? | ||
| null, | ||
| subscription?.stripeSubscriptionId ?? subscriptionState.team.stripeSubscriptionId ?? null, |
There was a problem hiding this comment.
Keep hasActiveSubscription consistent with fallback status.
With the new fallback on Line 193 and Lines 195–197, subscriptionStatus can come from team state while hasActiveSubscription still comes from subscription-only state. This can return contradictory values (e.g., status "active" but hasActiveSubscription: false).
Suggested fix
handler: async (ctx, args) => {
const { membership } = await requireTeamAccess(ctx, args.teamId);
const subscriptionState = await getTeamSubscriptionState(ctx, args.teamId);
const storageUsedBytes = await getTeamStorageUsedBytes(ctx, args.teamId);
const subscription = subscriptionState.subscription;
+ const effectiveStatus = subscription?.status ?? subscriptionState.team.billingStatus ?? null;
+ const hasActiveSubscription = hasActiveTeamSubscriptionStatus(effectiveStatus);
return {
plan: subscriptionState.plan,
monthlyPriceUsd: TEAM_PLAN_MONTHLY_PRICE_USD[subscriptionState.plan],
storageLimitBytes: TEAM_PLAN_STORAGE_LIMIT_BYTES[subscriptionState.plan],
storageUsedBytes,
- hasActiveSubscription: subscriptionState.hasActiveSubscription,
- subscriptionStatus: subscription?.status ?? subscriptionState.team.billingStatus ?? null,
+ hasActiveSubscription,
+ subscriptionStatus: effectiveStatus,
stripeCustomerId:
subscriptionState.team.stripeCustomerId ?? subscription?.stripeCustomerId ?? null,
stripeSubscriptionId:
subscription?.stripeSubscriptionId ?? subscriptionState.team.stripeSubscriptionId ?? null,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@convex/billing.ts` around lines 193 - 197, subscriptionStatus can come from
subscription?.status or fallback to subscriptionState.team.billingStatus, but
hasActiveSubscription is currently derived only from subscription and can
contradict subscriptionStatus; update the logic so hasActiveSubscription uses
the resolved subscriptionStatus (or the same fallback order) instead of
subscription-only data. Specifically, compute subscriptionStatus using
subscription?.status ?? subscriptionState.team.billingStatus ?? null and then
set hasActiveSubscription by evaluating that resolved status (e.g., ===
"active") or the same fallback expression, ensuring consistency between
subscriptionStatus and hasActiveSubscription.
Comment on lines
+77
to
80
| if (e.key === "Enter" && !e.shiftKey && !e.metaKey && !e.ctrlKey && !e.altKey) { | ||
| e.preventDefault(); | ||
| void submitComment(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/components/comments/CommentInput.tsx | head -100Repository: pingdotgg/lawn
Length of output: 3391
🏁 Script executed:
rg "isComposing" --type tsx --type tsRepository: pingdotgg/lawn
Length of output: 85
🏁 Script executed:
rg "handleKeyDown\|KeyboardEvent" src/components/comments/CommentInput.tsx -A 10 -B 5Repository: pingdotgg/lawn
Length of output: 40
🏁 Script executed:
rg "composing\|composition\|IME" --ignore-caseRepository: pingdotgg/lawn
Length of output: 40
🌐 Web query:
IME composition handling React keydown isComposing best practices
💡 Result:
Best practices for IME-safe onKeyDown in React
-
Don’t treat
Enter(or arrow keys/Esc) as “final” while the user is composing text. During an IME composition session, key events can fire even though the text isn’t finalized yet. UseKeyboardEvent.isComposingto detect this window (betweencompositionstartandcompositionend). [1] -
In React, check
e.nativeEvent.isComposinginonKeyDown. React wraps the browserKeyboardEvent, so the most reliable flag is usually on the native event. [2] -
Safari workaround: Safari can report
isComposing === falseon thekeydownforEntereven though composition is still effectively happening; a common workaround is to also ignorekeyCode === 229(“composition in progress” / “process key”). [2] -
For maximum robustness, track composition state explicitly with
onCompositionStart/onCompositionEndand consult that state inonKeyDown(rather than relying only onisComposing). This also aligns with the platform model of composition sessions. [1][3]
React pattern (practical and cross-browser)
function ChatInput({ onSubmit }: { onSubmit: (text: string) => void }) {
const composingRef = React.useRef(false);
const inputRef = React.useRef<HTMLInputElement>(null);
return (
<input
ref={inputRef}
onCompositionStart={() => { composingRef.current = true; }}
onCompositionEnd={() => { composingRef.current = false; }}
onKeyDown={(e) => {
const ne = e.nativeEvent as KeyboardEvent;
// IME guard (incl. Safari fallback)
if (composingRef.current || ne.isComposing || ne.keyCode === 229) return;
if (e.key === "Enter") onSubmit(inputRef.current?.value ?? "");
}}
/>
);
}Rule of thumb: use onInput/onChange to react to text changes, and reserve onKeyDown for shortcuts/submit—but always gate those shortcuts with IME composition checks. [1][2]
Sources: MDN on KeyboardEvent.isComposing [1]; discussion + Safari/229 workaround example [2]; UI Events spec for composition session semantics [3].
🏁 Script executed:
rg "onComposition"Repository: pingdotgg/lawn
Length of output: 40
🏁 Script executed:
rg "textarea\|TextArea" src/ -lRepository: pingdotgg/lawn
Length of output: 40
Add IME composition check to prevent submit during text composition.
Line 77 can submit while users are composing text with IME (e.g., Japanese/Chinese/Korean), interrupting input mid-composition.
Add e.nativeEvent.isComposing check before processing Enter:
Suggested fix
const handleKeyDown = (e: React.KeyboardEvent) => {
+ if (e.nativeEvent.isComposing) return;
if (e.key === "Enter" && !e.shiftKey && !e.metaKey && !e.ctrlKey && !e.altKey) {
e.preventDefault();
void submitComment();
}Note: For maximum cross-browser robustness (especially Safari), consider also checking e.nativeEvent.keyCode === 229 as a fallback, or explicitly track composition state with onCompositionStart/onCompositionEnd.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (e.key === "Enter" && !e.shiftKey && !e.metaKey && !e.ctrlKey && !e.altKey) { | |
| e.preventDefault(); | |
| void submitComment(); | |
| } | |
| const handleKeyDown = (e: React.KeyboardEvent) => { | |
| if (e.nativeEvent.isComposing) return; | |
| if (e.key === "Enter" && !e.shiftKey && !e.metaKey && !e.ctrlKey && !e.altKey) { | |
| e.preventDefault(); | |
| void submitComment(); | |
| } | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/comments/CommentInput.tsx` around lines 77 - 80, The Enter-key
handler that calls submitComment currently doesn't prevent submission during IME
composition; update the keydown handler (the block checking e.key === "Enter" &&
!e.shiftKey && !e.metaKey && !e.ctrlKey && !e.altKey) to also check for IME
composition by verifying !e.nativeEvent.isComposing before calling
e.preventDefault() and submitComment(), and for broader compatibility add a
fallback that treats nativeEvent.keyCode === 229 as composing (or alternatively
implement onCompositionStart/onCompositionEnd to track a composing flag and
check that flag here); ensure you reference and update the same handler around
the submitComment invocation so composition prevents accidental submits.
Comment on lines
+829
to
+832
| onChange={(e) => { | ||
| e.stopPropagation(); | ||
| setVideoVolume(Number(e.target.value)); | ||
| }} |
There was a problem hiding this comment.
Stop propagation on slider pointer/click events too, not only change.
At Line 829, e.stopPropagation() only runs for onChange. Parent pointerdown/click handlers can still fire while interacting with the slider. Add onPointerDown (and optionally onClick) propagation guards to fully isolate control interaction.
Suggested patch
<input
aria-label="Volume"
type="range"
min={0}
max={1}
step={0.01}
value={isMuted ? 0 : volume}
+ onPointerDown={(e) => {
+ e.stopPropagation();
+ }}
+ onClick={(e) => {
+ e.stopPropagation();
+ }}
onChange={(e) => {
e.stopPropagation();
setVideoVolume(Number(e.target.value));
}}
className="h-1 w-24 cursor-pointer accent-[color:var(--accent)]"
/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| onChange={(e) => { | |
| e.stopPropagation(); | |
| setVideoVolume(Number(e.target.value)); | |
| }} | |
| <input | |
| aria-label="Volume" | |
| type="range" | |
| min={0} | |
| max={1} | |
| step={0.01} | |
| value={isMuted ? 0 : volume} | |
| onPointerDown={(e) => { | |
| e.stopPropagation(); | |
| }} | |
| onClick={(e) => { | |
| e.stopPropagation(); | |
| }} | |
| onChange={(e) => { | |
| e.stopPropagation(); | |
| setVideoVolume(Number(e.target.value)); | |
| }} | |
| className="h-1 w-24 cursor-pointer accent-[color:var(--accent)]" | |
| /> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/video-player/VideoPlayer.tsx` around lines 829 - 832, In
VideoPlayer.tsx the volume slider only calls e.stopPropagation() inside the
onChange handler (where setVideoVolume is called), allowing parent pointer/click
handlers to still fire; add onPointerDown and onClick handlers on the same
slider element that call e.stopPropagation() (and prevent default if needed) to
fully isolate interactions with the control so parent listeners won't receive
pointer or click events when the user manipulates the slider.
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.
Note
Switch build tooling from Bun to vite-plus and pnpm
vp) as the primary dev/build/lint toolchain; scripts in package.json, vercel.json, and docs/ are updated accordingly.defineConfigfromvite-plusand configures an integrated lint pipeline with oxc; tsconfigPaths plugin is removed in favor of vite-plus built-in resolution.vp staged.../node_modules/tailwindcss/index.css); the external Google Fonts import is removed and the font is loaded via a<link>tag added to the root route head instead.baseUrlfrom tsconfig.json may affect TypeScript module resolution; the Tailwind path change may affect style resolution during SSR or build.Macroscope summarized d975bb4.
Summary by CodeRabbit
Documentation
Improvements
Chores