Conversation
Co-authored-by: raavtube <raavtube@icloud.com>
|
Cursor Agent can help with this pull request. Just |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughIntroduces a floating action bar with copy-to-clipboard (URL and CLI), download, and list actions; adds mobile textarea auto-resize; integrates tracking and toasts for copy actions; adjusts responsive visibility of installation instructions and stats; adds refs/state; reorganizes layout to accommodate the new bar. No exported signatures changed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant P as PublicRulePage
participant CB as Clipboard API
participant T as Toast
participant Tr as Tracking
U->>P: Click "Copy URL"
P->>CB: writeText(window.location.href)
alt success
P->>Tr: track(copy_url)
P->>T: show("URL copied")
else error
P->>T: show("Failed to copy")
end
U->>P: Click "Copy CLI"
P->>CB: writeText("npm i -g ...")
alt success
P->>Tr: track(copy_cli)
P->>T: show("CLI copied")
else error
P->>T: show("Failed to copy")
end
sequenceDiagram
participant W as Window (resize/load)
participant P as PublicRulePage effect
participant TA as Textarea (ref)
W-->>P: resize or content change
P->>TA: measure scrollHeight
P->>TA: set style.height to fit content (mobile only)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
| const handleCopyCLI = async () => { | ||
| if (!rule) return | ||
|
|
||
| const installCommand = `npx shadcn add ${window.location.origin}/api/registry/${rule.id}` | ||
| await navigator.clipboard.writeText(installCommand) | ||
| setCliCopied(true) | ||
| track("CLI Command Copied", { ruleId: rule.id }) | ||
| setTimeout(() => setCliCopied(false), 2000) | ||
| toast.success("Install command copied to clipboard!") | ||
| } |
There was a problem hiding this comment.
The handleCopyCLI function is a duplicate of the existing handleInstallCopy function, creating unnecessary code duplication.
View Details
Analysis
The code contains two identical functions that perform the same operation:
handleInstallCopy(lines 134-143) - original function used by the CLI cardhandleCopyCLI(lines 171-180) - duplicate function used by the floating action bar
Both functions:
- Generate the same install command
- Copy to clipboard
- Set the same state (
setCliCopied(true)) - Show the same toast message
- Use the same timeout duration
The only difference is the tracking event name ("CLI Copied" vs "CLI Command Copied"). This duplication increases bundle size and creates maintenance overhead. Since the CLI card is now hidden on mobile (hidden md:block on line 260), the floating action bar could reuse the existing handleInstallCopy function instead of duplicating it. Either remove the duplicate function and use handleInstallCopy, or remove the original and keep handleCopyCLI if the different tracking event name is intentional.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/rule/[slug]/page.tsx (2)
291-294: Fix SSR crash: avoidwindowin render.Referencing
windowduring render can throw when pre-rendered. Use a guarded expression.- <code className="flex-1 text-sm text-gray-300 font-mono"> - npx shadcn add {window.location.origin}/api/registry/{rule.id} - </code> + <code className="flex-1 text-sm text-gray-300 font-mono"> + {`npx shadcn add ${typeof window !== "undefined" ? window.location.origin : ""}/api/registry/${rule.id}`} + </code>
151-153: Sanitize download filename.Avoid invalid characters and overly long names.
- a.download = `${rule.title}.txt` + const safeTitle = rule.title.replace(/[^\w\-]+/g, "_").slice(0, 64) + a.download = `${safeTitle}.txt`
♻️ Duplicate comments (1)
app/rule/[slug]/page.tsx (1)
171-180: Remove duplicate CLI copy handler and make CLI action visible on mobile.Deduplicate by reusing
handleInstallCopyand expose the CLI action on small screens (matches PR goals).- const handleCopyCLI = async () => { - if (!rule) return - - const installCommand = `npx shadcn add ${window.location.origin}/api/registry/${rule.id}` - await navigator.clipboard.writeText(installCommand) - setCliCopied(true) - track("CLI Command Copied", { ruleId: rule.id }) - setTimeout(() => setCliCopied(false), 2000) - toast.success("Install command copied to clipboard!") - } ... - <Button - onClick={handleCopyCLI} + <Button + onClick={handleInstallCopy} variant="primary" size="sm" - className="hover:bg-[#90BAE0] px-2 py-2 sm:px-3 sm:py-1 aspect-square sm:aspect-auto hidden md:flex" + className="hover:bg-[#90BAE0] px-2 py-2 sm:px-3 sm:py-1 aspect-square sm:aspect-auto" > <Terminal className="h-3 w-3" /> <span className="hidden md:inline ml-1">{cliCopied ? "Copied!" : "CLI"}</span> </Button>Also applies to: 331-339
🧹 Nitpick comments (3)
app/rule/[slug]/page.tsx (3)
182-198: Smoother auto-resize: use rAF and avoid layout thrash.Replace timeout with
requestAnimationFrameand wrap resize handler.- useEffect(() => { - const adjustTextareaHeight = () => { - if (textareaRef.current && window.innerWidth < 768) { - textareaRef.current.style.height = 'auto' - textareaRef.current.style.height = `${textareaRef.current.scrollHeight}px` - } - } - - if (rule?.content) { - setTimeout(adjustTextareaHeight, 100) - } - - window.addEventListener('resize', adjustTextareaHeight) - return () => window.removeEventListener('resize', adjustTextareaHeight) - }, [rule?.content]) + useEffect(() => { + const adjustTextareaHeight = () => { + if (textareaRef.current && window.innerWidth < 768) { + textareaRef.current.style.height = 'auto' + const el = textareaRef.current + requestAnimationFrame(() => { + if (el) el.style.height = `${el.scrollHeight}px` + }) + } + } + if (rule?.content) requestAnimationFrame(adjustTextareaHeight) + const onResize = () => requestAnimationFrame(adjustTextareaHeight) + window.addEventListener('resize', onResize) + return () => window.removeEventListener('resize', onResize) + }, [rule?.content])
306-309: Respect safe-area and add toolbar semantics.Prevent overlap with iOS home indicator and improve a11y.
- <div className="fixed bottom-4 left-1/2 transform -translate-x-1/2 z-40 max-w-[calc(100vw-2rem)]"> + <div className="fixed left-1/2 transform -translate-x-1/2 z-40 max-w-[calc(100vw-2rem)]" style={{ bottom: 'max(1rem, env(safe-area-inset-bottom))' }}> <div className="bg-neutral-300/20 dark:bg-neutral-400/20 text-neutral-600 dark:text-neutral-300 backdrop-blur-[1px] border border-neutral-400/20 rounded-xl p-2 sm:p-3 shadow-2xl overflow-x-auto"> - <div className="flex items-center gap-2 sm:gap-3 min-w-max"> + <div className="flex items-center gap-2 sm:gap-3 min-w-max" role="toolbar" aria-label="Rule actions">
125-143: Analytics naming consistency.Events use mixed names (“CLI Copied” vs “CLI Command Copied”). Standardize for clean dashboards, e.g., “Public Rule CLI Copied”.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
app/rule/[slug]/page.tsx(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/rule/[slug]/page.tsx (1)
components/lists/add-to-list-button.tsx (1)
AddToListButton(15-48)
| const handleCopyViewURL = async () => { | ||
| if (!rule) return | ||
|
|
||
| const viewURL = window.location.href | ||
| await navigator.clipboard.writeText(viewURL) | ||
| setUrlCopied(true) | ||
| track("Public Rule View URL Copied", { ruleId: rule.id }) | ||
| setTimeout(() => setUrlCopied(false), 2000) | ||
| toast.success("View URL copied to clipboard!") | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden clipboard copy with error handling.
Wrap in try/catch and show a failure toast.
- const handleCopyViewURL = async () => {
- if (!rule) return
-
- const viewURL = window.location.href
- await navigator.clipboard.writeText(viewURL)
- setUrlCopied(true)
- track("Public Rule View URL Copied", { ruleId: rule.id })
- setTimeout(() => setUrlCopied(false), 2000)
- toast.success("View URL copied to clipboard!")
- }
+ const handleCopyViewURL = async () => {
+ if (!rule) return
+ try {
+ const viewURL = window.location.href
+ await navigator.clipboard.writeText(viewURL)
+ setUrlCopied(true)
+ track("Public Rule View URL Copied", { ruleId: rule.id })
+ setTimeout(() => setUrlCopied(false), 2000)
+ toast.success("View URL copied to clipboard!")
+ } catch (e) {
+ toast.error("Failed to copy URL. Please copy it manually.")
+ }
+ }Apply the same pattern to handleCopy and handleInstallCopy.
📝 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.
| const handleCopyViewURL = async () => { | |
| if (!rule) return | |
| const viewURL = window.location.href | |
| await navigator.clipboard.writeText(viewURL) | |
| setUrlCopied(true) | |
| track("Public Rule View URL Copied", { ruleId: rule.id }) | |
| setTimeout(() => setUrlCopied(false), 2000) | |
| toast.success("View URL copied to clipboard!") | |
| } | |
| const handleCopyViewURL = async () => { | |
| if (!rule) return | |
| try { | |
| const viewURL = window.location.href | |
| await navigator.clipboard.writeText(viewURL) | |
| setUrlCopied(true) | |
| track("Public Rule View URL Copied", { ruleId: rule.id }) | |
| setTimeout(() => setUrlCopied(false), 2000) | |
| toast.success("View URL copied to clipboard!") | |
| } catch (e) { | |
| toast.error("Failed to copy URL. Please copy it manually.") | |
| } | |
| } |
| value={rule.content} | ||
| readOnly | ||
| className="min-h-[500px] resize-none border-0 bg-[#1B1D21] text-white leading-relaxed" | ||
| className="min-h-[500px] md:min-h-[500px] resize-none border-0 bg-[#1B1D21] text-white leading-relaxed" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Honor PR goal on mobile: remove 500px min-height for small screens.
Currently forces 500px on mobile, causing overflow.
- className="min-h-[500px] md:min-h-[500px] resize-none border-0 bg-[#1B1D21] text-white leading-relaxed"
+ className="min-h-0 md:min-h-[500px] resize-none border-0 bg-[#1B1D21] text-white leading-relaxed"📝 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.
| className="min-h-[500px] md:min-h-[500px] resize-none border-0 bg-[#1B1D21] text-white leading-relaxed" | |
| className="min-h-0 md:min-h-[500px] resize-none border-0 bg-[#1B1D21] text-white leading-relaxed" |
🤖 Prompt for AI Agents
In app/rule/[slug]/page.tsx around line 252, the textarea/class currently forces
min-h-[500px] on all screen sizes which causes overflow on mobile; change the
classes so the 500px minimum only applies at md and above (for example remove
the global min-h-[500px] and use md:min-h-[500px] or add min-h-0
md:min-h-[500px]) and keep the rest of the classes intact to honor the PR goal
of no 500px min-height on small screens.
| <Button | ||
| onClick={handleDownload} | ||
| variant="primary" | ||
| size="sm" | ||
| className="hover:bg-[#70A7D7] px-2 py-2 sm:px-3 sm:py-1 aspect-square sm:aspect-auto" | ||
| > | ||
| <Download className="h-3 w-3" /> | ||
| <span className="hidden md:inline ml-1">Download</span> | ||
| </Button> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add aria-labels for icon-only buttons on mobile.
Improves screen reader accessibility.
<Button
onClick={handleDownload}
variant="primary"
size="sm"
className="hover:bg-[#70A7D7] px-2 py-2 sm:px-3 sm:py-1 aspect-square sm:aspect-auto"
+ aria-label="Download rule"
>
...
<Button
onClick={handleCopy}
variant="primary"
size="sm"
className="hover:bg-[#90BAE0] px-2 py-2 sm:px-3 sm:py-1 aspect-square sm:aspect-auto"
+ aria-label="Copy rule"
>
...
- <Button
- onClick={handleInstallCopy}
+ <Button
+ onClick={handleInstallCopy}
variant="primary"
size="sm"
className="hover:bg-[#90BAE0] px-2 py-2 sm:px-3 sm:py-1 aspect-square sm:aspect-auto"
+ aria-label="Copy CLI install command"
>
...
<Button
onClick={handleCopyViewURL}
variant="primary"
size="sm"
className="hover:bg-[#90BAE0] px-2 py-2 sm:px-3 sm:py-1 aspect-square sm:aspect-auto"
+ aria-label="Copy view URL"
>Also applies to: 321-329, 331-339, 341-349
🤖 Prompt for AI Agents
In app/rule/[slug]/page.tsx around lines 311-319 (and similarly for 321-329,
331-339, 341-349), several Button components render as icon-only on mobile
without aria-labels; add an appropriate aria-label prop to each icon-only Button
(e.g., aria-label="Download", "Copy", "Share", etc.) ensuring labels are
descriptive and match the button action so screen readers can announce their
purpose, and keep existing visible text for larger viewports unchanged.
2 participants
Add a hovering actions bar, hide the CLI card on mobile, and enable auto-expanding textarea on mobile to improve the view rule page's mobile experience and consistency.
The changes address three distinct issues:
Summary by CodeRabbit
New Features
Style