Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export const FrameView = observer(({ frame }: { frame: Frame }) => {
const [isResizing, setIsResizing] = useState(false);
const [reloadKey, setReloadKey] = useState(0);
const [hasTimedOut, setHasTimedOut] = useState(false);
const isSelected = editorEngine.frames.isSelected(frame.id);

// Check if sandbox is connecting for this frame's branch
const branchData = editorEngine.branches.getBranchDataById(frame.branchId);
Expand Down Expand Up @@ -60,7 +61,10 @@ export const FrameView = observer(({ frame }: { frame: Frame }) => {
<RightClickMenu>
<TopBar frame={frame} />
</RightClickMenu>
<div className="relative">
<div className="relative" style={{
outline: isSelected ? '2px solid rgb(94, 234, 212)' : 'none',
borderRadius: '4px',
}}>
<ResizeHandles frame={frame} setIsResizing={setIsResizing} />
<FrameComponent key={reloadKey} frame={frame} reloadIframe={reloadIframe} ref={iFrameRef} />
<GestureScreen frame={frame} isResizing={isResizing} />
Expand Down
97 changes: 95 additions & 2 deletions apps/web/client/src/app/project/[id]/_components/canvas/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@ import { useEditorEngine } from '@/components/store/editor';
import { EditorAttributes } from '@onlook/constants';
import { EditorMode } from '@onlook/models';
import { observer } from 'mobx-react-lite';
import { useCallback, useEffect, useMemo, useRef } from 'react';
import { useCallback, useEffect, useMemo, useRef, useState } from 'react';
import { Frames } from './frames';
import { HotkeysArea } from './hotkeys';
import { Overlay } from './overlay';
import { DragSelectOverlay } from './overlay/drag-select';
import { PanOverlay } from './overlay/pan';
import { RecenterCanvasButton } from './recenter-canvas-button';

Expand All @@ -25,12 +26,94 @@ export const Canvas = observer(() => {
const containerRef = useRef<HTMLDivElement>(null);
const scale = editorEngine.canvas.scale;
const position = editorEngine.canvas.position;
const [isDragSelecting, setIsDragSelecting] = useState(false);
const [dragSelectStart, setDragSelectStart] = useState({ x: 0, y: 0 });
const [dragSelectEnd, setDragSelectEnd] = useState({ x: 0, y: 0 });

const handleCanvasMouseDown = (event: React.MouseEvent<HTMLDivElement>) => {
if (event.target !== containerRef.current) {
return;
}
editorEngine.clearUI();

// Start drag selection only in design mode and when not holding middle mouse button
if (editorEngine.state.editorMode === EditorMode.DESIGN && event.button === 0) {
const rect = containerRef.current.getBoundingClientRect();
const x = event.clientX - rect.left;
const y = event.clientY - rect.top;

setIsDragSelecting(true);
setDragSelectStart({ x, y });
setDragSelectEnd({ x, y });

// Clear existing selections if not shift-clicking
if (!event.shiftKey) {
editorEngine.clearUI();
editorEngine.frames.deselectAll();
}
} else {
editorEngine.clearUI();
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logic error: The drag selection logic only triggers when event.button === 0 (left mouse button), but the else clause at line 53-55 calls editorEngine.clearUI() for ALL other mouse buttons including right-click and middle-click. This will incorrectly clear the UI when users right-click to open context menus or middle-click to pan. The else clause should either be removed or should only handle specific cases where clearing UI is actually desired.

Suggested change
if (editorEngine.state.editorMode === EditorMode.DESIGN && event.button === 0) {
const rect = containerRef.current.getBoundingClientRect();
const x = event.clientX - rect.left;
const y = event.clientY - rect.top;
setIsDragSelecting(true);
setDragSelectStart({ x, y });
setDragSelectEnd({ x, y });
// Clear existing selections if not shift-clicking
if (!event.shiftKey) {
editorEngine.clearUI();
editorEngine.frames.deselectAll();
}
} else {
editorEngine.clearUI();
}
if (editorEngine.state.editorMode === EditorMode.DESIGN && event.button === 0) {
const rect = containerRef.current.getBoundingClientRect();
const x = event.clientX - rect.left;
const y = event.clientY - rect.top;
setIsDragSelecting(true);
setDragSelectStart({ x, y });
setDragSelectEnd({ x, y });
// Clear existing selections if not shift-clicking
if (!event.shiftKey) {
editorEngine.clearUI();
editorEngine.frames.deselectAll();
}
}
// Removed the else clause that was incorrectly clearing UI for all non-left clicks

Spotted by Diamond

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

};

const handleCanvasMouseMove = (event: React.MouseEvent<HTMLDivElement>) => {
if (!isDragSelecting || !containerRef.current) {
return;
}

const rect = containerRef.current.getBoundingClientRect();
const x = event.clientX - rect.left;
const y = event.clientY - rect.top;
setDragSelectEnd({ x, y });
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mouse move handler has no bounds checking and will cause performance issues. The handler executes on every mouse movement even when not drag selecting, and when drag selecting, it performs expensive DOM operations (getBoundingClientRect) on every pixel movement without throttling. This will cause UI lag and poor user experience. Add early return when not drag selecting and consider throttling the coordinate updates.

Suggested change
const handleCanvasMouseMove = (event: React.MouseEvent<HTMLDivElement>) => {
if (!isDragSelecting || !containerRef.current) {
return;
}
const rect = containerRef.current.getBoundingClientRect();
const x = event.clientX - rect.left;
const y = event.clientY - rect.top;
setDragSelectEnd({ x, y });
};
const handleCanvasMouseMove = useCallback(
throttle((event: React.MouseEvent<HTMLDivElement>) => {
if (!isDragSelecting || !containerRef.current) {
return;
}
const rect = containerRef.current.getBoundingClientRect();
const x = event.clientX - rect.left;
const y = event.clientY - rect.top;
setDragSelectEnd({ x, y });
}, 16), // ~60fps
[isDragSelecting]
);

Spotted by Diamond

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.


const handleCanvasMouseUp = (event: React.MouseEvent<HTMLDivElement>) => {
if (!isDragSelecting) {
return;
}

// Calculate which frames are within the selection rectangle
const selectionRect = {
left: Math.min(dragSelectStart.x, dragSelectEnd.x),
top: Math.min(dragSelectStart.y, dragSelectEnd.y),
right: Math.max(dragSelectStart.x, dragSelectEnd.x),
bottom: Math.max(dragSelectStart.y, dragSelectEnd.y),
};

// Convert selection rect to canvas coordinates
const canvasSelectionRect = {
left: (selectionRect.left - position.x) / scale,
top: (selectionRect.top - position.y) / scale,
right: (selectionRect.right - position.x) / scale,
bottom: (selectionRect.bottom - position.y) / scale,
};

// Find all frames that intersect with the selection rectangle
const allFrames = editorEngine.frames.getAll();
const selectedFrames = allFrames.filter(frameData => {
const frame = frameData.frame;
const frameLeft = frame.position.x;
const frameTop = frame.position.y;
const frameRight = frame.position.x + frame.dimension.width;
const frameBottom = frame.position.y + frame.dimension.height;

// Check if frame intersects with selection rectangle
return !(
frameLeft > canvasSelectionRect.right ||
frameRight < canvasSelectionRect.left ||
frameTop > canvasSelectionRect.bottom ||
frameBottom < canvasSelectionRect.top
);
});

// Select the frames if any were found in the selection
if (selectedFrames.length > 0) {
editorEngine.frames.select(
selectedFrames.map(fd => fd.frame),
event.shiftKey // multiselect if shift is held
);
}

setIsDragSelecting(false);
};

const handleZoom = useCallback(
Expand Down Expand Up @@ -166,11 +249,21 @@ export const Canvas = observer(() => {
ref={containerRef}
className="overflow-hidden bg-background-onlook flex flex-grow relative"
onMouseDown={handleCanvasMouseDown}
onMouseMove={handleCanvasMouseMove}
onMouseUp={handleCanvasMouseUp}
onMouseLeave={handleCanvasMouseUp}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mouse leave handler will incorrectly terminate drag selection when user drags outside the canvas container but is still dragging. This breaks the expected drag selection behavior where users should be able to drag beyond the visible canvas area. The onMouseLeave should not call handleCanvasMouseUp, or should only do so when the mouse button is not pressed.

Suggested change
onMouseLeave={handleCanvasMouseUp}
onMouseLeave={(e) => {
// Only terminate drag if no mouse button is pressed
if (e.buttons === 0) {
handleCanvasMouseUp(e);
}
}}

Spotted by Diamond

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

>
<div id={EditorAttributes.CANVAS_CONTAINER_ID} style={transformStyle}>
<Frames />
</div>
<RecenterCanvasButton />
<DragSelectOverlay
startX={dragSelectStart.x}
startY={dragSelectStart.y}
endX={dragSelectEnd.x}
endY={dragSelectEnd.y}
isSelecting={isDragSelecting}
/>
<Overlay />
<PanOverlay
clampPosition={(position: { x: number; y: number }) =>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
'use client';

import { observer } from 'mobx-react-lite';

interface DragSelectOverlayProps {
startX: number;
startY: number;
endX: number;
endY: number;
isSelecting: boolean;
}

export const DragSelectOverlay = observer(({ startX, startY, endX, endY, isSelecting }: DragSelectOverlayProps) => {
if (!isSelecting) {
return null;
}

const left = Math.min(startX, endX);
const top = Math.min(startY, endY);
const width = Math.abs(endX - startX);
const height = Math.abs(endY - startY);

return (
<div
className="absolute border-2 border-blue-500 bg-blue-500/10 pointer-events-none"
style={{
left: `${left}px`,
top: `${top}px`,
width: `${width}px`,
height: `${height}px`,
}}
/>
);
});