From 81ac2283c618411e5158010d1dcdc077bd039b6d Mon Sep 17 00:00:00 2001 From: David Montero Crespo Date: Tue, 12 May 2026 18:03:32 -0300 Subject: [PATCH] fix(canvas): wires follow components on rotation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rotating a part with the 90° button used to leave every wire pinned to the pre-rotation pixel coordinates — the component visually unhooked from its cables. Two paths were missing: 1. useSimulatorStore.updateComponent only triggered updateWirePositions for x/y changes. A rotation went through properties.rotation, so wires never recomputed. 2. calculatePinPosition didn't know about rotation. Even when called, it returned the unrotated offset, so the new endpoints would still have been wrong. 3. recordRotate (undo/redo) skipped updateWirePositions on both legs, so Ctrl+Z after a rotate left the canvas inconsistent. Fix: - calculatePinPosition gets a 5th `rotation` argument. When non-zero, it finds the .dynamic-component-wrapper ancestor in the DOM, reads its offsetWidth/Height (layout-only, immune to CSS transforms) to locate the wrapper centre, and applies a 2D rotation matrix around that pivot. The wrapper top-left is recovered as (componentX - 4, componentY - 6) to match the offset convention updateWirePositions already uses. - updateWirePositions and recalculateAllWirePositions read the per- component rotation and thread it through. - updateComponent recomputes wires whenever properties.rotation changes, mirroring the existing x/y path. - recordRotate.execute and .undo both call updateWirePositions so Ctrl+Z keeps the canvas coherent. Tests (pin-position-rotation.test.ts, 6 cases): unrotated identity, 90° (left edge → bottom), 180° (point reflection), 360° round-trip, negative angles, and a store-level integration that rotates a fake component and asserts wires[0].start moves to the rotated coordinate. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../__tests__/pin-position-rotation.test.ts | 210 ++++++++++++++++++ frontend/src/store/useSimulatorStore.ts | 40 +++- frontend/src/utils/pinPositionCalculator.ts | 52 ++++- 3 files changed, 290 insertions(+), 12 deletions(-) create mode 100644 frontend/src/__tests__/pin-position-rotation.test.ts diff --git a/frontend/src/__tests__/pin-position-rotation.test.ts b/frontend/src/__tests__/pin-position-rotation.test.ts new file mode 100644 index 00000000..2ec9982d --- /dev/null +++ b/frontend/src/__tests__/pin-position-rotation.test.ts @@ -0,0 +1,210 @@ +// @vitest-environment jsdom +/** + * Regression test for the rotation bug: when a component rotates, its + * wire endpoints stayed at the unrotated pin positions and the part + * visually disconnected from its cables. + * + * Root cause was twofold: + * 1. `useSimulatorStore.updateComponent()` only triggered + * `updateWirePositions()` when x or y changed — never on rotation. + * 2. `calculatePinPosition()` knew nothing about rotation; it returned + * the unrotated offset even when the wrapper had a CSS transform. + * + * Both paths are now exercised here: + * - calculatePinPosition with rotation arg returns the correct rotated + * coordinate for a known wrapper + pin layout. + * - rotating a component through the store re-stamps wire endpoints. + */ +import { describe, it, expect, beforeEach } from 'vitest'; +import { calculatePinPosition } from '../utils/pinPositionCalculator'; +import { useSimulatorStore } from '../store/useSimulatorStore'; + +// Build a minimal DOM matching the runtime layout: +//
← what CSS rotates +//
+// ← .pinInfo carrier +//
+//
+function buildFakeComponent(opts: { + id: string; + wrapperW: number; + wrapperH: number; + pins: Array<{ name: string; x: number; y: number }>; +}) { + const wrapper = document.createElement('div'); + wrapper.className = 'dynamic-component-wrapper'; + // jsdom defaults offsetWidth / Height to 0; set them manually so the + // rotation math sees a real layout box. + Object.defineProperty(wrapper, 'offsetWidth', { value: opts.wrapperW, configurable: true }); + Object.defineProperty(wrapper, 'offsetHeight', { value: opts.wrapperH, configurable: true }); + + const container = document.createElement('div'); + container.className = 'web-component-container'; + + const inner = document.createElement('div'); + inner.id = opts.id; + (inner as any).pinInfo = opts.pins; + + container.appendChild(inner); + wrapper.appendChild(container); + document.body.appendChild(wrapper); + return { wrapper, inner }; +} + +describe('calculatePinPosition — rotation math', () => { + beforeEach(() => { + document.body.innerHTML = ''; + }); + + it('returns the unrotated offset when rotation is 0', () => { + buildFakeComponent({ + id: 'comp0', + wrapperW: 72, + wrapperH: 48, + pins: [{ name: 'A', x: 0, y: 14 }], + }); + // componentX/Y are the inner-element top-left after the +4/+6 wrapper + // offset that updateWirePositions applies. With component.x = 100 → + // componentX = 104. + const pos = calculatePinPosition('comp0', 'A', 104, 106, 0); + expect(pos).toEqual({ x: 104, y: 120 }); + }); + + it('rotates a left-side pin to the bottom when rotation = 90°', () => { + // Wrapper 72×48 (small 2-input gate). Pin A at (0, 14) on the LEFT + // edge. After a 90° CW rotation around the wrapper centre, that pin + // should land on the bottom of the wrapper. + buildFakeComponent({ + id: 'comp90', + wrapperW: 72, + wrapperH: 48, + pins: [{ name: 'A', x: 0, y: 14 }], + }); + const pos = calculatePinPosition('comp90', 'A', 104, 106, 90); + expect(pos).not.toBeNull(); + // Walk through the math to keep the assertion expressive: + // wrapperLeft = 104 - 4 = 100 + // wrapperTop = 106 - 6 = 100 + // pivot = (100 + 36, 100 + 24) = (136, 124) + // unrotated = (104 + 0, 106 + 14) = (104, 120) + // dx, dy = (-32, -4) + // 90° → (dx*0 - dy*1, dx*1 + dy*0) = (4, -32) + // result = (136 + 4, 124 - 32) = (140, 92) + expect(pos!.x).toBeCloseTo(140, 5); + expect(pos!.y).toBeCloseTo(92, 5); + }); + + it('rotates 180° flips both axes around the wrapper centre', () => { + buildFakeComponent({ + id: 'comp180', + wrapperW: 72, + wrapperH: 48, + pins: [{ name: 'Y', x: 72, y: 24 }], + }); + // unrotated Y is on the right edge midpoint + // wrapperLeft = 100, wrapperTop = 100, pivot = (136, 124) + // unrotated = (104+72, 106+24) = (176, 130) + // dx, dy = (40, 6) + // 180° → (-40, -6) → (96, 118) + const pos = calculatePinPosition('comp180', 'Y', 104, 106, 180); + expect(pos!.x).toBeCloseTo(96, 5); + expect(pos!.y).toBeCloseTo(118, 5); + }); + + it('rotating four 90° steps lands back on the original coordinates', () => { + buildFakeComponent({ + id: 'comp360', + wrapperW: 72, + wrapperH: 48, + pins: [{ name: 'Y', x: 72, y: 24 }], + }); + const base = calculatePinPosition('comp360', 'Y', 104, 106, 0); + const full = calculatePinPosition('comp360', 'Y', 104, 106, 360); + expect(full!.x).toBeCloseTo(base!.x, 5); + expect(full!.y).toBeCloseTo(base!.y, 5); + }); + + it('accepts negative angles', () => { + buildFakeComponent({ + id: 'compNeg', + wrapperW: 72, + wrapperH: 48, + pins: [{ name: 'A', x: 0, y: 14 }], + }); + // -90° (= 270°) sends a left-edge pin to the TOP of the wrapper. + // dx, dy = (-32, -4) + // -90° → (-dy, dx) = (4, -32) → wait, that's +90°. Let's check. + // Standard 2D rotation matrix [cos -sin; sin cos] with θ=-90° + // cos = 0, sin = -1 + // (dx*0 - dy*(-1), dx*(-1) + dy*0) = (dy, -dx) = (-4, 32) + // result = (136 - 4, 124 + 32) = (132, 156) + const pos = calculatePinPosition('compNeg', 'A', 104, 106, -90); + expect(pos!.x).toBeCloseTo(132, 5); + expect(pos!.y).toBeCloseTo(156, 5); + }); +}); + +describe('useSimulatorStore — rotating a component re-stamps wires', () => { + beforeEach(() => { + document.body.innerHTML = ''; + // Reset the store to a clean slate. Boards aren't needed for this test; + // we add the component directly. + useSimulatorStore.setState((s) => { + const ids = s.boards.map((b) => b.id); + ids.forEach((id) => s.removeBoard(id)); + return s; + }); + useSimulatorStore.setState({ + components: [], + wires: [], + }); + }); + + it('updateComponent({rotation}) recomputes wire endpoints', () => { + // Set up a gate-shaped component plus a wire anchored to its A pin. + buildFakeComponent({ + id: 'g1', + wrapperW: 72, + wrapperH: 48, + pins: [ + { name: 'A', x: 0, y: 14 }, + { name: 'B', x: 0, y: 34 }, + { name: 'Y', x: 72, y: 24 }, + ], + }); + + useSimulatorStore.setState({ + components: [ + { + id: 'g1', + metadataId: 'logic-gate-and', + x: 100, + y: 100, + properties: {}, + }, + ], + wires: [ + { + id: 'w1', + start: { componentId: 'g1', pinName: 'A', x: 104, y: 120 }, + end: { componentId: 'g1', pinName: 'A', x: 104, y: 120 }, + color: '#000', + waypoints: [], + }, + ], + }); + + // Initial position should match the unrotated math. + const before = useSimulatorStore.getState().wires[0]; + expect(before.start.x).toBe(104); + expect(before.start.y).toBe(120); + + // Now rotate 90° via updateComponent — the wire should follow. + useSimulatorStore.getState().updateComponent('g1', { + properties: { rotation: 90 }, + } as any); + const after = useSimulatorStore.getState().wires[0]; + expect(after.start.x).toBeCloseTo(140, 5); + expect(after.start.y).toBeCloseTo(92, 5); + }); +}); diff --git a/frontend/src/store/useSimulatorStore.ts b/frontend/src/store/useSimulatorStore.ts index fd3025e7..84d77368 100644 --- a/frontend/src/store/useSimulatorStore.ts +++ b/frontend/src/store/useSimulatorStore.ts @@ -1839,7 +1839,13 @@ export const useSimulatorStore = create((set, get) => { set((state) => ({ components: state.components.map((c) => (c.id === id ? { ...c, ...updates } : c)), })); - if (updates.x !== undefined || updates.y !== undefined) { + // Re-stamp wire endpoints when the geometry of the component changes: + // position (x/y) OR rotation. Without this, rotating a component + // leaves every wire anchored to the pre-rotation pin positions, so + // the part visually disconnects from its cables. + const rotationChanged = + updates.properties && 'rotation' in updates.properties; + if (updates.x !== undefined || updates.y !== undefined || rotationChanged) { get().updateWirePositions(id); } }, @@ -1947,15 +1953,21 @@ export const useSimulatorStore = create((set, get) => { // Boards are rendered directly without a wrapper, so no offset. const compX = component ? component.x + 4 : board ? board.x : state.boardPosition.x; const compY = component ? component.y + 6 : board ? board.y : state.boardPosition.y; + // Boards never rotate; components carry their angle in properties.rotation. + const rotation = component ? Number(component.properties?.rotation) || 0 : 0; const updatedWires = state.wires.map((wire) => { const updated = { ...wire }; if (wire.start.componentId === componentId) { - const pos = calculatePinPosition(componentId, wire.start.pinName, compX, compY); + const pos = calculatePinPosition( + componentId, wire.start.pinName, compX, compY, rotation, + ); if (pos) updated.start = { ...wire.start, x: pos.x, y: pos.y }; } if (wire.end.componentId === componentId) { - const pos = calculatePinPosition(componentId, wire.end.pinName, compX, compY); + const pos = calculatePinPosition( + componentId, wire.end.pinName, compX, compY, rotation, + ); if (pos) updated.end = { ...wire.end, x: pos.x, y: pos.y }; } return updated; @@ -1982,11 +1994,13 @@ export const useSimulatorStore = create((set, get) => { : startBoard ? startBoard.y : state.boardPosition.y; + const startRotation = startComp ? Number(startComp.properties?.rotation) || 0 : 0; const startPos = calculatePinPosition( wire.start.componentId, wire.start.pinName, startX, startY, + startRotation, ); updated.start = startPos ? { ...wire.start, x: startPos.x, y: startPos.y } @@ -1997,7 +2011,10 @@ export const useSimulatorStore = create((set, get) => { const endBoard = state.boards.find((b) => b.id === wire.end.componentId); const endX = endComp ? endComp.x + 4 : endBoard ? endBoard.x : state.boardPosition.x; const endY = endComp ? endComp.y + 6 : endBoard ? endBoard.y : state.boardPosition.y; - const endPos = calculatePinPosition(wire.end.componentId, wire.end.pinName, endX, endY); + const endRotation = endComp ? Number(endComp.properties?.rotation) || 0 : 0; + const endPos = calculatePinPosition( + wire.end.componentId, wire.end.pinName, endX, endY, endRotation, + ); updated.end = endPos ? { ...wire.end, x: endPos.x, y: endPos.y } : { ...wire.end, x: endX, y: endY }; @@ -2151,22 +2168,29 @@ export const useSimulatorStore = create((set, get) => { get().pushCommand( { description: 'Rotate component', - execute: () => + execute: () => { set((s) => ({ components: s.components.map((c) => c.id === id ? { ...c, properties: { ...c.properties, rotation: nextRotation } } : c, ), - })), - undo: () => + })); + // Wires must follow the part on undo / redo too, otherwise a + // Ctrl+Z after a rotate would re-show the post-rotation pin + // positions against the now-restored unrotated component. + get().updateWirePositions(id); + }, + undo: () => { set((s) => ({ components: s.components.map((c) => c.id === id ? { ...c, properties: { ...c.properties, rotation: prevRotation } } : c, ), - })), + })); + get().updateWirePositions(id); + }, }, { applyNow: false }, ); diff --git a/frontend/src/utils/pinPositionCalculator.ts b/frontend/src/utils/pinPositionCalculator.ts index 1239ebfa..09329cbf 100644 --- a/frontend/src/utils/pinPositionCalculator.ts +++ b/frontend/src/utils/pinPositionCalculator.ts @@ -17,8 +17,14 @@ * * @param componentId - The DOM ID of the component element * @param pinName - The name of the pin (e.g., 'A', 'C', 'GND.1', '13') - * @param componentX - Component's X position on canvas (pixels) + * @param componentX - Component's X position on canvas (pixels). For a + * rotated component this is still the UNROTATED inner-element top-left + * (callers already add the wrapper offset of 4 horizontal / 6 vertical). * @param componentY - Component's Y position on canvas (pixels) + * @param rotation - Optional CSS rotation in degrees applied to the + * component's wrapper (0 / 90 / 180 / 270). When non-zero the pin + * position is rotated around the wrapper's center so wire endpoints + * land on the visually-rotated pin instead of the old layout-space pin. * @returns Absolute canvas coordinates { x, y } or null if pin not found */ export function calculatePinPosition( @@ -26,6 +32,7 @@ export function calculatePinPosition( pinName: string, componentX: number, componentY: number, + rotation: number = 0, ): { x: number; y: number } | null { // Get the DOM element const element = document.getElementById(componentId); @@ -64,9 +71,46 @@ export function calculatePinPosition( return null; } - // Pin coordinates are already in CSS pixels, just add component position - const pinX = componentX + pin.x; - const pinY = componentY + pin.y; + // Unrotated pin position in canvas space. + let pinX = componentX + pin.x; + let pinY = componentY + pin.y; + + // Rotation: the DynamicComponent wrapper applies + // transform: rotate(deg); transform-origin: center center; + // around its OWN center (the wrapper, not the inner web component). So + // when the user rotates 90° the pin moves on an arc centered on the + // wrapper center, not on the component origin or the pin's own axis. + // + // We compute the wrapper center in canvas space from `offsetWidth / + // offsetHeight` of the wrapper — those reflect the layout box and are + // UNAFFECTED by CSS transforms, so reading them right after a state + // change but before React commits the new transform is safe. + // + // Wrapper top-left ≈ inner-element top-left minus the wrapper padding + // + border. updateWirePositions / recalculateAllWirePositions add + // (+4, +6) to component.x / component.y to land on the inner-element + // top-left, so the wrapper top-left is (componentX - 4, componentY - 6). + // This convention is hardcoded in the store; we honour it here so the + // math stays consistent across the rotation boundary. + const angle = ((rotation % 360) + 360) % 360; + if (angle !== 0) { + const wrapper = element.closest('.dynamic-component-wrapper') as HTMLElement | null; + if (wrapper) { + const wrapperW = wrapper.offsetWidth; + const wrapperH = wrapper.offsetHeight; + const wrapperLeft = componentX - 4; + const wrapperTop = componentY - 6; + const pivotX = wrapperLeft + wrapperW / 2; + const pivotY = wrapperTop + wrapperH / 2; + const theta = (angle * Math.PI) / 180; + const cos = Math.cos(theta); + const sin = Math.sin(theta); + const dx = pinX - pivotX; + const dy = pinY - pivotY; + pinX = pivotX + (dx * cos - dy * sin); + pinY = pivotY + (dx * sin + dy * cos); + } + } return { x: pinX, y: pinY }; }