-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat: remove text edit on double click and drag to reorder #2883
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughDisabled canvas drag/drag-and-drop and removed global drag lifecycle handlers; double-click now opens code in IDE for elements with an oid; clicking empty canvas while editing switches the right panel from DEV to CHAT; updated CodeControls button styling; code-tab scroll now targets startPos with a yMargin of 48. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Canvas as Canvas Element
participant IDE as IDE Opener
Note over Canvas: Drag interactions disabled / global drag handlers removed
User->>Canvas: Double-click element
alt element has oid
Canvas->>IDE: openCodeBlock(oid)
IDE-->>Canvas: opened
else no oid
Canvas-->>User: show toast error
end
sequenceDiagram
autonumber
actor User
participant Canvas as Canvas
participant UI as Right Panel
User->>Canvas: Click empty space (while code editing)
Canvas->>Canvas: clear UI selection/state
Canvas->>UI: setRightPanelTab(CHAT)
UI-->>User: CHAT tab active (mounted/visible)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
||
| if (editorEngine.move.shouldDrag) { | ||
| await editorEngine.move.drag(e, getRelativeMousePosition); | ||
| if (false) { // Disabled: editorEngine.move.shouldDrag |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid using 'if(false)' to disable code; prefer removing dead code or using a proper flag for clarity.
| description: "Element dragging has been disabled on this canvas", | ||
| }); | ||
|
|
||
| /* Disabled: Original drop functionality |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Disabled drag & drop functionality now shows a toast; consider cleaning up or removing the commented out drop logic to avoid clutter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/web/client/src/app/project/[id]/_components/canvas/index.tsx (2)
60-66: Remove debug console statements.Console logging statements should be removed from production code. These debug statements are helpful during development but should not remain in the final code.
- // Switch to chat mode when clicking on empty canvas space during code editing - console.log('Canvas click detected in design mode, current tab:', editorEngine.state.rightPanelTab); - if (editorEngine.state.rightPanelTab === EditorTabValue.DEV) { - console.log('Switching from DEV to CHAT mode'); - editorEngine.state.rightPanelTab = EditorTabValue.CHAT; - } + // Switch to chat mode when clicking on empty canvas space during code editing + if (editorEngine.state.rightPanelTab === EditorTabValue.DEV) { + editorEngine.state.rightPanelTab = EditorTabValue.CHAT; + }
71-76: Remove debug console statements and check code duplication.The same logic is duplicated in two places with identical console logging statements. Consider extracting this into a helper function to reduce duplication.
Extract the common logic:
+ const switchToChat = () => { + if (editorEngine.state.rightPanelTab === EditorTabValue.DEV) { + editorEngine.state.rightPanelTab = EditorTabValue.CHAT; + } + }; const handleCanvasMouseDown = (event: React.MouseEvent<HTMLDivElement>) => { // ... existing code ... - // Switch to chat mode when clicking on empty canvas space during code editing - console.log('Canvas click detected in design mode, current tab:', editorEngine.state.rightPanelTab); - if (editorEngine.state.rightPanelTab === EditorTabValue.DEV) { - console.log('Switching from DEV to CHAT mode'); - editorEngine.state.rightPanelTab = EditorTabValue.CHAT; - } + switchToChat(); } else if (event.button === 0) { // Only clear UI for left clicks that don't start drag selection editorEngine.clearUI(); - // Switch to chat mode when clicking on empty canvas space during code editing - console.log('Canvas click detected, current tab:', editorEngine.state.rightPanelTab); - if (editorEngine.state.rightPanelTab === EditorTabValue.DEV) { - console.log('Switching from DEV to CHAT mode'); - editorEngine.state.rightPanelTab = EditorTabValue.CHAT; - } + switchToChat(); }apps/web/client/src/app/project/[id]/_components/canvas/frame/gesture.tsx (1)
104-105: Fix constant condition warning.The static analysis tool correctly identified a constant condition. The
falseshould be replaced with the appropriate condition or the entire block should be removed if drag functionality is permanently disabled.Based on the PR objectives, either remove this dead code entirely or replace with a proper feature flag:
- if (false) { // Disabled: editorEngine.move.shouldDrag - // Disabled: await editorEngine.move.drag(e, getRelativeMousePosition); + // Drag functionality has been disabled + // TODO: Remove this block if drag is permanently disabled + if (false) { + // Disabled drag functionalityOr remove entirely:
- if (false) { // Disabled: editorEngine.move.shouldDrag - // Disabled: await editorEngine.move.drag(e, getRelativeMousePosition); - } else if ( + if (
📜 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/frame/gesture.tsx(4 hunks)apps/web/client/src/app/project/[id]/_components/canvas/index.tsx(2 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/code-tab/code-controls.tsx(2 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/code-tab/index.tsx(1 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/canvas/frame/gesture.tsxapps/web/client/src/app/project/[id]/_components/right-panel/code-tab/index.tsxapps/web/client/src/app/project/[id]/_components/canvas/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/code-tab/code-controls.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/canvas/frame/gesture.tsxapps/web/client/src/app/project/[id]/_components/right-panel/code-tab/index.tsxapps/web/client/src/app/project/[id]/_components/canvas/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/code-tab/code-controls.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/canvas/frame/gesture.tsxapps/web/client/src/app/project/[id]/_components/right-panel/code-tab/index.tsxapps/web/client/src/app/project/[id]/_components/canvas/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/code-tab/code-controls.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/canvas/frame/gesture.tsxapps/web/client/src/app/project/[id]/_components/right-panel/code-tab/index.tsxapps/web/client/src/app/project/[id]/_components/canvas/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/code-tab/code-controls.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/canvas/frame/gesture.tsxapps/web/client/src/app/project/[id]/_components/right-panel/code-tab/index.tsxapps/web/client/src/app/project/[id]/_components/canvas/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/code-tab/code-controls.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/canvas/frame/gesture.tsxapps/web/client/src/app/project/[id]/_components/right-panel/code-tab/index.tsxapps/web/client/src/app/project/[id]/_components/canvas/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/code-tab/code-controls.tsx
🧬 Code graph analysis (1)
apps/web/client/src/app/project/[id]/_components/canvas/frame/gesture.tsx (1)
packages/ui/src/components/sonner.tsx (1)
toast(19-19)
🪛 Biome (2.1.2)
apps/web/client/src/app/project/[id]/_components/canvas/frame/gesture.tsx
[error] 104-104: Unexpected constant condition.
(lint/correctness/noConstantCondition)
🔇 Additional comments (7)
apps/web/client/src/app/project/[id]/_components/canvas/index.tsx (1)
5-5: Import looks correct.The import of
EditorTabValueis appropriate for the new functionality.apps/web/client/src/app/project/[id]/_components/canvas/frame/gesture.tsx (3)
82-85: LGTM! Double-click now opens code instead of text editing.The change from text editing to opening code blocks is exactly what the PR objectives describe. The implementation correctly checks for
el.oidbefore attempting to open the code block.
219-222: Clean implementation of disabled drag-and-drop.The disabled drag-and-drop implementation is well-handled with proper error messaging and preservation of original code in comments. The toast notification clearly informs users about the disabled functionality.
Also applies to: 226-232, 234-266
78-78: Confirm drag functionality should remain disabled.startDragPreparation is commented out in gesture.tsx, but the move store and frame/preload APIs still implement/expose drag — verify the removal is intentional across editor modes and remote APIs.
- apps/web/client/src/app/project/[id]/_components/canvas/frame/gesture.tsx — "// Disabled: editorEngine.move.startDragPreparation(el, pos, frameData);"
- apps/web/client/src/components/store/editor/move/index.ts — startDragPreparation(...) definition and frameData.view.startDrag(...) usage.
- apps/web/client/src/app/project/[id]/_components/canvas/frame/view.tsx — exposes startDrag/drag via promisifyMethod.
- apps/web/preload/script/api/index.ts and apps/web/preload/script/api/elements/move/drag.ts — startDrag/drag exported in preload API.
Verify intent and confirm there are no other callers or required editor modes before leaving this disabled.
apps/web/client/src/app/project/[id]/_components/right-panel/code-tab/code-controls.tsx (2)
36-36: Clean up class name.The removal of group-hover related styling and addition of items-center alignment looks good for the new button layout.
89-89: Button variant and styling updates look appropriate.The changes from "ghost" to "secondary" variant, updated padding/margins, and addition of the "Save" text span create a more prominent save button. The styling adjustments align well with the new variant.
Also applies to: 94-94, 104-104
apps/web/client/src/app/project/[id]/_components/right-panel/code-tab/index.tsx (1)
164-171: Verify scroll target change and UX consistency.Changed scroll target from
selection.main.anchortostartPoswithy: 'start', yMargin: 48. Another call usesEditorView.scrollIntoView(pos, { y: 'center' })(apps/web/client/src/app/project/[id]/_components/right-panel/code-tab/code-mirror-config.ts:244). Confirm the differing behaviors are intentional (element selection should snap to top with margin vs search centering); if not, align or document the intended UX.
| console.log('Canvas click detected in design mode, current tab:', editorEngine.state.rightPanelTab); | ||
| if (editorEngine.state.rightPanelTab === EditorTabValue.DEV) { | ||
| console.log('Switching from DEV to CHAT mode'); | ||
| editorEngine.state.rightPanelTab = EditorTabValue.CHAT; | ||
| } | ||
| } else if (event.button === 0) { | ||
| // Only clear UI for left clicks that don't start drag selection | ||
| editorEngine.clearUI(); | ||
|
|
||
| // Switch to chat mode when clicking on empty canvas space during code editing | ||
| console.log('Canvas click detected, current tab:', editorEngine.state.rightPanelTab); | ||
| if (editorEngine.state.rightPanelTab === EditorTabValue.DEV) { | ||
| console.log('Switching from DEV to CHAT mode'); | ||
| editorEngine.state.rightPanelTab = EditorTabValue.CHAT; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Debug console.log statements left in production code. These console.log statements (lines 62, 64, 72, 74) should be removed as they will clutter the browser console and may expose internal application state to users.
| console.log('Canvas click detected in design mode, current tab:', editorEngine.state.rightPanelTab); | |
| if (editorEngine.state.rightPanelTab === EditorTabValue.DEV) { | |
| console.log('Switching from DEV to CHAT mode'); | |
| editorEngine.state.rightPanelTab = EditorTabValue.CHAT; | |
| } | |
| } else if (event.button === 0) { | |
| // Only clear UI for left clicks that don't start drag selection | |
| editorEngine.clearUI(); | |
| // Switch to chat mode when clicking on empty canvas space during code editing | |
| console.log('Canvas click detected, current tab:', editorEngine.state.rightPanelTab); | |
| if (editorEngine.state.rightPanelTab === EditorTabValue.DEV) { | |
| console.log('Switching from DEV to CHAT mode'); | |
| editorEngine.state.rightPanelTab = EditorTabValue.CHAT; | |
| if (editorEngine.state.rightPanelTab === EditorTabValue.DEV) { | |
| editorEngine.state.rightPanelTab = EditorTabValue.CHAT; | |
| } | |
| } else if (event.button === 0) { | |
| // Only clear UI for left clicks that don't start drag selection | |
| editorEngine.clearUI(); | |
| // Switch to chat mode when clicking on empty canvas space during code editing | |
| if (editorEngine.state.rightPanelTab === EditorTabValue.DEV) { | |
| editorEngine.state.rightPanelTab = EditorTabValue.CHAT; |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
apps/web/client/src/app/project/[id]/_components/canvas/frame/gesture.tsx
Outdated
Show resolved
Hide resolved
| <TabsContent | ||
| forceMount | ||
| className={cn( | ||
| "h-full overflow-y-auto", | ||
| editorEngine.state.rightPanelTab !== EditorTabValue.CHAT && 'hidden', | ||
| )} value={EditorTabValue.CHAT}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential rendering bug: Using forceMount with conditional visibility via 'hidden' class can cause issues with component lifecycle and accessibility. The TabsContent will always be mounted but hidden with CSS, which may cause the ChatTab component to initialize and run effects even when not visible, potentially leading to unnecessary API calls or state updates. Consider using conditional rendering instead of forceMount + hidden class.
| <TabsContent | |
| forceMount | |
| className={cn( | |
| "h-full overflow-y-auto", | |
| editorEngine.state.rightPanelTab !== EditorTabValue.CHAT && 'hidden', | |
| )} value={EditorTabValue.CHAT}> | |
| <TabsContent | |
| className="h-full overflow-y-auto" | |
| value={EditorTabValue.CHAT}> |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/client/src/app/project/[id]/_components/canvas/frame/gesture.tsx (1)
62-79: Click handler overrides element selection; limit frame selection to empty/body clicks.
onMouseDownselects the element under the cursor, but the unconditionalonClicklater re-selects the frame, clearing the element selection.-const handleClick = useCallback( - (e: React.MouseEvent<HTMLDivElement>) => { - editorEngine.frames.select([frame]); - }, - [editorEngine.frames], -); +const handleClick = useCallback( + async (e: React.MouseEvent<HTMLDivElement>) => { + // Only select the frame when the click targets the page body (i.e., empty canvas) + const frameData = getFrameData(); + if (!frameData?.view) return; + const pos = getRelativeMousePosition(e); + const el = await frameData.view.getElementAtLoc(pos.x, pos.y, false); + if (el && el.tagName?.toLowerCase() !== 'body') return; + editorEngine.frames.select([frame], e.shiftKey); + }, + [editorEngine.frames, frame, getFrameData, getRelativeMousePosition], +);This aligns
onClickbehavior with the body-check you already perform inMOUSE_DOWN.Also applies to: 122-127
🧹 Nitpick comments (5)
apps/web/client/src/app/project/[id]/_components/right-panel/index.tsx (2)
82-87: Force-mounted CHAT content: ensure it’s non-focusable/inert when hidden.With
forceMount+ conditionalhiddenclass, ensure keyboard focus can’t tab into hidden CHAT content. Prefer relying on the tabs library’s inactive state or addaria-hidden/inertwhen hidden.Apply one of these:
Option A: Use data-state selector (if supported by @onlook/ui/tabs/Radix wrapper) and drop manual hidden logic:
-<TabsContent - forceMount - className={cn( - "h-full overflow-y-auto", - editorEngine.state.rightPanelTab !== EditorTabValue.CHAT && 'hidden', - )} value={EditorTabValue.CHAT}> +<TabsContent + forceMount + className="h-full overflow-y-auto data-[state=inactive]:hidden" + value={EditorTabValue.CHAT} +>Option B: Keep your conditional but explicitly mark as inert:
-<TabsContent - forceMount - className={cn( - "h-full overflow-y-auto", - editorEngine.state.rightPanelTab !== EditorTabValue.CHAT && 'hidden', - )} value={EditorTabValue.CHAT}> +<TabsContent + forceMount + aria-hidden={editorEngine.state.rightPanelTab !== EditorTabValue.CHAT} + inert={editorEngine.state.rightPanelTab !== EditorTabValue.CHAT ? '' : undefined} + className={cn( + 'h-full overflow-y-auto', + editorEngine.state.rightPanelTab !== EditorTabValue.CHAT && 'hidden', + )} + value={EditorTabValue.CHAT} +>
71-76: Avoid hardcoded user-facing text; localize “Code”.Follow app i18n guidelines and use next-intl keys like CHAT.
-<Icons.Code className="mr-1 h-4 w-4" /> -Code +<Icons.Code className="mr-1 h-4 w-4" /> +{t(transKeys.editor.panels.edit.tabs.code.name)}If
transKeys.editor.panels.edit.tabs.code.namedoesn’t exist, add it to messages.apps/web/client/src/app/project/[id]/_components/canvas/frame/gesture.tsx (3)
81-87: Double-click open: localize toast and confirm panel switching to DEV.Great that double‑click jumps to code. Avoid hardcoded text and ensure right panel actually switches to DEV when opening code.
- if (el.oid) { - editorEngine.ide.openCodeBlock(el.oid); - } else { - toast.error('Cannot find element in code panel'); - return; - } + if (el.oid) { + // Ensure the DEV tab is visible when opening code + editorEngine.state.rightPanelTab = EditorTabValue.DEV; + editorEngine.ide.openCodeBlock(el.oid); + } else { + // TODO i18n + toast.error('Cannot find element in code panel'); + return; + }Please confirm whether
openCodeBlockalready handles tab switching; if so, setting the tab here may be redundant.
63-66: UsetoLowerCase()for tag names.Using locale-aware casing can be surprising. Tag names are ASCII;
toLowerCase()is safer and marginally faster.-if (el.tagName.toLocaleLowerCase() === 'body') { +if (el.tagName.toLowerCase() === 'body') {
169-171: Localize toast strings per app i18n guidelines.Replace hardcoded strings with next-intl messages.
- toast.error("Drag and drop is disabled", { - description: "Element dragging has been disabled on this canvas", - }); + // TODO i18n + toast.error( + /* t(transKeys.canvas.drag.disabledTitle) */ 'Drag and drop is disabled', + { description: /* t(transKeys.canvas.drag.disabledDescription) */ 'Element dragging has been disabled on this canvas' }, + );Same for the double‑click error above.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/client/src/app/project/[id]/_components/canvas/frame/gesture.tsx(4 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/index.tsx(1 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/right-panel/index.tsxapps/web/client/src/app/project/[id]/_components/canvas/frame/gesture.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/right-panel/index.tsxapps/web/client/src/app/project/[id]/_components/canvas/frame/gesture.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/right-panel/index.tsxapps/web/client/src/app/project/[id]/_components/canvas/frame/gesture.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/right-panel/index.tsxapps/web/client/src/app/project/[id]/_components/canvas/frame/gesture.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/right-panel/index.tsxapps/web/client/src/app/project/[id]/_components/canvas/frame/gesture.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/right-panel/index.tsxapps/web/client/src/app/project/[id]/_components/canvas/frame/gesture.tsx
🔇 Additional comments (4)
apps/web/client/src/app/project/[id]/_components/canvas/frame/gesture.tsx (4)
158-163: Clean up “disabled” drag-over path or hide behind a flag.Commented code and noop handlers add noise. Either remove the dead path or guard it with a feature flag.
-const handleDragOver = async (e: React.DragEvent<HTMLDivElement>) => { - // Disabled drag and drop functionality - e.preventDefault(); - e.stopPropagation(); - // Disabled: await handleMouseEvent(e, MouseAction.MOVE); -}; +const handleDragOver = async (e: React.DragEvent<HTMLDivElement>) => { + e.preventDefault(); + e.stopPropagation(); +};
101-114: Mouse-move throttling looks good.The simplified branching and 16ms throttle are reasonable;
insert.drawremains responsive while non-drawing modes update hover state at ~60fps.
165-173: Drag-and-drop claims to be disabled, but drop logic still runs. Early return is missing.After the toast, the function proceeds to parse data and insert elements. This contradicts the “disabled” UX and can mutate the canvas.
const handleDrop = async (e: React.DragEvent<HTMLDivElement>) => { - // Disabled drag and drop functionality + // Drag and drop is currently disabled. e.preventDefault(); e.stopPropagation(); - toast.error("Drag and drop is disabled", { - description: "Element dragging has been disabled on this canvas", - }); + toast.error('Drag and drop is disabled', { + description: 'Element dragging has been disabled on this canvas', + }); + return; // IMPORTANT: do not process drops while disabledOptionally, gate the rest of the function behind a feature flag if you intend to re-enable later.
10-10: Client boundary confirmed — no change requiredapps/web/client/src/app/project/[id]/_components/canvas/frames.tsx contains
'use client'and imports FrameView (which imports GestureScreen), so the ancestor client boundary covers GestureScreen's hooks +observer.
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Important
This PR removes text editing on double-click, introduces code opening on double-click, and updates canvas and right panel interactions.
gesture.tsx).canvas/index.tsx).right-panel/index.tsx).code-tab/index.tsx).code-controls.tsx).gesture.tsx).This description was created by
for 52129bc. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Style
Chores