fix(canvas): wires off pins after rotation — wrapper offset was (4,6) instead of (6,6)
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.
This commit is contained in:
David Montero
parent
161335a5cf
commit
467ca4455f
|
|
@ -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);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -2142,9 +2142,13 @@ export const useSimulatorStore = create<SimulatorState>((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<SimulatorState>((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<SimulatorState>((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(
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
Loading…
Reference in New Issue