feat: introduce unified fullscreen Tool Playground shell across tools#1
Open
naymurdev wants to merge 24 commits into
Open
feat: introduce unified fullscreen Tool Playground shell across tools#1naymurdev wants to merge 24 commits into
naymurdev wants to merge 24 commits into
Conversation
❌ Deploy Preview for stellar-cendol-140753 failed. Why did it fail? →
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new ToolPlaygroundShell component and wraps multiple tool pages with it; updates tools layout to conditionally hide the header on /svg-line-draw; refactors the SVG line-draw view into a tabbed, grid-based editor and consolidates related UI/state. Changes
Sequence DiagramsequenceDiagram
participant User
participant Shell as ToolPlaygroundShell
participant Tabs as Tabs
participant Presets as PresetFilter
participant Drawer as ExportDrawer
participant Tool as ToolGenerator
User->>Shell: Open tool page
Shell->>Tabs: init topTab
Shell->>Presets: init filter
Shell->>Tool: render child generator
rect rgba(100,150,255,0.5)
User->>Tabs: Click "examples" / "docs"
Tabs->>Shell: update topTab
Shell->>Shell: render selected tab content
end
rect rgba(150,200,100,0.5)
User->>Presets: Type search
Presets->>Shell: update filter
Shell->>Presets: show filtered presets
end
rect rgba(255,200,100,0.5)
User->>Drawer: Click Export
Drawer->>Shell: toggle drawer open
User->>Drawer: Click Copy / Download
Drawer->>User: copy/download + toast
end
User->>Tool: Interact with controls
Tool->>Tool: generate output
Tool->>Shell: provide exportCode / updates
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 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)
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 84373f40ea
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+130
to
+132
| <section className="relative min-h-0 overflow-hidden bg-background/30"> | ||
| <div className="h-full overflow-hidden p-2"> | ||
| <div className="h-full overflow-hidden rounded-xl border bg-background">{children}</div> |
There was a problem hiding this comment.
The new shell wraps {children} in nested overflow-hidden containers, so any tool UI taller than the viewport is clipped with no remaining scroll path. This breaks existing tool pages that render full-page editors (for example, components/view/svg-line-draw/index.tsx uses h-screen/h-[100vh] sections) and rely on vertical movement to reach controls, making parts of the editor unreachable after this change.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/(tools)/color-lab/page.tsx (1)
1-1:⚠️ Potential issue | 🟡 MinorUnused import:
ClipPathGenerator.
ClipPathGeneratoris imported but never used — the page rendersColorConverter. This looks like a copy-paste artifact from the clip-paths page.Proposed fix
-import ClipPathGenerator from "@/components/view/clip-path";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/`(tools)/color-lab/page.tsx at line 1, The file imports ClipPathGenerator but never uses it (page renders ColorConverter), so remove the unused import to clean up the module; locate the import statement for ClipPathGenerator in page.tsx and delete it (leaving the ColorConverter import and export/render logic unchanged).
🧹 Nitpick comments (3)
app/(tools)/layout.tsx (1)
6-8: Decouple<main>height from header flow to prevent clipping.With
h-screenon the wrapper andh-fullon<main>, any non‑fixedToolsHeaderheight can push content below the viewport whileoverflow-hiddenclips it. A flex column layout makes the header height explicit and keeps<main>filling the remaining space without coupling to header positioning.Suggested tweak
- <div className="h-screen overflow-hidden bg-white text-black dark:bg-black dark:text-white"> + <div className="flex h-screen flex-col overflow-hidden bg-white text-black dark:bg-black dark:text-white"> <ToolsHeader /> - <main className="h-full overflow-hidden px-3 pb-3 pt-20 xl:px-6">{children}</main> + <main className="flex-1 overflow-hidden px-3 pb-3 pt-20 xl:px-6">{children}</main> </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/`(tools)/layout.tsx around lines 6 - 8, The container is using h-screen with h-full on <main>, which lets a non‑fixed ToolsHeader push content out of view and be clipped; change the wrapper div to a column flex container (e.g., add flex flex-col and use min-h-screen instead of h-screen) so header height is taken into account, and update the <main> element (the main in this layout that contains {children}) to use flex-1 and overflow-auto (remove h-full and overflow-hidden) so it fills remaining space under ToolsHeader and scrolls independently.app/(tools)/color-lab/page.tsx (1)
72-86:PageLoadingcomponent is defined but never used.The
PageLoadingskeleton is declared but not referenced anywhere in the returned JSX (no<Suspense>boundary wrapsColorConverter). Either remove it or wrapColorConverterin a<Suspense fallback={<PageLoading />}>for consistency with other tool pages likesvg-line-draw/page.tsx.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/`(tools)/color-lab/page.tsx around lines 72 - 86, The PageLoading skeleton component is declared but never used; either remove PageLoading or wrap the ColorConverter render in a React Suspense with fallback={<PageLoading />}. Locate the PageLoading declaration and the ColorConverter usage in this file (symbols: PageLoading, ColorConverter) and update the returned JSX to: <Suspense fallback={<PageLoading />}>...<ColorConverter .../></Suspense> (or delete PageLoading if you prefer), and ensure Suspense is imported from React if added.components/common/tool-playground-shell.tsx (1)
68-70:location.reload()for Reset is destructive in a SPA.A full page reload discards all React state across the entire page (not just the tool), triggers a network round-trip, and is jarring UX. Consider accepting an
onResetcallback prop so each tool can reset its own state, or at minimum use Next.jsrouter.refresh().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/common/tool-playground-shell.tsx` around lines 68 - 70, The Reset button currently calls location.reload(), which is destructive; change ToolPlaygroundShell to accept an optional onReset callback prop and replace the onClick handler so it calls props.onReset() when provided and otherwise uses Next.js router.refresh(); specifically, add an optional onReset?: () => void to the ToolPlaygroundShell props, import and use useRouter() inside the component, and update the Button's onClick from () => location.reload() to () => { if (onReset) onReset(); else router.refresh(); } to avoid a full page reload.
🤖 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/`(tools)/mesh-gradients/page.tsx:
- Around line 56-65: The ShaderGradientGenerator's fixed viewport sizing
(h-[95vh]) and its internal control/sidebar conflict with ToolPlaygroundShell's
h-full overflow-hidden container and duplicate the shell's presets; update the
integration by adding a prop to ToolPlaygroundShell (e.g., disableInnerControls
or embedded=true) and modify ShaderGradientGenerator to accept an isEmbedded
prop so it switches to container-relative sizing (replace h-[95vh] with h-full
or flex-grow styles) and hides or collapses its own preset/sidebar and top-level
control panel when isEmbedded is true; alternatively remove the
ToolPlaygroundShell wrapper here until the component is refactored, but the
preferred fix is to pass a prop from the page and conditionally adjust
ShaderGradientGenerator's layout and control rendering.
In `@components/common/tool-playground-shell.tsx`:
- Around line 107-113: The preset buttons rendered in ToolPlaygroundShell (the
<button> created for each {item}) have no onClick handler so they do nothing;
add an onClick prop to the button that calls a handler (e.g., handleSelectPreset
or onSelectPreset prop) which loads/applies the preset (or, if still
unimplemented, disable the button via disabled and add a "coming soon" badge or
aria-label) and ensure the handler receives the unique preset identifier (item)
so the preset state/update logic in ToolPlaygroundShell can apply it.
- Around line 89-128: The sidebar can push main content off-screen on small
viewports because the grid becomes single-column; update the layout so the
sidebar (the <aside> element in tool-playground-shell.tsx) is hidden or
collapsible below lg: either add a responsive visibility class (e.g., hide on
<lg and show on lg) to the aside or implement a simple toggle state (e.g.,
sidebarOpen with a button) so the aside is not rendered/occupying space on small
screens; ensure the main content region (the sibling grid column) expands to
full width when the sidebar is hidden and keep the existing search/controls
accessible via the toggle or a separate mobile-only control.
- Around line 53-59: The mobile UI hides the Tabs component
(Tabs/TabsList/TabsTrigger) so mobile users can't switch tabs; update the mobile
dropdown logic (the component that currently offers Export/Copy/Reset) to
include the three tab options "Playground", "Examples", and "Docs", render the
active tab based on the topTab state, and call setTopTab with the appropriate
value when a mobile option is selected so mobile users can switch tabs the same
way desktop does.
- Around line 156-158: Add an onClick to the Download Button that calls a new
handler (e.g., handleDownload) and implement that handler inside the same
component (components/common/tool-playground-shell.tsx). The handler should read
the current playground content (the component state or prop that holds the
output/editor value), create a Blob from that string, generate an object URL via
URL.createObjectURL, create and click a temporary anchor with download
attribute, and then revoke the object URL to free resources; wire this handler
to the <Button ...> containing the <Download /> icon so the button triggers a
file download.
- Around line 40-43: The copyCode function calls
navigator.clipboard.writeText(exportCode) without handling rejections; wrap the
async call in try/catch inside copyCode, await
navigator.clipboard.writeText(exportCode) in the try, call
toast.success("Copied") on success, and in the catch call toast.error with a
friendly message and optionally fall back to a legacy copy approach (e.g.,
creating a temporary textarea and document.execCommand('copy')) to handle
insecure contexts or permission denials; reference the copyCode function,
navigator.clipboard.writeText, exportCode, and toast.success/toast.error when
applying the change.
- Around line 137-161: The export drawer currently toggles via drawerOpen and
setDrawerOpen but lacks an overlay, Escape-key handler, and focus trapping;
update the component (tool-playground-shell.tsx) to render a backdrop element
when drawerOpen is true that calls setDrawerOpen(false) on click, add a
useEffect that listens for 'keydown' and closes the drawer when event.key ===
'Escape' using setDrawerOpen(false), and implement a focus trap around the
drawer (preferably by wrapping the drawer content in a FocusLock from
react-focus-lock or a similar utility) so keyboard focus cannot tab out while
open; ensure copyCode, exportCode and exportLabel behavior remains unchanged.
---
Outside diff comments:
In `@app/`(tools)/color-lab/page.tsx:
- Line 1: The file imports ClipPathGenerator but never uses it (page renders
ColorConverter), so remove the unused import to clean up the module; locate the
import statement for ClipPathGenerator in page.tsx and delete it (leaving the
ColorConverter import and export/render logic unchanged).
---
Nitpick comments:
In `@app/`(tools)/color-lab/page.tsx:
- Around line 72-86: The PageLoading skeleton component is declared but never
used; either remove PageLoading or wrap the ColorConverter render in a React
Suspense with fallback={<PageLoading />}. Locate the PageLoading declaration and
the ColorConverter usage in this file (symbols: PageLoading, ColorConverter) and
update the returned JSX to: <Suspense fallback={<PageLoading
/>}>...<ColorConverter .../></Suspense> (or delete PageLoading if you prefer),
and ensure Suspense is imported from React if added.
In `@app/`(tools)/layout.tsx:
- Around line 6-8: The container is using h-screen with h-full on <main>, which
lets a non‑fixed ToolsHeader push content out of view and be clipped; change the
wrapper div to a column flex container (e.g., add flex flex-col and use
min-h-screen instead of h-screen) so header height is taken into account, and
update the <main> element (the main in this layout that contains {children}) to
use flex-1 and overflow-auto (remove h-full and overflow-hidden) so it fills
remaining space under ToolsHeader and scrolls independently.
In `@components/common/tool-playground-shell.tsx`:
- Around line 68-70: The Reset button currently calls location.reload(), which
is destructive; change ToolPlaygroundShell to accept an optional onReset
callback prop and replace the onClick handler so it calls props.onReset() when
provided and otherwise uses Next.js router.refresh(); specifically, add an
optional onReset?: () => void to the ToolPlaygroundShell props, import and use
useRouter() inside the component, and update the Button's onClick from () =>
location.reload() to () => { if (onReset) onReset(); else router.refresh(); } to
avoid a full page reload.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
app/(tools)/background-snippets/page.tsxapp/(tools)/clip-paths/page.tsxapp/(tools)/color-lab/page.tsxapp/(tools)/layout.tsxapp/(tools)/mesh-gradients/page.tsxapp/(tools)/shadows/page.tsxapp/(tools)/svg-line-draw/page.tsxcomponents/common/tool-playground-shell.tsx
Comment on lines
+56
to
+65
| <ToolPlaygroundShell | ||
| title="Mesh Gradient Generator" | ||
| description="Build fluid mesh gradients with draggable color points." | ||
| examples={["Aurora", "Sunset", "Neon", "Pastel"]} | ||
| docs="Use Playground for live editing, Examples to load quick presets, and Export to copy production-ready output." | ||
| exportLabel="CSS" | ||
| exportCode={`/* export output from active tool */`} | ||
| > | ||
| <ShaderGradientGenerator /> | ||
| </> | ||
| </ToolPlaygroundShell> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "*.tsx" -o -name "*.ts" | grep -i "mesh-gradient" | head -20Repository: ui-layouts/ui-tools
Length of output: 292
🏁 Script executed:
rg -n 'h-\[.*vh\]' --type=tsx --type=ts -g '*/mesh-gradient/*'Repository: ui-layouts/ui-tools
Length of output: 90
🏁 Script executed:
rg -n 'col-span|ScrollArea|overflow' --type=tsx --type=ts -g '*/mesh-gradient/*'Repository: ui-layouts/ui-tools
Length of output: 90
🏁 Script executed:
rg 'h-\[.*vh\]' ./components/view/mesh-gradient/Repository: ui-layouts/ui-tools
Length of output: 471
🏁 Script executed:
rg 'col-span|ScrollArea|overflow' ./components/view/mesh-gradient/Repository: ui-layouts/ui-tools
Length of output: 1737
🏁 Script executed:
wc -l ./components/view/mesh-gradient/index.tsxRepository: ui-layouts/ui-tools
Length of output: 108
🏁 Script executed:
cat -n ./components/view/mesh-gradient/index.tsx | head -100Repository: ui-layouts/ui-tools
Length of output: 3243
🏁 Script executed:
cat -n ./components/view/mesh-gradient/index.tsx | sed -n '100,200p'Repository: ui-layouts/ui-tools
Length of output: 2299
🏁 Script executed:
cat -n ./app/\(tools\)/mesh-gradients/page.tsxRepository: ui-layouts/ui-tools
Length of output: 2671
🏁 Script executed:
cat -n ./components/view/mesh-gradient/index.tsx | sed -n '200,300p'Repository: ui-layouts/ui-tools
Length of output: 3226
🏁 Script executed:
cat -n ./components/view/mesh-gradient/index.tsx | sed -n '250,350p'Repository: ui-layouts/ui-tools
Length of output: 4166
🏁 Script executed:
wc -l ./components/common/tool-playground-shell.tsxRepository: ui-layouts/ui-tools
Length of output: 112
🏁 Script executed:
cat -n ./components/common/tool-playground-shell.tsx | head -150Repository: ui-layouts/ui-tools
Length of output: 6158
🏁 Script executed:
cat -n ./components/view/mesh-gradient/index.tsx | sed -n '343,400p'Repository: ui-layouts/ui-tools
Length of output: 2739
🏁 Script executed:
cat -n ./components/view/mesh-gradient/index.tsx | sed -n '348,380p'Repository: ui-layouts/ui-tools
Length of output: 1655
🏁 Script executed:
cat -n ./components/view/mesh-gradient/index.tsx | sed -n '400,460p'Repository: ui-layouts/ui-tools
Length of output: 2680
🏁 Script executed:
cat -n ./components/view/mesh-gradient/control-panel.tsx | head -30Repository: ui-layouts/ui-tools
Length of output: 1042
🏁 Script executed:
cat -n ./components/view/mesh-gradient/example-gradients.tsx | head -20Repository: ui-layouts/ui-tools
Length of output: 631
🏁 Script executed:
cat -n ./components/common/tool-playground-shell.tsx | sed -n '128,135p'Repository: ui-layouts/ui-tools
Length of output: 390
🏁 Script executed:
# Check if there's a render of this page to verify actual layout behavior
git log --oneline -20 -- ./app/\(tools\)/mesh-gradients/page.tsx 2>/dev/null | head -5Repository: ui-layouts/ui-tools
Length of output: 121
🏁 Script executed:
# Search for other tools using ToolPlaygroundShell to compare patterns
rg 'ToolPlaygroundShell' --type=tsx -lRepository: ui-layouts/ui-tools
Length of output: 90
🏁 Script executed:
# Check other tool pages to see if they have similar issues
fd 'page.tsx' --path '*/tools/*' --type f | head -10Repository: ui-layouts/ui-tools
Length of output: 278
🏁 Script executed:
# Get context on one other tool page for comparison
cat -n ./app/\(tools\)/*/page.tsx 2>/dev/null | head -80Repository: ui-layouts/ui-tools
Length of output: 3528
🏁 Script executed:
# Check how BackgroundSnippetsGenerator is structured for comparison
wc -l ./components/view/background-snippets/index.tsxRepository: ui-layouts/ui-tools
Length of output: 114
🏁 Script executed:
# Peek at BackgroundSnippetsGenerator to see if it has similar viewport-height issues
rg 'h-\[.*vh\]' ./components/view/background-snippets/ | head -20Repository: ui-layouts/ui-tools
Length of output: 461
🏁 Script executed:
# Check other tool generators for comparison
fd 'index.tsx' --path '*/view/*' | head -15Repository: ui-layouts/ui-tools
Length of output: 278
🏁 Script executed:
# Check structure of BackgroundSnippetsGenerator
cat -n ./components/view/background-snippets/index.tsx | head -100Repository: ui-layouts/ui-tools
Length of output: 4513
🏁 Script executed:
# Check what other view components exist
fd 'index.tsx' ./components/viewRepository: ui-layouts/ui-tools
Length of output: 335
🏁 Script executed:
# Check if BackgroundSnippetsGenerator also uses grid layout like ShaderGradientGenerator
cat -n ./components/view/background-snippets/index.tsx | sed -n '150,250p'Repository: ui-layouts/ui-tools
Length of output: 3677
🏁 Script executed:
# Look for the actual grid structure in BackgroundSnippetsGenerator
rg -n 'lg:grid|lg:col-span|grid-cols' ./components/view/background-snippets/index.tsxRepository: ui-layouts/ui-tools
Length of output: 680
Layout breakage: ShaderGradientGenerator viewport-height sizing conflicts with shell's overflow-constrained container.
The generator renders a full grid layout with fixed h-[95vh] dimensions (control panel at line 351, preview area at line 371 in components/view/mesh-gradient/index.tsx). When wrapped by ToolPlaygroundShell, these elements are placed inside a h-full overflow-hidden container (line 132 of tool-playground-shell.tsx), causing the viewport-height content to be clipped. Additionally, the generator's own control panel and example-gradient selector will appear alongside the shell's preset sidebar, creating duplicate control areas.
The generator needs either a refactored layout that respects the shell's constraints, or should skip the shell wrapper until the layout is redesigned.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/`(tools)/mesh-gradients/page.tsx around lines 56 - 65, The
ShaderGradientGenerator's fixed viewport sizing (h-[95vh]) and its internal
control/sidebar conflict with ToolPlaygroundShell's h-full overflow-hidden
container and duplicate the shell's presets; update the integration by adding a
prop to ToolPlaygroundShell (e.g., disableInnerControls or embedded=true) and
modify ShaderGradientGenerator to accept an isEmbedded prop so it switches to
container-relative sizing (replace h-[95vh] with h-full or flex-grow styles) and
hides or collapses its own preset/sidebar and top-level control panel when
isEmbedded is true; alternatively remove the ToolPlaygroundShell wrapper here
until the component is refactored, but the preferred fix is to pass a prop from
the page and conditionally adjust ShaderGradientGenerator's layout and control
rendering.
Comment on lines
+40
to
+43
| const copyCode = async () => { | ||
| await navigator.clipboard.writeText(exportCode); | ||
| toast.success("Copied"); | ||
| }; |
There was a problem hiding this comment.
Unhandled clipboard API rejection.
navigator.clipboard.writeText rejects in insecure contexts (plain HTTP, some iframes) and when the user denies permission. The unhandled rejection will surface as a console error with no user feedback.
🛡️ Proposed fix
const copyCode = async () => {
- await navigator.clipboard.writeText(exportCode);
- toast.success("Copied");
+ try {
+ await navigator.clipboard.writeText(exportCode);
+ toast.success("Copied");
+ } catch {
+ toast.error("Failed to copy");
+ }
};📝 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
| const copyCode = async () => { | |
| await navigator.clipboard.writeText(exportCode); | |
| toast.success("Copied"); | |
| }; | |
| const copyCode = async () => { | |
| try { | |
| await navigator.clipboard.writeText(exportCode); | |
| toast.success("Copied"); | |
| } catch { | |
| toast.error("Failed to copy"); | |
| } | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/common/tool-playground-shell.tsx` around lines 40 - 43, The
copyCode function calls navigator.clipboard.writeText(exportCode) without
handling rejections; wrap the async call in try/catch inside copyCode, await
navigator.clipboard.writeText(exportCode) in the try, call
toast.success("Copied") on success, and in the catch call toast.error with a
friendly message and optionally fall back to a legacy copy approach (e.g.,
creating a temporary textarea and document.execCommand('copy')) to handle
insecure contexts or permission denials; reference the copyCode function,
navigator.clipboard.writeText, exportCode, and toast.success/toast.error when
applying the change.
Comment on lines
+53
to
+59
| <Tabs value={topTab} onValueChange={setTopTab} className="hidden md:block"> | ||
| <TabsList> | ||
| <TabsTrigger value="playground">Playground</TabsTrigger> | ||
| <TabsTrigger value="examples">Examples</TabsTrigger> | ||
| <TabsTrigger value="docs">Docs</TabsTrigger> | ||
| </TabsList> | ||
| </Tabs> |
There was a problem hiding this comment.
Mobile users cannot switch between Playground / Examples / Docs tabs.
The <Tabs> component is hidden md:block, so it's invisible on small screens. The mobile dropdown menu (lines 73-86) only exposes Export/Copy/Reset — tab switching is missing entirely, leaving mobile users locked on the default "playground" tab.
🛡️ Proposed fix: add tab options to the mobile dropdown
<DropdownMenuContent align="end">
+ <DropdownMenuItem onClick={() => setTopTab("playground")}>Playground</DropdownMenuItem>
+ <DropdownMenuItem onClick={() => setTopTab("examples")}>Examples</DropdownMenuItem>
+ <DropdownMenuItem onClick={() => setTopTab("docs")}>Docs</DropdownMenuItem>
+ <DropdownMenuSeparator />
<DropdownMenuItem onClick={() => setDrawerOpen(true)}>
Export code
</DropdownMenuItem>Also applies to: 73-86
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/common/tool-playground-shell.tsx` around lines 53 - 59, The mobile
UI hides the Tabs component (Tabs/TabsList/TabsTrigger) so mobile users can't
switch tabs; update the mobile dropdown logic (the component that currently
offers Export/Copy/Reset) to include the three tab options "Playground",
"Examples", and "Docs", render the active tab based on the topTab state, and
call setTopTab with the appropriate value when a mobile option is selected so
mobile users can switch tabs the same way desktop does.
Comment on lines
+89
to
+128
| <div className="grid min-h-0 flex-1 grid-cols-1 lg:grid-cols-[380px_minmax(0,1fr)]"> | ||
| <aside className="min-h-0 border-r"> | ||
| <div className="sticky top-0 z-10 border-b bg-card-bg p-3"> | ||
| <Input | ||
| placeholder="Search presets" | ||
| value={filter} | ||
| onChange={(e) => setFilter(e.target.value)} | ||
| /> | ||
| </div> | ||
| <div className="h-[calc(100%-60px)] overflow-y-auto p-3"> | ||
| {topTab !== "docs" && ( | ||
| <section className="mb-4 space-y-2"> | ||
| <h2 className="font-medium text-sm">Presets / Examples</h2> | ||
| {examples | ||
| .filter((item) => | ||
| item.toLowerCase().includes(filter.toLowerCase()), | ||
| ) | ||
| .map((item) => ( | ||
| <button | ||
| type="button" | ||
| key={item} | ||
| className="mb-2 block w-full rounded-md border px-3 py-2 text-left text-sm hover:bg-accent" | ||
| > | ||
| {item} | ||
| </button> | ||
| ))} | ||
| </section> | ||
| )} | ||
| <section> | ||
| <h2 className="mb-2 font-medium text-sm"> | ||
| {topTab === "docs" ? "Docs" : "Controls"} | ||
| </h2> | ||
| <div className="rounded-md border bg-background p-3 text-muted-foreground text-sm"> | ||
| {topTab === "docs" | ||
| ? docs | ||
| : "All existing controls are preserved below in the tool panel. Use this sidebar as the single scroll region."} | ||
| </div> | ||
| </section> | ||
| </div> | ||
| </aside> |
There was a problem hiding this comment.
Sidebar layout may be unusable on mobile viewports.
On < lg screens the grid is grid-cols-1, so the sidebar and content area stack vertically inside a flex container with min-h-0 and overflow-hidden. The sidebar has no max-height constraint in this mode, potentially pushing the main content entirely off-screen (or both regions collapsing to zero). Consider hiding the sidebar on small screens or making it collapsible.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/common/tool-playground-shell.tsx` around lines 89 - 128, The
sidebar can push main content off-screen on small viewports because the grid
becomes single-column; update the layout so the sidebar (the <aside> element in
tool-playground-shell.tsx) is hidden or collapsible below lg: either add a
responsive visibility class (e.g., hide on <lg and show on lg) to the aside or
implement a simple toggle state (e.g., sidebarOpen with a button) so the aside
is not rendered/occupying space on small screens; ensure the main content region
(the sibling grid column) expands to full width when the sidebar is hidden and
keep the existing search/controls accessible via the toggle or a separate
mobile-only control.
Comment on lines
+107
to
+113
| <button | ||
| type="button" | ||
| key={item} | ||
| className="mb-2 block w-full rounded-md border px-3 py-2 text-left text-sm hover:bg-accent" | ||
| > | ||
| {item} | ||
| </button> |
There was a problem hiding this comment.
Preset buttons are non-functional — no click handler.
Every preset item renders as a <button> but has no onClick. Users will click these expecting to load a preset and nothing will happen. If this is intentional scaffolding, consider either disabling the buttons or adding a visual indicator (e.g., "coming soon" badge) so users don't perceive the UI as broken.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/common/tool-playground-shell.tsx` around lines 107 - 113, The
preset buttons rendered in ToolPlaygroundShell (the <button> created for each
{item}) have no onClick handler so they do nothing; add an onClick prop to the
button that calls a handler (e.g., handleSelectPreset or onSelectPreset prop)
which loads/applies the preset (or, if still unimplemented, disable the button
via disabled and add a "coming soon" badge or aria-label) and ensure the handler
receives the unique preset identifier (item) so the preset state/update logic in
ToolPlaygroundShell can apply it.
Comment on lines
+137
to
+161
| <div | ||
| className={cn( | ||
| "pointer-events-none fixed top-0 right-0 z-[90] h-screen w-full max-w-xl translate-x-full border-l bg-background/95 opacity-0 backdrop-blur transition-all", | ||
| drawerOpen && "pointer-events-auto translate-x-0 opacity-100", | ||
| )} | ||
| > | ||
| <div className="flex h-16 items-center justify-between border-b px-4"> | ||
| <h3 className="font-semibold">Export</h3> | ||
| <Button variant="outline" size="sm" onClick={() => setDrawerOpen(false)}> | ||
| Close | ||
| </Button> | ||
| </div> | ||
| <div className="h-[calc(100vh-64px)] overflow-y-auto p-4"> | ||
| <p className="mb-2 text-muted-foreground text-xs">{exportLabel}</p> | ||
| <pre className="overflow-x-auto rounded-md border bg-card p-3 text-xs">{exportCode}</pre> | ||
| <div className="mt-3 flex gap-2"> | ||
| <Button size="sm" onClick={copyCode}> | ||
| <Copy className="mr-1 h-4 w-4" /> Copy | ||
| </Button> | ||
| <Button size="sm" variant="outline"> | ||
| <Download className="mr-1 h-4 w-4" /> Download | ||
| </Button> | ||
| </div> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
Export drawer lacks backdrop, keyboard dismiss, and focus trap.
The slide-in drawer has no overlay behind it (clicking outside does nothing), no Escape key listener to close, and no focus trap. This degrades both usability (easy to get "stuck") and accessibility (keyboard/screen-reader users can tab behind the drawer).
🛡️ Minimal fix: add overlay + Escape handler
+ {/* Overlay */}
+ {drawerOpen && (
+ <div
+ className="fixed inset-0 z-[89] bg-black/40"
+ onClick={() => setDrawerOpen(false)}
+ onKeyDown={(e) => e.key === "Escape" && setDrawerOpen(false)}
+ role="presentation"
+ />
+ )}
<div
className={cn(
"pointer-events-none fixed top-0 right-0 z-[90] h-screen w-full max-w-xl translate-x-full border-l bg-background/95 opacity-0 backdrop-blur transition-all",
drawerOpen && "pointer-events-auto translate-x-0 opacity-100",
)}
>For a proper focus trap, consider using a library like @radix-ui/react-dialog or react-focus-lock.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/common/tool-playground-shell.tsx` around lines 137 - 161, The
export drawer currently toggles via drawerOpen and setDrawerOpen but lacks an
overlay, Escape-key handler, and focus trapping; update the component
(tool-playground-shell.tsx) to render a backdrop element when drawerOpen is true
that calls setDrawerOpen(false) on click, add a useEffect that listens for
'keydown' and closes the drawer when event.key === 'Escape' using
setDrawerOpen(false), and implement a focus trap around the drawer (preferably
by wrapping the drawer content in a FocusLock from react-focus-lock or a similar
utility) so keyboard focus cannot tab out while open; ensure copyCode,
exportCode and exportLabel behavior remains unchanged.
Comment on lines
+156
to
+158
| <Button size="sm" variant="outline"> | ||
| <Download className="mr-1 h-4 w-4" /> Download | ||
| </Button> |
There was a problem hiding this comment.
Download button has no onClick handler.
The Download button renders but does nothing. Same concern as the preset buttons — users will click it expecting a file download.
I can help implement a Blob-based download if you'd like — want me to open an issue or generate the handler?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/common/tool-playground-shell.tsx` around lines 156 - 158, Add an
onClick to the Download Button that calls a new handler (e.g., handleDownload)
and implement that handler inside the same component
(components/common/tool-playground-shell.tsx). The handler should read the
current playground content (the component state or prop that holds the
output/editor value), create a Blob from that string, generate an object URL via
URL.createObjectURL, create and click a temporary anchor with download
attribute, and then revoke the object URL to free resources; wire this handler
to the <Button ...> containing the <Download /> icon so the button triggers a
file download.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
components/view/svg-line-draw/index.tsx (4)
158-164:⚠️ Potential issue | 🟡 Minor
navigator.clipboard.writeTextis async — success toast fires even on failure.The promise is not awaited and has no
.catch, so the "code copied" toast appears regardless of whether the write succeeded.✏️ Proposed fix
-const copyCode = () => { +const copyCode = async () => { const code = generateCode(); - navigator.clipboard.writeText(code); - toast.success("code copied", { - description: "The code has been copied to your clipboard", - }); + try { + await navigator.clipboard.writeText(code); + toast.success("Code copied", { + description: "The code has been copied to your clipboard", + }); + } catch { + toast.error("Failed to copy", { + description: "Could not access the clipboard", + }); + } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/view/svg-line-draw/index.tsx` around lines 158 - 164, The copyCode function calls navigator.clipboard.writeText without awaiting or handling errors, causing the success toast to show even if the copy fails; update copyCode to await navigator.clipboard.writeText(generateCode()) (or use .then/.catch) and only call toast.success after the promise resolves, and call toast.error (with a helpful description) if the promise rejects; reference the copyCode function, generateCode(), navigator.clipboard.writeText, toast.success and toast.error when making the change.
32-32:⚠️ Potential issue | 🟠 MajorUse the project's
ScrollAreawrapper instead of the raw Radix primitive.
@radix-ui/react-scroll-area'sScrollArearoot is unstyled and requires explicitViewport/Scrollbarchildren to function. The project already provides a composed wrapper at@/components/ui/scroll-area(seecomponents/ui/scroll-area.tsx). Theoverflow-autoclass on line 284 is a band-aid that bypasses Radix's scroll virtualization and custom scrollbar rendering.♻️ Proposed fix
-import { ScrollArea } from "@radix-ui/react-scroll-area"; +import { ScrollArea } from "@/components/ui/scroll-area";Also applies to: 280-291
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/view/svg-line-draw/index.tsx` at line 32, Replace the raw Radix ScrollArea import with the project's composed wrapper and remove the workaround overflow styles: change the import "import { ScrollArea } from \"@radix-ui/react-scroll-area\";" to import the project's ScrollArea (from "@/components/ui/scroll-area"), then update the component usage that currently relies on the unstyled Radix root (the block around the class "overflow-auto" and lines ~280-291) to use the wrapper's API/children (no manual Viewport/Scrollbar or overflow-auto band-aid). Ensure any props passed to the Radix root are compatible with the wrapper (adjust prop names if needed) and delete the explicit "overflow-auto" class so the wrapper can manage scrolling and custom scrollbars.
137-141:⚠️ Potential issue | 🟡 MinorWrong toast variant:
toast.successused for a validation failure."No path to save" is not a success. Use
toast.errorortoast.warninginstead.- toast.success("No path to save", { + toast.error("No path to save", {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/view/svg-line-draw/index.tsx` around lines 137 - 141, The toast uses the wrong variant for a validation failure: replace the call to toast.success when currentPath is falsy with a non-success variant (e.g., toast.error or toast.warning) so the UI reflects an error state; locate the early-return block that checks currentPath (inside the save handler where the code reads "if (!currentPath) { toast.success(\"No path to save\", ...); return; }") and change the toast invocation to toast.error (or toast.warning) while keeping the same message and description.
29-33:⚠️ Potential issue | 🟡 MinorImport
TabsTriggerfrom the project'stabs.tsxinstead of directly from Radix.The project's
components/ui/tabs.tsxexplicitly exportsTabsTrigger(line 55), but this file imports it directly from@radix-ui/react-tabsinstead. This is inconsistent with howTabs,TabsContent, andTabsListare imported from the project's wrapper. Use the wrapper export to ensure consistent styling and behavior across all tabs components.Suggested fix
-import { Tabs, TabsContent, TabsList } from "@/components/ui/tabs"; +import { Tabs, TabsContent, TabsList, TabsTrigger } from "@/components/ui/tabs"; ... -import { TabsTrigger } from "@radix-ui/react-tabs";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/view/svg-line-draw/index.tsx` around lines 29 - 33, The file imports TabsTrigger from "@radix-ui/react-tabs" while Tabs, TabsContent and TabsList are imported from the project's wrapper; change the import so TabsTrigger is imported from the project's tabs module (the same module that exports Tabs/TabsContent/TabsList) to ensure consistent styling and behavior; update the import statement to pull TabsTrigger alongside Tabs, TabsContent, TabsList (referencing the TabsTrigger symbol) and remove the direct Radix import.
🧹 Nitpick comments (6)
components/view/svg-line-draw/index.tsx (6)
367-370: Prefer a narrowing cast overas anyfor type-safe Select handlers.
as anysuppresses all type checking. A targeted cast likevalue as AnimationSettings["strokeLinecap"]keeps the compiler guarding against invalid values. Same applies at lines 463, 623, and 690.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/view/svg-line-draw/index.tsx` around lines 367 - 370, The handler currently uses a blanket cast "as any" which defeats type checking; change the cast to a narrowing cast using the AnimationSettings index type, e.g. replace the cast in the Select onValueChange for settings.strokeLinecap so it calls updateSetting("strokeLinecap", value as AnimationSettings["strokeLinecap"]) instead of value as any, and apply the same pattern for the other Select handlers flagged (the other onValueChange occurrences at the same pattern around lines 463, 623, and 690) so each uses the corresponding AnimationSettings["<propertyName>"] type when calling updateSetting.
178-186: Significant amount of commented-out code throughout the file.There are multiple blocks of commented-out code (lines 146-149, 178-186, 273-278, 481-486, 583, 662-700, 746). If these are no longer needed for the new playground layout, remove them; if they represent planned features, track them as issues/TODOs instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/view/svg-line-draw/index.tsx` around lines 178 - 186, Remove the large commented-out blocks that are residual from the old playground layout (e.g., the commented openEditorForExample function and other commented sections around the component), or if the functionality is intended for future work, convert each block into a focused TODO comment and create corresponding issue tickets instead; specifically, delete the commented definition for openEditorForExample and any other commented helper logic, or replace them with concise TODOs that reference the function names (openEditorForExample, any commented state setters like setCurrentPath/setEditorViewBox/setShowEditor) so future work is tracked in the issue tracker rather than left as commented code.
522-527: No-op ternary: both branches resolve to"justify-between".Simplify to a static class string.
✏️ Proposed fix
- className={cn( - "flex flex-wrap items-center", - customDrawLine || editPath - ? "justify-between" - : "justify-between", - )} + className="flex flex-wrap items-center justify-between"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/view/svg-line-draw/index.tsx` around lines 522 - 527, The ternary inside the JSX className call is a no-op because both branches return "justify-between"; in the className={cn(...)} expression remove the conditional and replace the ternary expression with the static class "justify-between" (leave the surrounding cn call and other classes intact); reference the existing variables customDrawLine and editPath only to confirm they are no longer needed in this class expression within this component's render.
136-150: Remove debugconsole.logon line 144.This appears to be a leftover from development.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/view/svg-line-draw/index.tsx` around lines 136 - 150, The savePath function contains a leftover debug console.log(currentPath) that should be removed; locate the savePath function and delete the console.log(currentPath) statement (leave the existing toast and setShowSaveDialog/currentPath checks intact) so no development debug output remains in production.
96-100: Dead state:savedPathsis never mutated, and several state variables are fully unused.
savedPathsis initialized to[]and_setSavedPathsis never called, sosavedPaths.length > 0(line 657) is alwaysfalse— the truthy branch is unreachable dead code, andgenerateCode(line 191) always takes the single-path branch._showEditor/setShowEditorand_editorViewBox/_setEditorViewBoxare remnants with no live consumers (the only caller,_openEditor, is itself dead).Consider removing these to avoid confusing future readers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/view/svg-line-draw/index.tsx` around lines 96 - 100, Remove the dead React state and unreachable branches: delete the unused savedPaths state and its setter (_setSavedPaths) and simplify any conditional that checks savedPaths.length (make generateCode and the UI always use the single-path flow via currentPath); also remove the unused editor-related state variables (_showEditor, setShowEditor, _editorViewBox, _setEditorViewBox) and the dead helper _openEditor so no dangling references remain. Update references in generateCode and the component render to rely only on currentPath and previewKey (or remove previewKey if unused) and remove any UI/logic that depended on the removed savedPaths or editor state.
226-232: No-op conditional: both branches callsetEditPath(!editPath).✏️ Proposed fix
const handleEditPath = () => { - if (activePresets) { - setEditPath(!editPath); - } else { - setEditPath(!editPath); - } + setEditPath(!editPath); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/view/svg-line-draw/index.tsx` around lines 226 - 232, The conditional in handleEditPath is a no-op because both branches call setEditPath(!editPath); remove the if/else and simplify handleEditPath to a single call to setEditPath(!editPath) (referencing the handleEditPath function and the editPath state/ setter; activePresets can be left checked elsewhere if needed).
🤖 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/`(tools)/layout.tsx:
- Around line 7-20: Toolslayout currently always renders ToolsHeader except for
"/svg-line-draw", causing duplicate headers when pages use ToolPlaygroundShell;
update Toolslayout to detect shell-backed routes (e.g., compute a boolean like
isToolPlaygroundRoute by matching pathname against the routes that render
ToolPlaygroundShell such as clip-paths, color-lab, shadows, background-snippets,
mesh-gradients) and change the header render to {!isLineDraw &&
!isToolPlaygroundRoute && <ToolsHeader />}; alternatively, if you prefer
consolidating, remove ToolsHeader from Toolslayout entirely and ensure
ToolPlaygroundShell provides the shared 64px header (move any unique ToolsHeader
content into ToolPlaygroundShell) so only one header component is rendered for
those routes.
In `@components/view/svg-line-draw/index.tsx`:
- Line 59: The import in index.tsx currently uses the misspelled symbol
compoentCode—rename the export in components/view/svg-line-draw/data.ts from
compoentCode to componentCode, update the import in
components/view/svg-line-draw/index.tsx to import componentCode, and update any
usage sites (notably the reference at the code block around line 579) to use
componentCode so the identifier is correct and consistent across data.ts and
index.tsx.
---
Outside diff comments:
In `@components/view/svg-line-draw/index.tsx`:
- Around line 158-164: The copyCode function calls navigator.clipboard.writeText
without awaiting or handling errors, causing the success toast to show even if
the copy fails; update copyCode to await
navigator.clipboard.writeText(generateCode()) (or use .then/.catch) and only
call toast.success after the promise resolves, and call toast.error (with a
helpful description) if the promise rejects; reference the copyCode function,
generateCode(), navigator.clipboard.writeText, toast.success and toast.error
when making the change.
- Line 32: Replace the raw Radix ScrollArea import with the project's composed
wrapper and remove the workaround overflow styles: change the import "import {
ScrollArea } from \"@radix-ui/react-scroll-area\";" to import the project's
ScrollArea (from "@/components/ui/scroll-area"), then update the component usage
that currently relies on the unstyled Radix root (the block around the class
"overflow-auto" and lines ~280-291) to use the wrapper's API/children (no manual
Viewport/Scrollbar or overflow-auto band-aid). Ensure any props passed to the
Radix root are compatible with the wrapper (adjust prop names if needed) and
delete the explicit "overflow-auto" class so the wrapper can manage scrolling
and custom scrollbars.
- Around line 137-141: The toast uses the wrong variant for a validation
failure: replace the call to toast.success when currentPath is falsy with a
non-success variant (e.g., toast.error or toast.warning) so the UI reflects an
error state; locate the early-return block that checks currentPath (inside the
save handler where the code reads "if (!currentPath) { toast.success(\"No path
to save\", ...); return; }") and change the toast invocation to toast.error (or
toast.warning) while keeping the same message and description.
- Around line 29-33: The file imports TabsTrigger from "@radix-ui/react-tabs"
while Tabs, TabsContent and TabsList are imported from the project's wrapper;
change the import so TabsTrigger is imported from the project's tabs module (the
same module that exports Tabs/TabsContent/TabsList) to ensure consistent styling
and behavior; update the import statement to pull TabsTrigger alongside Tabs,
TabsContent, TabsList (referencing the TabsTrigger symbol) and remove the direct
Radix import.
---
Nitpick comments:
In `@components/view/svg-line-draw/index.tsx`:
- Around line 367-370: The handler currently uses a blanket cast "as any" which
defeats type checking; change the cast to a narrowing cast using the
AnimationSettings index type, e.g. replace the cast in the Select onValueChange
for settings.strokeLinecap so it calls updateSetting("strokeLinecap", value as
AnimationSettings["strokeLinecap"]) instead of value as any, and apply the same
pattern for the other Select handlers flagged (the other onValueChange
occurrences at the same pattern around lines 463, 623, and 690) so each uses the
corresponding AnimationSettings["<propertyName>"] type when calling
updateSetting.
- Around line 178-186: Remove the large commented-out blocks that are residual
from the old playground layout (e.g., the commented openEditorForExample
function and other commented sections around the component), or if the
functionality is intended for future work, convert each block into a focused
TODO comment and create corresponding issue tickets instead; specifically,
delete the commented definition for openEditorForExample and any other commented
helper logic, or replace them with concise TODOs that reference the function
names (openEditorForExample, any commented state setters like
setCurrentPath/setEditorViewBox/setShowEditor) so future work is tracked in the
issue tracker rather than left as commented code.
- Around line 522-527: The ternary inside the JSX className call is a no-op
because both branches return "justify-between"; in the className={cn(...)}
expression remove the conditional and replace the ternary expression with the
static class "justify-between" (leave the surrounding cn call and other classes
intact); reference the existing variables customDrawLine and editPath only to
confirm they are no longer needed in this class expression within this
component's render.
- Around line 136-150: The savePath function contains a leftover debug
console.log(currentPath) that should be removed; locate the savePath function
and delete the console.log(currentPath) statement (leave the existing toast and
setShowSaveDialog/currentPath checks intact) so no development debug output
remains in production.
- Around line 96-100: Remove the dead React state and unreachable branches:
delete the unused savedPaths state and its setter (_setSavedPaths) and simplify
any conditional that checks savedPaths.length (make generateCode and the UI
always use the single-path flow via currentPath); also remove the unused
editor-related state variables (_showEditor, setShowEditor, _editorViewBox,
_setEditorViewBox) and the dead helper _openEditor so no dangling references
remain. Update references in generateCode and the component render to rely only
on currentPath and previewKey (or remove previewKey if unused) and remove any
UI/logic that depended on the removed savedPaths or editor state.
- Around line 226-232: The conditional in handleEditPath is a no-op because both
branches call setEditPath(!editPath); remove the if/else and simplify
handleEditPath to a single call to setEditPath(!editPath) (referencing the
handleEditPath function and the editPath state/ setter; activePresets can be
left checked elsewhere if needed).
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/(tools)/layout.tsxapp/(tools)/svg-line-draw/page.tsxcomponents/view/svg-line-draw/index.tsx
| import { CodePreview } from "./code-preview"; | ||
| import { CustomLineInput } from "./custom-line-input"; | ||
| import { compoentCode, examplesSvgPath } from "./data"; | ||
| import { compoentCode } from "./data"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the data file in the svg-line-draw directory and check for the typo
fd --type f 'data' --full-path 'components/view/svg-line-draw' --exec cat {}Repository: ui-layouts/ui-tools
Length of output: 23919
Fix typo: compoentCode → componentCode
Rename the misspelled export in ./data and update the corresponding import and usage at line 579. While the code runs without errors (both import and export are misspelled consistently), this is clearly unintentional.
✏️ Changes needed
In components/view/svg-line-draw/data.ts:
-export const compoentCode = `
+export const componentCode = `In components/view/svg-line-draw/index.tsx at line 59:
-import { compoentCode } from "./data";
+import { componentCode } from "./data";At line 579:
- text={compoentCode}
+ text={componentCode}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/view/svg-line-draw/index.tsx` at line 59, The import in index.tsx
currently uses the misspelled symbol compoentCode—rename the export in
components/view/svg-line-draw/data.ts from compoentCode to componentCode, update
the import in components/view/svg-line-draw/index.tsx to import componentCode,
and update any usage sites (notably the reference at the code block around line
579) to use componentCode so the identifier is correct and consistent across
data.ts and index.tsx.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
app/(tools)/layout.tsx (1)
9-9:⚠️ Potential issue | 🟠 MajorDuplicate headers on all non-svg-line-draw ToolPlaygroundShell routes remain unresolved.
isLineDrawonly suppressesToolsHeaderfor/svg-line-draw, but the five other routes that also wrap their content inToolPlaygroundShell(clip-paths, color-lab, shadows, background-snippets, mesh-gradients) still renderToolsHeaderon top of the shell's own built-in header, wasting ~80–128 px of viewport height on every one of those routes.Extend the guard to cover all shell-backed routes:
🐛 Proposed fix
- const isLineDraw = pathname === "/svg-line-draw"; + const SHELL_ROUTES = new Set([ + "/svg-line-draw", + "/clip-paths", + "/color-lab", + "/shadows", + "/background-snippets", + "/mesh-gradients", + ]); + const isShellRoute = SHELL_ROUTES.has(pathname); return ( <div className="h-screen overflow-hidden bg-white text-black dark:bg-black dark:text-white"> - {!isLineDraw && <ToolsHeader />} + {!isShellRoute && <ToolsHeader />} <main className={ - isLineDraw + isShellRoute ? "h-full overflow-hidden" : "h-full overflow-hidden px-3 pt-20 pb-3 xl:px-6" } >Also applies to: 13-13
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/`(tools)/layout.tsx at line 9, isLineDraw currently only suppresses ToolsHeader for "/svg-line-draw" but other routes that render inside ToolPlaygroundShell ("/clip-paths", "/color-lab", "/shadows", "/background-snippets", "/mesh-gradients") still show duplicate headers; update the guard in layout.tsx to compute a boolean like isToolPlaygroundRoute (e.g., check pathname equality or membership in an array of these routes or a startsWith pattern) and use that boolean instead of isLineDraw to conditionally render/suppress ToolsHeader when the page is hosted inside ToolPlaygroundShell (references: isLineDraw, pathname, ToolsHeader, ToolPlaygroundShell).components/view/svg-line-draw/index.tsx (1)
59-59: Fix thecompoentCodetypo tocomponentCode.Line 59 still imports the misspelled symbol and it’s used later, which reads accidental. Rename the export and update import/usage for clarity.
✏️ Suggested rename
-import { compoentCode } from "./data"; +import { componentCode } from "./data";- text={compoentCode} + text={componentCode}-export const compoentCode = ` +export const componentCode = `🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/view/svg-line-draw/index.tsx` at line 59, The import uses a misspelled symbol "compoentCode" — rename the exported identifier in the module that provides it to "componentCode" and update this file's import to import { componentCode } from "./data"; then replace all usages of compoentCode with componentCode (including any other files importing it) so the export/import and references match the corrected name.
🧹 Nitpick comments (1)
app/(tools)/layout.tsx (1)
1-4: Avoid making the entire layout a Client Component just forusePathname.Marking the layout
"use client"pulls the whole layout (and its static markup) into the client bundle. The Next.js App Router idiom is to keep layouts as Server Components and extract only the reactive slice into a tiny client component.♻️ Proposed refactor — extract a `ConditionalHeader` client component
Create
components/common/conditional-header.tsx:"use client"; import ToolsHeader from "@/components/common/tools-header"; import { usePathname } from "next/navigation"; const SHELL_ROUTES = new Set([ "/svg-line-draw", "/clip-paths", "/color-lab", "/shadows", "/background-snippets", "/mesh-gradients", ]); export function ConditionalHeader() { const pathname = usePathname(); if (SHELL_ROUTES.has(pathname)) return null; return <ToolsHeader />; }Then revert
layout.tsxto a Server Component:-"use client"; - import ToolsHeader from "@/components/common/tools-header"; -import { usePathname } from "next/navigation"; +import { ConditionalHeader } from "@/components/common/conditional-header"; import type React from "react"; function Toolslayout({ children }: { children: React.ReactNode }) { - const pathname = usePathname(); - const isLineDraw = pathname === "/svg-line-draw"; - return ( <div className="h-screen overflow-hidden bg-white text-black dark:bg-black dark:text-white"> - {!isLineDraw && <ToolsHeader />} - <main - className={ - isLineDraw - ? "h-full overflow-hidden" - : "h-full overflow-hidden px-3 pt-20 pb-3 xl:px-6" - } - > + <ConditionalHeader /> + <main className="h-full overflow-hidden"> {children} </main> </div> ); }Note: shell-backed pages already own their own padding/layout via
ToolPlaygroundShell, so a uniformh-full overflow-hiddenonmainis sufficient; individual pages handle their own internal padding.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/`(tools)/layout.tsx around lines 1 - 4, The layout is marked "use client" only to call usePathname, which unnecessarily ships the whole layout to the client; remove the "use client" directive from app/(tools)/layout.tsx and convert it back to a Server Component, then create a small client component (e.g., components/common/conditional-header.tsx) that is "use client", imports ToolsHeader and usePathname, defines SHELL_ROUTES (the given set of shell-backed paths) and returns null when pathname is in that set otherwise returns <ToolsHeader />; finally import and render ConditionalHeader from the layout in place of ToolsHeader (leave main's h-full overflow-hidden etc. as-is and let per-page shells handle internal padding).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/view/svg-line-draw/index.tsx`:
- Around line 243-246: The left panel wrapper that contains the Tabs (the div
with the long class string including "hidden h-full ... lg:block") is fully
hidden below lg, making the Tabs (defaultValue="presets") unreachable on small
screens; change the layout so the panel is accessible on mobile by either
removing the unconditional "hidden" class and using responsive visibility (e.g.,
show as block on small screens and keep lg layout as-is) or implement a
collapsible/drawer pattern: add a small-screen toggle button that controls a new
local state (e.g., showLeftPanel) and conditionally render the same wrapper/div
(and the Tabs component) as an overlay/drawer when showLeftPanel is true while
keeping the existing lg:block behavior for large screens; ensure the toggle
button is placed in the parent header/toolbar so users can open/close
presets/settings on mobile.
---
Duplicate comments:
In `@app/`(tools)/layout.tsx:
- Line 9: isLineDraw currently only suppresses ToolsHeader for "/svg-line-draw"
but other routes that render inside ToolPlaygroundShell ("/clip-paths",
"/color-lab", "/shadows", "/background-snippets", "/mesh-gradients") still show
duplicate headers; update the guard in layout.tsx to compute a boolean like
isToolPlaygroundRoute (e.g., check pathname equality or membership in an array
of these routes or a startsWith pattern) and use that boolean instead of
isLineDraw to conditionally render/suppress ToolsHeader when the page is hosted
inside ToolPlaygroundShell (references: isLineDraw, pathname, ToolsHeader,
ToolPlaygroundShell).
In `@components/view/svg-line-draw/index.tsx`:
- Line 59: The import uses a misspelled symbol "compoentCode" — rename the
exported identifier in the module that provides it to "componentCode" and update
this file's import to import { componentCode } from "./data"; then replace all
usages of compoentCode with componentCode (including any other files importing
it) so the export/import and references match the corrected name.
---
Nitpick comments:
In `@app/`(tools)/layout.tsx:
- Around line 1-4: The layout is marked "use client" only to call usePathname,
which unnecessarily ships the whole layout to the client; remove the "use
client" directive from app/(tools)/layout.tsx and convert it back to a Server
Component, then create a small client component (e.g.,
components/common/conditional-header.tsx) that is "use client", imports
ToolsHeader and usePathname, defines SHELL_ROUTES (the given set of shell-backed
paths) and returns null when pathname is in that set otherwise returns
<ToolsHeader />; finally import and render ConditionalHeader from the layout in
place of ToolsHeader (leave main's h-full overflow-hidden etc. as-is and let
per-page shells handle internal padding).
There was a problem hiding this comment.
♻️ Duplicate comments (2)
components/view/svg-line-draw/index.tsx (2)
59-59:⚠️ Potential issue | 🟡 MinorRename
compoentCode→componentCodeto avoid perpetuating the typo.The misspelling is consistent but confusing. Please rename the export in
components/view/svg-line-draw/data.tsand update the import/usage here to match.✏️ Proposed fix in this file
-import { compoentCode } from "./data"; +import { componentCode } from "./data"; @@ - text={compoentCode} + text={componentCode}✏️ Also update the export in data.ts
-export const compoentCode = ` +export const componentCode = `Also applies to: 583-586
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/view/svg-line-draw/index.tsx` at line 59, The import name compoentCode is misspelled; rename the exported identifier in the data module from compoentCode to componentCode, then update this file's import to import { componentCode } and replace all usages of compoentCode (including the occurrences around lines 583-586) with componentCode so the import/export names match. Ensure the data module's export statement and any re-exports are updated to the new name and run a quick type-check to catch any remaining references to the old identifier.
239-241:⚠️ Potential issue | 🟠 MajorLeft panel is hidden below
lg, making presets/settings unreachable on mobile.The
hidden ... lg:blockwrapper removes all tabs on small screens. Please add a small-screen drawer/toggle or make the panel accessible belowlg.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/view/svg-line-draw/index.tsx` around lines 239 - 241, The left panel is fully hidden below lg due to the "hidden ... lg:block" classes in the className on the panel element; make it accessible on small screens by replacing that static hide with a responsive toggle/drawer: add a boolean state (e.g., isLeftPanelOpen) and a small-screen toggle button (visible <lg) that sets it, change the panel's class logic around the className in components/view/svg-line-draw/index.tsx to conditionally apply "hidden" only when isLeftPanelOpen is false (e.g., use cn(isLeftPanelOpen ? 'block' : 'hidden', 'lg:block', ...existing classes)), and implement keyboard/ARIA attributes (aria-expanded, role="dialog" or region) so the drawer is accessible; ensure the existing lg:block behavior remains unchanged for large screens.
🧹 Nitpick comments (1)
components/view/svg-line-draw/index.tsx (1)
492-497: Avoid double re-render by removing extrasetPreviewKeycalls.
updateSettingalready incrementspreviewKey, so the additionalsetPreviewKeycauses an extra render.♻️ Proposed fix
onSelectPath={(path, viewBox) => { setCurrentPath(path); setCustomDrawLine(false); updateSetting("viewBox", viewBox); - setPreviewKey((prev) => prev + 1); }}onSelectPath={(path, viewBox) => { setCurrentPath(path); setCustomDrawLine(false); updateSetting("viewBox", viewBox); - setPreviewKey((prev) => prev + 1); }}Also applies to: 506-511
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/view/svg-line-draw/index.tsx` around lines 492 - 497, The onSelectPath handler (and the similar handler at the other location) is causing a double re-render because updateSetting("viewBox", viewBox) already increments previewKey; remove the extra setPreviewKey((prev) => prev + 1) call and leave the rest (setCurrentPath, setCustomDrawLine, updateSetting) intact so the previewKey is only incremented once by updateSetting.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@components/view/svg-line-draw/index.tsx`:
- Line 59: The import name compoentCode is misspelled; rename the exported
identifier in the data module from compoentCode to componentCode, then update
this file's import to import { componentCode } and replace all usages of
compoentCode (including the occurrences around lines 583-586) with componentCode
so the import/export names match. Ensure the data module's export statement and
any re-exports are updated to the new name and run a quick type-check to catch
any remaining references to the old identifier.
- Around line 239-241: The left panel is fully hidden below lg due to the
"hidden ... lg:block" classes in the className on the panel element; make it
accessible on small screens by replacing that static hide with a responsive
toggle/drawer: add a boolean state (e.g., isLeftPanelOpen) and a small-screen
toggle button (visible <lg) that sets it, change the panel's class logic around
the className in components/view/svg-line-draw/index.tsx to conditionally apply
"hidden" only when isLeftPanelOpen is false (e.g., use cn(isLeftPanelOpen ?
'block' : 'hidden', 'lg:block', ...existing classes)), and implement
keyboard/ARIA attributes (aria-expanded, role="dialog" or region) so the drawer
is accessible; ensure the existing lg:block behavior remains unchanged for large
screens.
---
Nitpick comments:
In `@components/view/svg-line-draw/index.tsx`:
- Around line 492-497: The onSelectPath handler (and the similar handler at the
other location) is causing a double re-render because updateSetting("viewBox",
viewBox) already increments previewKey; remove the extra setPreviewKey((prev) =>
prev + 1) call and leave the rest (setCurrentPath, setCustomDrawLine,
updateSetting) intact so the previewKey is only incremented once by
updateSetting.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
components/view/svg-line-draw/index.tsx (2)
59-59: RenamecompoentCodetocomponentCodefor clarity.The misspelled identifier is still present at import and usage. Please align the export/import/usage to the corrected spelling (also in
components/view/svg-line-draw/data.ts).✏️ Suggested fix (this file)
-import { compoentCode } from "./data"; +import { componentCode } from "./data"; @@ - text={compoentCode} + text={componentCode}Also applies to: 627-630
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/view/svg-line-draw/index.tsx` at line 59, The import uses a misspelled identifier "compoentCode" — rename the export and all usages to "componentCode": update the import in this file to import { componentCode } from "./data", and change the corresponding export name in components/view/svg-line-draw/data.ts and any references inside this file (search for "compoentCode") to the corrected "componentCode" to keep export/import names consistent.
276-321:⚠️ Potential issue | 🟠 MajorSidebar tabs are still unreachable on small screens.
The left panel is
hiddenbelowlg, so presets/settings/edited/saved remain inaccessible on mobile. Consider making the panel visible on small screens or providing a drawer/toggle.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/view/svg-line-draw/index.tsx` around lines 276 - 321, Sidebar panel is currently hidden on small screens because the container class string includes "hidden ... lg:block", making TabsList/TabsTrigger unreachable on mobile; either remove the "hidden" (or replace it with responsive visibility like "block lg:block") so the existing container renders on small screens, or implement a mobile toggle/drawer: add a state flag (e.g., isSidebarOpen) and a button to toggle it, and render the same panel (the element with the class string and the Tabs/TabsList/TabsTrigger children) as an overlay drawer (fixed inset and z-index classes) when isSidebarOpen is true so presets/settings/edited/saved are accessible on mobile. Ensure you update the class string referenced above (the container around Tabs) and wire the toggle to open/close the panel on small screens.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/view/svg-line-draw/index.tsx`:
- Around line 240-265: The sidebar buttons use single-letter labels
("P","S","E","V") which are not accessible; update the button render in the map
that calls setActiveSidebarTab and reads activeSidebarTab to include accessible
names—add an aria-label and title with full words (e.g., "Presets", "Settings",
"Edited", "Saved") or include an sr-only text node for each item so screen
readers get the full label while keeping the visual single-letter UI; ensure
aria-label/title values are derived from the item.label mapping or add a new
label property in the array and pass it to each button.
---
Duplicate comments:
In `@components/view/svg-line-draw/index.tsx`:
- Line 59: The import uses a misspelled identifier "compoentCode" — rename the
export and all usages to "componentCode": update the import in this file to
import { componentCode } from "./data", and change the corresponding export name
in components/view/svg-line-draw/data.ts and any references inside this file
(search for "compoentCode") to the corrected "componentCode" to keep
export/import names consistent.
- Around line 276-321: Sidebar panel is currently hidden on small screens
because the container class string includes "hidden ... lg:block", making
TabsList/TabsTrigger unreachable on mobile; either remove the "hidden" (or
replace it with responsive visibility like "block lg:block") so the existing
container renders on small screens, or implement a mobile toggle/drawer: add a
state flag (e.g., isSidebarOpen) and a button to toggle it, and render the same
panel (the element with the class string and the Tabs/TabsList/TabsTrigger
children) as an overlay drawer (fixed inset and z-index classes) when
isSidebarOpen is true so presets/settings/edited/saved are accessible on mobile.
Ensure you update the class string referenced above (the container around Tabs)
and wire the toggle to open/close the panel on small screens.
naymurdev
force-pushed
the
codex/build-developer-first-tool-playground-ui
branch
from
35282cd to
3b152b6
Compare
❌ Deploy Preview for stellar-cendol-140753 failed. Why did it fail? →
|
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.
Motivation
100vh) and a single scrolling area to avoid nested/double scrollbars and keep previews non-scrolling.Description
ToolPlaygroundShellcomponent that implements the required 3-part shell: fixed top bar (title/description,Playground | Examples | Docstabs, primary actions), single scrollable left sidebar (presets, controls, output options), non-scrolling preview canvas area, and a viewport-fixed export drawer with copy/download actions (components/common/tool-playground-shell.tsx).h-screenand hiding page-level scroll (app/(tools)/layout.tsx).SVG Line Draw,Shadows Lab,Clip-path Generator,Mesh Gradient Generator,Background Snippets Generator, andColor Lab(files updated underapp/(tools)/*/page.tsx).exportCodeprop from each wrapper so tools remain fully mounted and usable; the shell is ready to be wired to each tool’s live state later.Testing
npm run lint, which reported existing repository-wide Biome/formatting diagnostics and style suggestions for updated files (lint produced errors/warnings; these are repo-level issues not introduced by the shell changes).npm run build, which failed in this environment due to an external Google Fonts fetch error forSpace Grotesk(font download blocked), preventing a full production build verification.npm run devand validated the new shell locally in development; the server started and the/shadowsroute rendered successfully and a screenshot was captured for verification (dev run succeeded).Codex Task
Summary by CodeRabbit
New Features
UI/UX Improvements
Refactor