Simulating Onlook -> Agent implementation #3049
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
WalkthroughThis PR adds a context menu to the Canvas component with sidebar tab switching capabilities and refactors the left panel's UI state management. The Canvas now accepts an Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Main
participant Canvas
participant LeftPanel
participant DesignPanel
User->>Canvas: Right-click (context menu)
Canvas->>Canvas: Show context menu
User->>Canvas: Click menu item (e.g., "Layers")
Canvas->>Main: onSidebarClick("Layers")
Main->>Main: handleSidebarClick("Layers")<br/>Set activeSection state
Main->>LeftPanel: Pass onClose & activeSection props
Main->>DesignPanel: Receives activeSection
DesignPanel->>DesignPanel: Derive selectedTab from activeSection<br/>Render header + content
DesignPanel->>User: Display Layers tab
User->>DesignPanel: Click close button
DesignPanel->>LeftPanel: onClose()
LeftPanel->>Main: handleClosePanel()
Main->>Main: Clear activeSection state
Main->>DesignPanel: activeSection = null
DesignPanel->>User: Panel closes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ 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 |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
| export const DesignPanel = observer(() => { | ||
| interface DesignPanelProps { | ||
| onClose?: () => void; | ||
| activeSection?: string | null; |
There was a problem hiding this comment.
Refine the type of the activeSection prop to use LeftPanelTabValue | null instead of string | null for better type-safety.
| activeSection?: string | null; | |
| activeSection?: LeftPanelTabValue | null; |
| leftPanelRef, | ||
| rightPanelRef, | ||
| ); | ||
| const [activeSection, setActiveSection] = useState<string | null>(null); |
There was a problem hiding this comment.
Consider typing the activeSection state as LeftPanelTabValue | null instead of string | null for improved type consistency.
| const [activeSection, setActiveSection] = useState<string | null>(null); | |
| const [activeSection, setActiveSection] = useState<LeftPanelTabValue | null>(null); |
![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
🧹 Nitpick comments (3)
apps/web/client/src/app/project/[id]/_components/left-panel/design-panel/index.tsx (1)
144-169: Consider refactoring repetitive icon and label rendering.Lines 146-157 use repetitive conditional rendering for icons and labels. This could be simplified by leveraging the existing
tabsarray defined at the top of the file.+const selectedTabConfig = tabs.find(tab => tab.value === selectedTab); + <div className="flex items-center gap-2"> - {selectedTab === LeftPanelTabValue.LAYERS && <Icons.Layers className="w-4 h-4" />} - {selectedTab === LeftPanelTabValue.BRAND && <Icons.Brand className="w-4 h-4" />} - {selectedTab === LeftPanelTabValue.PAGES && <Icons.File className="w-4 h-4" />} - {selectedTab === LeftPanelTabValue.IMAGES && <Icons.Image className="w-4 h-4" />} - {selectedTab === LeftPanelTabValue.BRANCHES && <Icons.Branch className="w-4 h-4" />} + {selectedTabConfig && React.cloneElement(selectedTabConfig.icon as React.ReactElement, { className: "w-4 h-4" })} <span className="text-sm font-medium"> - {selectedTab === LeftPanelTabValue.LAYERS && t(transKeys.editor.panels.layers.tabs.layers)} - {selectedTab === LeftPanelTabValue.BRAND && t(transKeys.editor.panels.layers.tabs.brand)} - {selectedTab === LeftPanelTabValue.PAGES && t(transKeys.editor.panels.layers.tabs.pages)} - {selectedTab === LeftPanelTabValue.IMAGES && t(transKeys.editor.panels.layers.tabs.images)} - {selectedTab === LeftPanelTabValue.BRANCHES && t(transKeys.editor.panels.layers.tabs.branches)} + {selectedTabConfig && t(selectedTabConfig.label)} </span> </div>apps/web/client/src/app/project/[id]/_components/main.tsx (2)
34-46: Consider consolidating dual state management.The component maintains
activeSection(local state) andeditorEngine.state.leftPanelTab(MobX store) in parallel. Both are updated together inhandleSidebarClickandhandleClosePanel, which could lead to synchronization issues if one state is modified independently elsewhere.Consider one of these approaches:
Option 1: Single source of truth (prefer MobX store)
Remove local state and derive everything fromeditorEngine.state.leftPanelTab:-const [activeSection, setActiveSection] = useState<string | null>(null); - -const handleSidebarClick = (tab: LeftPanelTabValue) => { - editorEngine.state.leftPanelTab = tab; - editorEngine.state.leftPanelLocked = true; - setActiveSection(tab); -}; +const handleSidebarClick = (tab: LeftPanelTabValue) => { + editorEngine.state.leftPanelTab = tab; + editorEngine.state.leftPanelLocked = true; +}; -const handleClosePanel = () => { - editorEngine.state.leftPanelTab = null; - editorEngine.state.leftPanelLocked = false; - setActiveSection(null); -}; +const handleClosePanel = () => { + editorEngine.state.leftPanelTab = null; + editorEngine.state.leftPanelLocked = false; +}; -<LeftPanel onClose={handleClosePanel} activeSection={activeSection} /> +<LeftPanel onClose={handleClosePanel} activeSection={editorEngine.state.leftPanelTab} />Option 2: Use a ref for coordination
If local state is needed for render optimization, document the synchronization contract clearly.
107-107: Document the layout position change.The left panel position changed from
left-0toleft-2. This appears intentional but lacks documentation about why the 2px offset is needed.Consider adding a comment explaining the spacing:
<div ref={leftPanelRef} - className="absolute top-10 left-2 h-[calc(100%-40px)] z-50" + className="absolute top-10 left-2 h-[calc(100%-40px)] z-50" // 2px offset for visual spacing from edge >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/web/client/src/app/project/[id]/_components/canvas/index.tsx(3 hunks)apps/web/client/src/app/project/[id]/_components/left-panel/design-panel/index.tsx(4 hunks)apps/web/client/src/app/project/[id]/_components/left-panel/index.tsx(1 hunks)apps/web/client/src/app/project/[id]/_components/main.tsx(4 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
apps/web/client/src/app/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/app/**/*.tsx: Default to Server Components; add 'use client' when using events, state/effects, browser APIs, or client‑only libraries
Do not use process.env in client code; import env from @/env insteadAvoid hardcoded user-facing text; use next-intl messages/hooks
Files:
apps/web/client/src/app/project/[id]/_components/main.tsxapps/web/client/src/app/project/[id]/_components/left-panel/design-panel/index.tsxapps/web/client/src/app/project/[id]/_components/left-panel/index.tsxapps/web/client/src/app/project/[id]/_components/canvas/index.tsx
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks insteadUse path aliases @/* and ~/* for imports mapping to src/*
Files:
apps/web/client/src/app/project/[id]/_components/main.tsxapps/web/client/src/app/project/[id]/_components/left-panel/design-panel/index.tsxapps/web/client/src/app/project/[id]/_components/left-panel/index.tsxapps/web/client/src/app/project/[id]/_components/canvas/index.tsx
apps/web/client/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable references across renders
Keep the active MobX store in a useRef and perform async cleanup with setTimeout(() => storeRef.current?.clear(), 0) to avoid route-change races
Avoid useMemo for creating MobX store instances
Avoid putting the MobX store instance in effect dependency arrays if it causes loops; split concerns by domain
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable identities across renders
Keep the active MobX store in a useRef and clean up asynchronously with setTimeout(() => storeRef.current?.clear(), 0)
Do not use useMemo to create MobX stores
Avoid placing MobX store instances in effect dependency arrays if it causes loops; split concerns instead
observer components must be client components; place a single client boundary at the feature entry; child observers need not repeat 'use client'
Files:
apps/web/client/src/app/project/[id]/_components/main.tsxapps/web/client/src/app/project/[id]/_components/left-panel/design-panel/index.tsxapps/web/client/src/app/project/[id]/_components/left-panel/index.tsxapps/web/client/src/app/project/[id]/_components/canvas/index.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
apps/web/client/src/app/project/[id]/_components/main.tsxapps/web/client/src/app/project/[id]/_components/left-panel/design-panel/index.tsxapps/web/client/src/app/project/[id]/_components/left-panel/index.tsxapps/web/client/src/app/project/[id]/_components/canvas/index.tsx
apps/web/client/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Default to Server Components; add 'use client' only when using events, state/effects, browser APIs, or client-only libs
Files:
apps/web/client/src/app/project/[id]/_components/main.tsxapps/web/client/src/app/project/[id]/_components/left-panel/design-panel/index.tsxapps/web/client/src/app/project/[id]/_components/left-panel/index.tsxapps/web/client/src/app/project/[id]/_components/canvas/index.tsx
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
apps/web/client/src/app/project/[id]/_components/main.tsxapps/web/client/src/app/project/[id]/_components/left-panel/design-panel/index.tsxapps/web/client/src/app/project/[id]/_components/left-panel/index.tsxapps/web/client/src/app/project/[id]/_components/canvas/index.tsx
🧠 Learnings (2)
📚 Learning: 2025-09-14T01:44:21.209Z
Learnt from: CR
Repo: onlook-dev/onlook PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T01:44:21.209Z
Learning: Applies to apps/web/client/src/app/**/*.tsx : Default to Server Components; add 'use client' when using events, state/effects, browser APIs, or client‑only libraries
Applied to files:
apps/web/client/src/app/project/[id]/_components/main.tsx
📚 Learning: 2025-09-16T19:22:52.461Z
Learnt from: CR
Repo: onlook-dev/onlook PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-16T19:22:52.461Z
Learning: Applies to apps/web/client/src/app/**/*.{ts,tsx} : Default to Server Components; add 'use client' only when using events, state/effects, browser APIs, or client-only libs
Applied to files:
apps/web/client/src/app/project/[id]/_components/main.tsx
🧬 Code graph analysis (4)
apps/web/client/src/app/project/[id]/_components/main.tsx (2)
apps/web/client/src/app/project/[id]/_components/canvas/index.tsx (1)
Canvas(41-413)apps/web/client/src/app/project/[id]/_components/left-panel/index.tsx (1)
LeftPanel(13-23)
apps/web/client/src/app/project/[id]/_components/left-panel/design-panel/index.tsx (5)
apps/web/client/src/components/store/editor/index.tsx (1)
useEditorEngine(10-14)apps/web/client/src/app/project/[id]/_components/left-panel/design-panel/brand-tab/index.tsx (1)
BrandTab(21-125)apps/web/client/src/app/project/[id]/_components/left-panel/design-panel/page-tab/index.tsx (1)
PagesTab(15-211)apps/web/client/src/app/project/[id]/_components/left-panel/design-panel/image-tab/index.tsx (1)
ImagesTab(19-158)apps/web/client/src/app/project/[id]/_components/left-panel/design-panel/branches-tab/index.tsx (1)
BranchesTab(12-112)
apps/web/client/src/app/project/[id]/_components/left-panel/index.tsx (2)
apps/web/client/src/components/store/editor/index.tsx (1)
useEditorEngine(10-14)apps/web/client/src/app/project/[id]/_components/left-panel/design-panel/index.tsx (1)
DesignPanel(56-186)
apps/web/client/src/app/project/[id]/_components/canvas/index.tsx (7)
packages/ui/src/components/context-menu.tsx (8)
ContextMenu(212-212)ContextMenuTrigger(226-226)ContextMenuContent(214-214)ContextMenuItem(216-216)ContextMenuSeparator(221-221)ContextMenuSub(223-223)ContextMenuSubTrigger(225-225)ContextMenuSubContent(224-224)apps/web/client/src/app/project/[id]/_components/canvas/frames.tsx (1)
Frames(8-23)apps/web/client/src/app/project/[id]/_components/canvas/recenter-canvas-button.tsx (1)
RecenterCanvasButton(9-31)apps/web/client/src/app/project/[id]/_components/canvas/overlay/drag-select.tsx (1)
DragSelectOverlay(14-37)apps/web/client/src/app/project/[id]/_components/canvas/overlay/index.tsx (1)
Overlay(16-73)apps/web/client/src/app/project/[id]/_components/canvas/overlay/pan.tsx (1)
PanOverlay(10-49)packages/ui/src/components/icons/index.tsx (1)
Icons(138-3667)
🔇 Additional comments (3)
apps/web/client/src/app/project/[id]/_components/left-panel/index.tsx (1)
8-17: LGTM! Clean refactor to support external state management.The addition of
onCloseandactiveSectionprops properly enables parent-controlled state management of the left panel, which aligns well with the new context menu-driven workflow.apps/web/client/src/app/project/[id]/_components/canvas/index.tsx (1)
382-389: Verify menu structure matches PR objectives.According to the PR objectives, "Branches" should be inside the "Panels" submenu with a separator, but line 383-389 places it as a top-level menu item after a separator. This creates a different menu structure than specified.
Expected structure (from PR objectives):
- Panels submenu → Layers, Brand, Pages, Images, Separator, Branches, Separator
Current structure:
- Panels submenu → Layers, Brand, Pages, Images
- (Separator)
- Branches (top-level)
Please confirm whether this is intentional or if Branches should be moved inside the Panels submenu:
<ContextMenuSubContent> <ContextMenuItem onClick={() => { onSidebarClick?.(LeftPanelTabValue.LAYERS); }}> Layers </ContextMenuItem> <ContextMenuItem onClick={() => { onSidebarClick?.(LeftPanelTabValue.BRAND); }}> Brand </ContextMenuItem> <ContextMenuItem onClick={() => { onSidebarClick?.(LeftPanelTabValue.PAGES); }}> Pages </ContextMenuItem> <ContextMenuItem onClick={() => { onSidebarClick?.(LeftPanelTabValue.IMAGES); }}> Images </ContextMenuItem> + <ContextMenuSeparator /> + <ContextMenuItem onClick={() => { onSidebarClick?.(LeftPanelTabValue.BRANCHES); }}> + Branches + </ContextMenuItem> + <ContextMenuSeparator /> </ContextMenuSubContent> -<ContextMenuSeparator /> -<ContextMenuItem onClick={() => { onSidebarClick?.(LeftPanelTabValue.BRANCHES); }}> - Branches -</ContextMenuItem> -<ContextMenuSeparator />apps/web/client/src/app/project/[id]/_components/left-panel/design-panel/index.tsx (1)
51-60: The review comment concerns are unfounded—the cast is runtime-safe due to controlled data flow.The
activeSectionprop cannot contain invalid values. It is only set by two code paths: (1)setActiveSection(tab)wheretabhas typeLeftPanelTabValue, and (2)setActiveSection(null). The Canvas component only invokesonSidebarClickwith explicit enum values (PAGES, LAYERS, COMPONENTS, IMAGES, WINDOWS, BRAND, BRANCHES, APPS). No external code can introduce arbitrary strings, so the cast at line 60 is type-safe at runtime. The suggested runtime validation is unnecessary.A minor improvement would be to tighten the prop type from
string | nulltoLeftPanelTabValue | nullto eliminate the cast entirely, but this is a type-level refinement, not a correctness issue.Likely an incorrect or invalid review comment.
| <ContextMenuContent className="w-64 bg-background/95 backdrop-blur-lg"> | ||
| <ContextMenuItem | ||
| onClick={() => { | ||
| // Add Element action | ||
| }} | ||
| > | ||
| <Icons.Plus className="mr-2 h-4 w-4" /> | ||
| Add Element | ||
| </ContextMenuItem> | ||
| <ContextMenuItem | ||
| onClick={() => { | ||
| // Add Component action | ||
| }} | ||
| > | ||
| <Icons.ComponentInstance className="mr-2 h-4 w-4" /> | ||
| Add Component | ||
| </ContextMenuItem> | ||
| <ContextMenuSeparator /> | ||
| <ContextMenuSub> | ||
| <ContextMenuSubTrigger> | ||
| <Icons.ViewGrid className="mr-2 h-4 w-4" /> | ||
| Panels | ||
| </ContextMenuSubTrigger> | ||
| <ContextMenuSubContent> | ||
| <ContextMenuItem | ||
| onClick={() => { | ||
| onSidebarClick?.(LeftPanelTabValue.LAYERS); | ||
| }} | ||
| > | ||
| Layers | ||
| </ContextMenuItem> | ||
| <ContextMenuItem | ||
| onClick={() => { | ||
| onSidebarClick?.(LeftPanelTabValue.BRAND); | ||
| }} | ||
| > | ||
| Brand | ||
| </ContextMenuItem> | ||
| <ContextMenuItem | ||
| onClick={() => { | ||
| onSidebarClick?.(LeftPanelTabValue.PAGES); | ||
| }} | ||
| > | ||
| Pages | ||
| </ContextMenuItem> | ||
| <ContextMenuItem | ||
| onClick={() => { | ||
| onSidebarClick?.(LeftPanelTabValue.IMAGES); | ||
| }} | ||
| > | ||
| Images | ||
| </ContextMenuItem> | ||
| </ContextMenuSubContent> | ||
| </ContextMenuSub> | ||
| <ContextMenuSeparator /> | ||
| <ContextMenuItem | ||
| onClick={() => { | ||
| onSidebarClick?.(LeftPanelTabValue.BRANCHES); | ||
| }} | ||
| > | ||
| Branches | ||
| </ContextMenuItem> | ||
| <ContextMenuSeparator /> | ||
| <ContextMenuItem | ||
| onClick={() => { | ||
| editorEngine.copy.copy(); | ||
| }} | ||
| > | ||
| <Icons.Clipboard className="mr-2 h-4 w-4" /> | ||
| Copy | ||
| <span className="ml-auto text-xs text-muted-foreground">⌘ C</span> | ||
| </ContextMenuItem> | ||
| <ContextMenuItem | ||
| onClick={() => { | ||
| editorEngine.copy.paste(); | ||
| }} | ||
| > | ||
| <Icons.ClipboardCopy className="mr-2 h-4 w-4" /> | ||
| Paste | ||
| <span className="ml-auto text-xs text-muted-foreground">⌘ V</span> | ||
| </ContextMenuItem> |
There was a problem hiding this comment.
Internationalize hardcoded menu text.
Lines 335, 343, 349, 357, 364, 371, 378, 388, 397, and 406 contain hardcoded user-facing text ("Add Element", "Add Component", "Panels", "Layers", "Brand", "Pages", "Images", "Branches", "Copy", "Paste"). This violates the coding guideline to use next-intl messages/hooks instead of hardcoded text.
Based on coding guidelines.
The DesignPanel component (apps/web/client/src/app/project/[id]/_components/left-panel/design-panel/index.tsx) already uses useTranslations() and translation keys from transKeys.editor.panels.layers.tabs.*. Apply a similar pattern here:
+import { useTranslations } from 'next-intl';
+import { transKeys } from '@/i18n/keys';
+
export const Canvas = observer(({ onSidebarClick }: CanvasProps) => {
const editorEngine = useEditorEngine();
+ const t = useTranslations();
// ... rest of component
<ContextMenuItem onClick={() => { /* ... */ }}>
<Icons.Plus className="mr-2 h-4 w-4" />
- Add Element
+ {t(transKeys.editor.contextMenu.addElement)}
</ContextMenuItem>Ensure corresponding translation keys exist in your i18n configuration.
Committable suggestion skipped: line range outside the PR's diff.
| <ContextMenuItem | ||
| onClick={() => { | ||
| // Add Element action | ||
| }} | ||
| > | ||
| <Icons.Plus className="mr-2 h-4 w-4" /> | ||
| Add Element | ||
| </ContextMenuItem> | ||
| <ContextMenuItem | ||
| onClick={() => { | ||
| // Add Component action | ||
| }} | ||
| > | ||
| <Icons.ComponentInstance className="mr-2 h-4 w-4" /> | ||
| Add Component | ||
| </ContextMenuItem> |
There was a problem hiding this comment.
Implement missing onClick handlers for Add Element and Add Component.
The context menu items for "Add Element" and "Add Component" have empty onClick handlers. These actions are non-functional and will confuse users if the menu is accessible.
Either:
- Implement the handlers if the functionality exists
- Remove these menu items until they're ready
- Disable them with a "Coming soon" indicator
Do you want me to help search for existing add element/component functionality that could be wired up?
🤖 Prompt for AI Agents
In apps/web/client/src/app/project/[id]/_components/canvas/index.tsx around
lines 329 to 344, the ContextMenuItem onClick handlers for "Add Element" and
"Add Component" are empty; either wire them to the existing
addElement/addComponent functions (import or access from the canvas/project
context or hooks and call them with the current selection/parent), or remove
these menu items; if the features are planned but not implemented, set the items
to disabled and show a "Coming soon" label or tooltip so users aren’t presented
with inert controls.
| <ContextMenuItem | ||
| onClick={() => { | ||
| editorEngine.copy.copy(); | ||
| }} | ||
| > | ||
| <Icons.Clipboard className="mr-2 h-4 w-4" /> | ||
| Copy | ||
| <span className="ml-auto text-xs text-muted-foreground">⌘ C</span> | ||
| </ContextMenuItem> | ||
| <ContextMenuItem | ||
| onClick={() => { | ||
| editorEngine.copy.paste(); | ||
| }} | ||
| > | ||
| <Icons.ClipboardCopy className="mr-2 h-4 w-4" /> | ||
| Paste | ||
| <span className="ml-auto text-xs text-muted-foreground">⌘ V</span> | ||
| </ContextMenuItem> |
There was a problem hiding this comment.
🧩 Analysis chain
Verify keyboard shortcut labels match actual hotkeys.
The context menu displays "⌘ C" and "⌘ V" for Copy and Paste operations, but these might not reflect the actual keyboard shortcuts registered in the HotkeysArea or elsewhere in the application.
Verify that the displayed shortcuts match the actual hotkey bindings:
Additionally, consider making shortcuts cross-platform:
- macOS:
⌘ C/⌘ V - Windows/Linux:
Ctrl+C/Ctrl+V
🏁 Script executed:
#!/bin/bash
# Description: Search for keyboard shortcut definitions for copy/paste
echo "=== Searching for copy hotkey definitions ==="
rg -n "copy|Copy" --type=ts --type=tsx -B 2 -A 2 | rg -i "cmd|meta|ctrl|key"
echo -e "\n=== Searching for paste hotkey definitions ==="
rg -n "paste|Paste" --type=ts --type=tsx -B 2 -A 2 | rg -i "cmd|meta|ctrl|key"
echo -e "\n=== Searching in HotkeysArea component ==="
fd -e ts -e tsx "hotkey" --exec cat {}Length of output: 4228
🏁 Script executed:
#!/bin/bash
# Search for context menu usage of Copy/Paste with keyboard shortcuts in canvas component
echo "=== Finding canvas component file ==="
fd -type f -path "*canvas/index.tsx"
echo -e "\n=== Showing lines 391-408 of canvas component ==="
fd -type f -path "*canvas/index.tsx" -exec sed -n '391,408p' {} \;
echo -e "\n=== Broader context: lines 380-420 to understand the component ==="
fd -type f -path "*canvas/index.tsx" -exec sed -n '380,420p' {} \;
echo -e "\n=== Search for Hotkey imports in canvas component ==="
fd -type f -path "*canvas/index.tsx" -exec grep -n "Hotkey\|import.*hotkey" {} \;Length of output: 895
🏁 Script executed:
#!/bin/bash
# Search for canvas component with correct fd syntax
echo "=== Finding canvas component file ==="
fd canvas/index.tsx
echo -e "\n=== Showing lines 391-408 of canvas component ==="
fd canvas/index.tsx -exec sed -n '391,408p' {} \;
echo -e "\n=== Broader context: lines 380-420 ==="
fd canvas/index.tsx -exec sed -n '380,420p' {} \;
echo -e "\n=== Search for Hotkey imports in canvas component ==="
fd canvas/index.tsx -exec grep -n "Hotkey\|import.*hotkey" {} \;
echo -e "\n=== Search for hardcoded keyboard shortcuts ==="
fd canvas/index.tsx -exec grep -n "⌘\|text-muted-foreground" {} \;Length of output: 1156
🏁 Script executed:
#!/bin/bash
# Search for the canvas component more broadly
echo "=== Finding all index.tsx files in canvas directories ==="
find . -type f -path "*canvas/index.tsx" 2>/dev/null
echo -e "\n=== List recent .tsx files ==="
find . -type f -name "index.tsx" -path "*canvas*" 2>/dev/null
echo -e "\n=== Search for files containing Copy/Paste context menu ==="
rg -l "ContextMenuItem.*[Cc]opy|ContextMenuItem.*[Pp]aste" -g "*.tsx" 2>/dev/null | head -5
echo -e "\n=== Look for the exact file from the review path ==="
find . -type f -path "*project*\[id\]*canvas*index.tsx" 2>/dev/nullLength of output: 1210
🏁 Script executed:
#!/bin/bash
# Extract the exact lines from the canvas component
echo "=== Lines 391-408 from canvas/index.tsx ==="
sed -n '391,408p' ./apps/web/client/src/app/project/[id]/_components/canvas/index.tsx
echo -e "\n=== Broader context: lines 380-420 ==="
sed -n '380,420p' ./apps/web/client/src/app/project/[id]/_components/canvas/index.tsx
echo -e "\n=== Check for Hotkey imports in canvas/index.tsx ==="
grep -n "import.*Hotkey\|from.*hotkey" ./apps/web/client/src/app/project/[id]/_components/canvas/index.tsx
echo -e "\n=== Check if COPY or PASTE hotkeys are used ==="
grep -n "Hotkey\.COPY\|Hotkey\.PASTE\|\.COPY\|\.PASTE" ./apps/web/client/src/app/project/[id]/_components/canvas/index.tsx
echo -e "\n=== Search for ContextMenuItem usage ==="
grep -n "ContextMenuItem" ./apps/web/client/src/app/project/[id]/_components/canvas/index.tsx | head -10
echo -e "\n=== Search for keyboard shortcut display patterns ==="
grep -n "⌘\|Ctrl\|text-xs\|muted-foreground" ./apps/web/client/src/app/project/[id]/_components/canvas/index.tsxLength of output: 3154
🏁 Script executed:
#!/bin/bash
# Check the HotkeysArea component to understand keyboard handling
echo "=== Content of hotkeys/index.tsx ==="
cat ./apps/web/client/src/app/project/[id]/_components/canvas/hotkeys/index.tsx
echo -e "\n=== Search for Hotkey class usage in canvas components ==="
grep -r "Hotkey\." ./apps/web/client/src/app/project/[id]/_components/canvas/ --include="*.tsx" | head -15
echo -e "\n=== Check imports at top of canvas/index.tsx ==="
head -30 ./apps/web/client/src/app/project/[id]/_components/canvas/index.tsxLength of output: 9112
Use the Hotkey system to display platform-appropriate keyboard shortcuts instead of hardcoded Mac-only labels.
The context menu displays hardcoded "⌘ C" and "⌘ V" at lines 398 and 407, but the application's keyboard binding system (HotkeysArea in hotkeys/index.tsx) correctly supports cross-platform shortcuts via the Hotkey class, which dynamically converts mod to ⌘ on macOS and Ctrl on Windows/Linux.
Import the Hotkey class and replace the hardcoded labels with:
- Line 398:
{Hotkey.COPY.readableCommand} - Line 407:
{Hotkey.PASTE.readableCommand}
This ensures the displayed shortcuts match the actual keyboard bindings on all platforms.
🤖 Prompt for AI Agents
In apps/web/client/src/app/project/[id]/_components/canvas/index.tsx around
lines 391 to 408, the context menu uses hardcoded Mac-only labels ("⌘ C" and "⌘
V"); import the Hotkey class from the hotkeys module and replace those hardcoded
spans with {Hotkey.COPY.readableCommand} and {Hotkey.PASTE.readableCommand}
respectively so the displayed shortcuts are platform-appropriate; also add the
Hotkey import at the top of the file (e.g., import { Hotkey } from
'path/to/hotkeys') and remove the old hardcoded text.
Description
Checking if Cursor was able to implement the following summary of changes from a mockup of the Onlook Canvas.
"I need to implement a right-click context menu system for the canvas with the following requirements:
Hide the left sidebar buttons - Comment out or hide the Sidebar component that contains the vertical button panel on the left side
Install and configure context menu if it's not already in there. If IT IS, then don't replace it. - Add @radix-ui/react-context-menu package and create a reusable context menu component at /src/components/ui/context-menu.tsx with these exports:
ContextMenu, ContextMenuTrigger, ContextMenuContent
ContextMenuItem, ContextMenuSeparator
ContextMenuSub, ContextMenuSubTrigger, ContextMenuSubContent
Style with dark theme
SubTrigger should have a right arrow icon indicator
Wrap Canvas in context menu if not already there - Update the Canvas right click to:
Accept an onSidebarClick prop
Add a context menu with this structure:
"Add Element" (with PlusIcon)
"Add Component" (with ComponentInstanceIcon)
Separator
"Panels" submenu (with grid icon) that expands to show:
Layers
Brand
Pages
Images
Separator
Branches
Separator
"Copy" (with ⌘ C shortcut text)
"Paste" (with ⌘ V shortcut text)
Update LeftPanel - Add:
onClose prop to LeftPanel component
X button (Cross2Icon from Radix) in the upper right corner of the panel header
Support nullable activeSection prop (string | null)
Update page.tsx - Modify to:
Pass onSidebarClick to Canvas component
Hide/comment out the Sidebar component
Reposition LeftPanel from left-20 to left-2 (since sidebar is hidden)
Pass onClose handler to LeftPanel that closes the panel
The end result should allow users to right-click the canvas, see a professional context menu with a "Panels" submenu option that opens the various side panels, and close panels with an X button in the upper-right corner
"
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Important
Implements a context menu for the canvas, updates the left panel with close functionality, and modifies the main component to integrate these changes.
Canvasinindex.tsxwith options like "Add Element", "Add Component", and a "Panels" submenu.ContextMenucomponents from@onlook/ui/context-menu.onSidebarClickprop to open specific panels.DesignPanelandLeftPanelto supportonCloseandactiveSectionprops.DesignPanel.DesignPanel.Mainto passonSidebarClicktoCanvasandonClosetoLeftPanel.LeftPanelposition toleft-2inmain.tsx.@radix-ui/react-context-menupackage for context menu functionality.This description was created by
for 0811c8f. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit