From 78f1e68bfbddbc0df9c7e936f08743397f6b02ef Mon Sep 17 00:00:00 2001 From: Zeid Diez Date: Thu, 9 Jul 2026 17:28:07 -0700 Subject: [PATCH] Fix UI Builder shift-click resize collapsing layers without a drag Shift-click near a corner used a generic 'corner' handle that applied both left and right (and top and bottom) in one move, shrinking the layer to the 8px minimum. Use directional corner handles, compute resize from mousedown origin geometry, and ignore gestures that never cross a small drag threshold so a click-in-place is a no-op. --- apps/cli/tests/ui-builder-resize.test.ts | 104 +++++++++++++++++ apps/web/src/UIBuilder.tsx | 138 ++++++++++++++++------- apps/web/src/builderResize.ts | 86 ++++++++++++++ 3 files changed, 287 insertions(+), 41 deletions(-) create mode 100644 apps/cli/tests/ui-builder-resize.test.ts create mode 100644 apps/web/src/builderResize.ts diff --git a/apps/cli/tests/ui-builder-resize.test.ts b/apps/cli/tests/ui-builder-resize.test.ts new file mode 100644 index 0000000..fab5f4b --- /dev/null +++ b/apps/cli/tests/ui-builder-resize.test.ts @@ -0,0 +1,104 @@ +import { describe, it, expect } from 'vitest'; +import { + getResizeHandle, + applyResizeHandle, + DRAG_THRESHOLD_PX, + MIN_LAYER_SIZE, + type ResizeHandle, +} from '../../web/src/builderResize'; +import type { Layer } from '@placeholderer/schemas'; + +function rectLayer(x: number, y: number, width: number, height: number): Layer { + return { + id: 'l1', + type: 'rect', + name: 'R', + visible: true, + locked: false, + opacity: 1, + blendMode: 'source-over', + x, + y, + width, + height, + fill: '#000', + }; +} + +describe('getResizeHandle', () => { + const layer = rectLayer(100, 100, 200, 100); + + it('returns distinct corner handles, not a generic corner', () => { + expect(getResizeHandle(layer, 100, 100)).toBe('nw'); + expect(getResizeHandle(layer, 300, 100)).toBe('ne'); + expect(getResizeHandle(layer, 100, 200)).toBe('sw'); + expect(getResizeHandle(layer, 300, 200)).toBe('se'); + }); + + it('returns edge handles', () => { + expect(getResizeHandle(layer, 100, 150)).toBe('left'); + expect(getResizeHandle(layer, 300, 150)).toBe('right'); + expect(getResizeHandle(layer, 200, 100)).toBe('top'); + expect(getResizeHandle(layer, 200, 200)).toBe('bottom'); + }); + + it('returns null when not near an edge', () => { + expect(getResizeHandle(layer, 200, 150)).toBeNull(); + }); +}); + +describe('applyResizeHandle', () => { + const origin = { x: 100, y: 100, w: 200, h: 100 }; + + it('resizes from the right edge without moving x', () => { + const r = applyResizeHandle('right', origin.x, origin.y, origin.w, origin.h, 350, 150); + expect(r).toEqual({ x: 100, y: 100, width: 250, height: 100 }); + }); + + it('resizes from the left edge keeping the right edge fixed', () => { + const r = applyResizeHandle('left', origin.x, origin.y, origin.w, origin.h, 120, 150); + expect(r.x).toBe(120); + expect(r.width).toBe(180); + expect(r.y).toBe(100); + expect(r.height).toBe(100); + }); + + it('nw corner only adjusts left and top (does not collapse via right/bottom)', () => { + // Pointer still near the NW corner (click-in-place). Old 'corner' + // logic would also run the right/bottom branches and shrink to min. + const r = applyResizeHandle('nw', origin.x, origin.y, origin.w, origin.h, 100, 100); + expect(r.width).toBeGreaterThan(MIN_LAYER_SIZE); + expect(r.height).toBeGreaterThan(MIN_LAYER_SIZE); + expect(r).toEqual({ x: 100, y: 100, width: 200, height: 100 }); + }); + + it('se corner grows when pointer moves out', () => { + const r = applyResizeHandle('se', origin.x, origin.y, origin.w, origin.h, 400, 250); + expect(r).toEqual({ x: 100, y: 100, width: 300, height: 150 }); + }); + + it('clamps to MIN_LAYER_SIZE', () => { + const r = applyResizeHandle('right', origin.x, origin.y, origin.w, origin.h, origin.x + 2, 150); + expect(r.width).toBe(MIN_LAYER_SIZE); + }); + + it.each([ + ['nw', 100, 100], + ['ne', 300, 100], + ['sw', 100, 200], + ['se', 300, 200], + ] as [ResizeHandle, number, number][])( + 'click-in-place on %s leaves geometry unchanged', + (handle, mx, my) => { + const r = applyResizeHandle(handle, origin.x, origin.y, origin.w, origin.h, mx, my); + expect(r).toEqual({ x: 100, y: 100, width: 200, height: 100 }); + }, + ); +}); + +describe('DRAG_THRESHOLD_PX', () => { + it('is a small positive threshold', () => { + expect(DRAG_THRESHOLD_PX).toBeGreaterThan(0); + expect(DRAG_THRESHOLD_PX).toBeLessThanOrEqual(8); + }); +}); diff --git a/apps/web/src/UIBuilder.tsx b/apps/web/src/UIBuilder.tsx index 23b1db7..e458267 100644 --- a/apps/web/src/UIBuilder.tsx +++ b/apps/web/src/UIBuilder.tsx @@ -17,6 +17,12 @@ import { rasterLayer, filledShapeLayer, } from './builderLayerFactories'; +import { + DRAG_THRESHOLD_PX, + applyResizeHandle, + getResizeHandle, + type ResizeHandle, +} from './builderResize'; const STORAGE_KEY = 'placeholderer:builder'; const HISTORY_LIMIT = 5; @@ -81,7 +87,37 @@ export function UIBuilder() { const canvasRef = useRef(null); const containerRef = useRef(null); - const isInteracting = useRef<{ mode: 'move' | 'resize' | null; resizeHandle?: string; offsetX: number; offsetY: number; startW: number; startH: number; preState: BuilderState | null }>({ mode: null, offsetX: 0, offsetY: 0, startW: 0, startH: 0, preState: null }); + // Gesture state for move/resize. `didDrag` stays false until the + // pointer moves past DRAG_THRESHOLD_PX so a plain shift-click (no + // drag) never mutates geometry or pushes undo history. + const isInteracting = useRef<{ + mode: 'move' | 'resize' | null; + resizeHandle?: ResizeHandle; + offsetX: number; + offsetY: number; + /** Pointer position at mousedown (canvas coords). */ + startMx: number; + startMy: number; + /** Layer geometry at mousedown — resize is always computed from this. */ + originX: number; + originY: number; + originW: number; + originH: number; + preState: BuilderState | null; + didDrag: boolean; + }>({ + mode: null, + offsetX: 0, + offsetY: 0, + startMx: 0, + startMy: 0, + originX: 0, + originY: 0, + originW: 0, + originH: 0, + preState: null, + didDrag: false, + }); // Persist on every state change useEffect(() => { saveToStorage(state); }, [state]); @@ -405,6 +441,22 @@ export function UIBuilder() { return { x: e.clientX - rect.left, y: e.clientY - rect.top }; }; + const clearInteraction = () => { + isInteracting.current = { + mode: null, + offsetX: 0, + offsetY: 0, + startMx: 0, + startMy: 0, + originX: 0, + originY: 0, + originW: 0, + originH: 0, + preState: null, + didDrag: false, + }; + }; + const handleMouseDown = (e: React.MouseEvent) => { const { x: mx, y: my } = getMouse(e); if (e.shiftKey && selectedId) { @@ -412,7 +464,20 @@ export function UIBuilder() { if (sel) { const handle = getResizeHandle(sel, mx, my); if (handle) { - isInteracting.current = { mode: 'resize', resizeHandle: handle, offsetX: 0, offsetY: 0, startW: sel.width ?? 0, startH: sel.height ?? 0, preState: state }; + isInteracting.current = { + mode: 'resize', + resizeHandle: handle, + offsetX: 0, + offsetY: 0, + startMx: mx, + startMy: my, + originX: sel.x ?? 0, + originY: sel.y ?? 0, + originW: sel.width ?? 0, + originH: sel.height ?? 0, + preState: state, + didDrag: false, + }; return; } } @@ -428,15 +493,29 @@ export function UIBuilder() { mode: 'move', offsetX: mx - (hit.x ?? 0), offsetY: my - (hit.y ?? 0), - startW: hit.width ?? 0, - startH: hit.height ?? 0, + startMx: mx, + startMy: my, + originX: hit.x ?? 0, + originY: hit.y ?? 0, + originW: hit.width ?? 0, + originH: hit.height ?? 0, preState: state, + didDrag: false, }; }; const handleMouseMove = (e: React.MouseEvent) => { const { x: mx, y: my } = getMouse(e); const interact = isInteracting.current; + if (!interact.mode) return; + + // Ignore micro-jitter / shift-click without a real drag so we + // don't collapse a layer to the min size or nudge it by snap. + if (!interact.didDrag) { + const dist = Math.hypot(mx - interact.startMx, my - interact.startMy); + if (dist < DRAG_THRESHOLD_PX) return; + interact.didDrag = true; + } if (interact.mode === 'move' && selectedId) { const newX = snap(mx - interact.offsetX); @@ -448,26 +527,16 @@ export function UIBuilder() { return; } if (interact.mode === 'resize' && selectedId && interact.resizeHandle) { - const sel = state.layers.find((l) => l.id === selectedId); - if (!sel) return; - let { x, y, width, height } = { x: sel.x ?? 0, y: sel.y ?? 0, width: sel.width ?? 0, height: sel.height ?? 0 }; - const handle = interact.resizeHandle; - if (handle === 'left' || handle === 'corner') { - const newRight = x + width; - x = Math.min(snap(mx), newRight - 8); - width = newRight - x; - } - if (handle === 'right' || handle === 'corner') { - width = Math.max(8, snap(mx) - x); - } - if (handle === 'top' || handle === 'corner') { - const newBottom = y + height; - y = Math.min(snap(my), newBottom - 8); - height = newBottom - y; - } - if (handle === 'bottom' || handle === 'corner') { - height = Math.max(8, snap(my) - y); - } + const { x, y, width, height } = applyResizeHandle( + interact.resizeHandle, + interact.originX, + interact.originY, + interact.originW, + interact.originH, + mx, + my, + snap, + ); setState((s) => ({ ...s, layers: s.layers.map((l) => l.id === selectedId ? ({ ...l, x, y, width, height } as Layer) : l), @@ -476,7 +545,9 @@ export function UIBuilder() { }; const handleMouseUp = () => { - if (isInteracting.current.mode && isInteracting.current.preState) { + // Only commit undo history when the pointer actually dragged. + // A bare shift-click (or click) must leave the layer and history alone. + if (isInteracting.current.mode && isInteracting.current.preState && isInteracting.current.didDrag) { // Snapshot the PRE-gesture state into history so undo actually // restores the position the user started from, not the post-drag // position (which is what 'state' holds by now). @@ -487,7 +558,7 @@ export function UIBuilder() { }); setFuture([]); } - isInteracting.current = { mode: null, offsetX: 0, offsetY: 0, startW: 0, startH: 0, preState: null }; + clearInteraction(); }; const selectedLayer = state.layers.find((l) => l.id === selectedId) ?? null; @@ -690,21 +761,6 @@ export function UIBuilder() { ); } -function getResizeHandle(layer: Layer, mx: number, my: number): string | null { - const x = layer.x ?? 0, y = layer.y ?? 0, w = layer.width ?? 0, h = layer.height ?? 0; - const T = 8; - const nearLeft = Math.abs(mx - x) < T; - const nearRight = Math.abs(mx - (x + w)) < T; - const nearTop = Math.abs(my - y) < T; - const nearBottom = Math.abs(my - (y + h)) < T; - if ((nearLeft || nearRight) && (nearTop || nearBottom)) return 'corner'; - if (nearLeft) return 'left'; - if (nearRight) return 'right'; - if (nearTop) return 'top'; - if (nearBottom) return 'bottom'; - return null; -} - function download(blob: Blob, filename: string): void { const url = URL.createObjectURL(blob); const a = document.createElement('a'); diff --git a/apps/web/src/builderResize.ts b/apps/web/src/builderResize.ts new file mode 100644 index 0000000..0a4fd71 --- /dev/null +++ b/apps/web/src/builderResize.ts @@ -0,0 +1,86 @@ +// Pure resize geometry helpers for the UI Builder canvas. +// Kept free of React so unit tests and the builder share one path. + +import type { Layer } from '@placeholderer/schemas'; + +/** Min pointer travel (canvas px) before a gesture counts as a drag. */ +export const DRAG_THRESHOLD_PX = 3; + +/** Min layer size when resizing. */ +export const MIN_LAYER_SIZE = 8; + +export type ResizeHandle = + | 'left' + | 'right' + | 'top' + | 'bottom' + | 'nw' + | 'ne' + | 'sw' + | 'se'; + +/** + * Which resize handle is under the pointer (if any). Corners are + * distinct directions (nw/ne/sw/se) — a single 'corner' token used to + * apply left+right and top+bottom in one move, which collapsed the + * layer to MIN_LAYER_SIZE on a shift-click without drag. + */ +export function getResizeHandle(layer: Layer, mx: number, my: number): ResizeHandle | null { + const x = layer.x ?? 0, y = layer.y ?? 0, w = layer.width ?? 0, h = layer.height ?? 0; + const T = 8; + const nearLeft = Math.abs(mx - x) < T; + const nearRight = Math.abs(mx - (x + w)) < T; + const nearTop = Math.abs(my - y) < T; + const nearBottom = Math.abs(my - (y + h)) < T; + if (nearLeft && nearTop) return 'nw'; + if (nearRight && nearTop) return 'ne'; + if (nearLeft && nearBottom) return 'sw'; + if (nearRight && nearBottom) return 'se'; + if (nearLeft) return 'left'; + if (nearRight) return 'right'; + if (nearTop) return 'top'; + if (nearBottom) return 'bottom'; + return null; +} + +/** + * Compute new layer bounds for a resize gesture. + * Always derived from the geometry at mousedown (`origin*`) so + * intermediate snap steps don't accumulate. + */ +export function applyResizeHandle( + handle: ResizeHandle, + originX: number, + originY: number, + originW: number, + originH: number, + mx: number, + my: number, + snapFn: (n: number) => number = (n) => n, +): { x: number; y: number; width: number; height: number } { + const right = originX + originW; + const bottom = originY + originH; + let x = originX; + let y = originY; + let width = originW; + let height = originH; + + if (handle === 'left' || handle === 'nw' || handle === 'sw') { + x = Math.min(snapFn(mx), right - MIN_LAYER_SIZE); + width = right - x; + } + if (handle === 'right' || handle === 'ne' || handle === 'se') { + width = Math.max(MIN_LAYER_SIZE, snapFn(mx) - originX); + x = originX; + } + if (handle === 'top' || handle === 'nw' || handle === 'ne') { + y = Math.min(snapFn(my), bottom - MIN_LAYER_SIZE); + height = bottom - y; + } + if (handle === 'bottom' || handle === 'sw' || handle === 'se') { + height = Math.max(MIN_LAYER_SIZE, snapFn(my) - originY); + y = originY; + } + + return { x, y, width, height }; +}