From 467ca4455f9357ea034dd5c225253f39d61d810d Mon Sep 17 00:00:00 2001 From: David Montero Date: Tue, 26 May 2026 15:15:50 +0200 Subject: [PATCH] =?UTF-8?q?fix(canvas):=20wires=20off=20pins=20after=20rot?= =?UTF-8?q?ation=20=E2=80=94=20wrapper=20offset=20was=20(4,6)=20instead=20?= =?UTF-8?q?of=20(6,6)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit User report: "rotating components messes up their connections" — pressing R on a placed component visibly slid every wire endpoint off its pin tip. Root cause: the DynamicComponent wrapper has padding:4px + border:2px on EVERY side, so the inner web-component element sits 6 px in from the wrapper top-left on BOTH axes. The wire layer assumed an asymmetric (4, 6) offset, baked into: * useSimulatorStore.updateWirePositions — store.x + 4, store.y + 6 * useSimulatorStore.recalculateAllWirePositions — start (startComp.x + 4, startComp.y + 6) — end (endComp.x + 4, endComp.y + 6) * pinPositionCalculator.calculatePinPosition — inverse: (componentX - 4, componentY - 6) Unrotated the 2 px X bias was visible only as a very-slightly-off wire, which nobody filed. When the user rotated the component, the bias rotated WITH it — at 90° it became a 2 px Y offset (wires hanging below the pin), at 180° a 2 px X offset on the other side, at 270° upward. UX read as "wires disconnected". Verified the real CSS box via chrome-devtools-mcp against several live components on velxio.dev (RGB LED + 3 resistors + analog joystick): all report padding-left/top = 4 px, border-left/top = 2 px, inner offset = 6 on both axes. Fix: use (+6, +6) at every site, single source of truth in a comment explaining padding+border arithmetic. Updated the rotation regression test to match the corrected math (numbers shift by 2 px on every expectation that referenced the old offset). Pin position math, pivot derivation and the rotate-N×90° round trip unchanged — only the offset constant moved. --- .../__tests__/pin-position-rotation.test.ts | 61 ++++++++++--------- frontend/src/store/useSimulatorStore.ts | 19 +++--- frontend/src/utils/pinPositionCalculator.ts | 16 +++-- 3 files changed, 53 insertions(+), 43 deletions(-) diff --git a/frontend/src/__tests__/pin-position-rotation.test.ts b/frontend/src/__tests__/pin-position-rotation.test.ts index 2ec9982d..e70bb219 100644 --- a/frontend/src/__tests__/pin-position-rotation.test.ts +++ b/frontend/src/__tests__/pin-position-rotation.test.ts @@ -63,11 +63,11 @@ describe('calculatePinPosition — rotation math', () => { 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 }); + // componentX/Y are the inner-element top-left after the +6/+6 + // wrapper offset that updateWirePositions applies. With + // component.x = 100 → componentX = 106. + const pos = calculatePinPosition('comp0', 'A', 106, 106, 0); + expect(pos).toEqual({ x: 106, y: 120 }); }); it('rotates a left-side pin to the bottom when rotation = 90°', () => { @@ -80,18 +80,18 @@ describe('calculatePinPosition — rotation math', () => { wrapperH: 48, pins: [{ name: 'A', x: 0, y: 14 }], }); - const pos = calculatePinPosition('comp90', 'A', 104, 106, 90); + const pos = calculatePinPosition('comp90', 'A', 106, 106, 90); expect(pos).not.toBeNull(); // Walk through the math to keep the assertion expressive: - // wrapperLeft = 104 - 4 = 100 + // wrapperLeft = 106 - 6 = 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) + // unrotated = (106 + 0, 106 + 14) = (106, 120) + // dx, dy = (-30, -4) + // 90° → (dx*0 - dy*1, dx*1 + dy*0) = (4, -30) + // result = (136 + 4, 124 - 30) = (140, 94) expect(pos!.x).toBeCloseTo(140, 5); - expect(pos!.y).toBeCloseTo(92, 5); + expect(pos!.y).toBeCloseTo(94, 5); }); it('rotates 180° flips both axes around the wrapper centre', () => { @@ -103,11 +103,11 @@ describe('calculatePinPosition — rotation math', () => { }); // 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); + // unrotated = (106+72, 106+24) = (178, 130) + // dx, dy = (42, 6) + // 180° → (-42, -6) → (94, 118) + const pos = calculatePinPosition('comp180', 'Y', 106, 106, 180); + expect(pos!.x).toBeCloseTo(94, 5); expect(pos!.y).toBeCloseTo(118, 5); }); @@ -118,8 +118,8 @@ describe('calculatePinPosition — rotation math', () => { 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); + const base = calculatePinPosition('comp360', 'Y', 106, 106, 0); + const full = calculatePinPosition('comp360', 'Y', 106, 106, 360); expect(full!.x).toBeCloseTo(base!.x, 5); expect(full!.y).toBeCloseTo(base!.y, 5); }); @@ -132,15 +132,14 @@ describe('calculatePinPosition — rotation math', () => { 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. + // dx, dy = (-30, -4) // 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); + // (dx*0 - dy*(-1), dx*(-1) + dy*0) = (dy, -dx) = (-4, 30) + // result = (136 - 4, 124 + 30) = (132, 154) + const pos = calculatePinPosition('compNeg', 'A', 106, 106, -90); expect(pos!.x).toBeCloseTo(132, 5); - expect(pos!.y).toBeCloseTo(156, 5); + expect(pos!.y).toBeCloseTo(154, 5); }); }); @@ -186,17 +185,19 @@ describe('useSimulatorStore — rotating a component re-stamps wires', () => { wires: [ { id: 'w1', - start: { componentId: 'g1', pinName: 'A', x: 104, y: 120 }, - end: { componentId: 'g1', pinName: 'A', x: 104, y: 120 }, + start: { componentId: 'g1', pinName: 'A', x: 106, y: 120 }, + end: { componentId: 'g1', pinName: 'A', x: 106, y: 120 }, color: '#000', waypoints: [], }, ], }); - // Initial position should match the unrotated math. + // Initial position should match the unrotated math (component.x=100 + // plus +6 wrapper offset, plus pin.x=0 → 106 for X; same on Y plus + // pin.y=14 → 120). const before = useSimulatorStore.getState().wires[0]; - expect(before.start.x).toBe(104); + expect(before.start.x).toBe(106); expect(before.start.y).toBe(120); // Now rotate 90° via updateComponent — the wire should follow. @@ -205,6 +206,6 @@ describe('useSimulatorStore — rotating a component re-stamps wires', () => { } as any); const after = useSimulatorStore.getState().wires[0]; expect(after.start.x).toBeCloseTo(140, 5); - expect(after.start.y).toBeCloseTo(92, 5); + expect(after.start.y).toBeCloseTo(94, 5); }); }); diff --git a/frontend/src/store/useSimulatorStore.ts b/frontend/src/store/useSimulatorStore.ts index d78ddddb..9e0ec36c 100644 --- a/frontend/src/store/useSimulatorStore.ts +++ b/frontend/src/store/useSimulatorStore.ts @@ -2142,9 +2142,13 @@ export const useSimulatorStore = create((set, get) => { const component = state.components.find((c) => c.id === componentId); // Check if this componentId matches a board id const board = state.boards.find((b) => b.id === componentId); - // Components have a DynamicComponent wrapper with border:2px + padding:4px → offset (4,6) - // Boards are rendered directly without a wrapper, so no offset. - const compX = component ? component.x + 4 : board ? board.x : state.boardPosition.x; + // Components have a DynamicComponent wrapper with border:2px + + // padding:4px on EVERY side → inner element sits at (+6, +6) + // from the wrapper top-left. Earlier code used (+4, +6) — the + // 2 px X bias rotated visibly with the component and looked + // like wires came off the pins when rotated. Boards are + // rendered directly without that wrapper, so no offset. + const compX = component ? component.x + 6 : 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; @@ -2174,11 +2178,12 @@ export const useSimulatorStore = create((set, get) => { const updatedWires = state.wires.map((wire) => { const updated = { ...wire }; - // Resolve start — components have wrapper offset (4,6), boards do not + // Resolve start — components have wrapper offset (6,6) on + // both axes (padding:4 + border:2). Boards have no wrapper. const startComp = state.components.find((c) => c.id === wire.start.componentId); const startBoard = state.boards.find((b) => b.id === wire.start.componentId); const startX = startComp - ? startComp.x + 4 + ? startComp.x + 6 : startBoard ? startBoard.x : state.boardPosition.x; @@ -2199,10 +2204,10 @@ export const useSimulatorStore = create((set, get) => { ? { ...wire.start, x: startPos.x, y: startPos.y } : { ...wire.start, x: startX, y: startY }; - // Resolve end — components have wrapper offset (4,6), boards do not + // Resolve end — same (6,6) wrapper offset as start above. const endComp = state.components.find((c) => c.id === wire.end.componentId); const endBoard = state.boards.find((b) => b.id === wire.end.componentId); - const endX = endComp ? endComp.x + 4 : endBoard ? endBoard.x : state.boardPosition.x; + const endX = endComp ? endComp.x + 6 : endBoard ? endBoard.x : state.boardPosition.x; const endY = endComp ? endComp.y + 6 : endBoard ? endBoard.y : state.boardPosition.y; const endRotation = endComp ? Number(endComp.properties?.rotation) || 0 : 0; const endPos = calculatePinPosition( diff --git a/frontend/src/utils/pinPositionCalculator.ts b/frontend/src/utils/pinPositionCalculator.ts index 704db6ce..a90eda91 100644 --- a/frontend/src/utils/pinPositionCalculator.ts +++ b/frontend/src/utils/pinPositionCalculator.ts @@ -96,18 +96,22 @@ export function calculatePinPosition( // 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. + // + border. The DynamicComponent wrapper has padding:4px + border:2px + // on EVERY side → the inner element sits 6 px in from the wrapper on + // both axes. updateWirePositions / recalculateAllWirePositions add + // (+6, +6) to component.x / component.y to land on the inner-element + // top-left, so the wrapper top-left is (componentX - 6, componentY - 6). + // The earlier code used (+4, +6) — a 2 px X bias that was invisible + // unrotated (a wire endpoint two pixels off looks fine) but rotated + // with the component, surfacing as an obvious "wires disconnected + // from the pin" once the user pressed R. 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 wrapperLeft = componentX - 6; const wrapperTop = componentY - 6; const pivotX = wrapperLeft + wrapperW / 2; const pivotY = wrapperTop + wrapperH / 2;