Skip to content

fix(frontend): place new nodes in nearest free space instead of overlapping#19

Open
LukasHirt wants to merge 1 commit into
mainfrom
fix/node-placement-collision
Open

fix(frontend): place new nodes in nearest free space instead of overlapping#19
LukasHirt wants to merge 1 commit into
mainfrom
fix/node-placement-collision

Conversation

@LukasHirt

Copy link
Copy Markdown
Collaborator

Summary

When adding a node via the "+" button, the new node's position was always computed as a fixed offset from its source node: { x: source.x + 260, y: source.y }. If a node already occupied (or nearly occupied) that spot — e.g. the user clicked "+" on the same source twice — the new node landed directly on top of the existing one instead of stacking below it.

Before

New node placement after a source node was a single fixed calculation with no awareness of existing nodes, so repeated "+" clicks on the same source stacked nodes exactly on top of each other.

After

Placement now goes through a new pure helper, computeNewNodePosition(existingNodes, sourceNode) in frontend/src/utils/nodeLayout.ts:

  • Starts at the same candidate spot (source.x + 260, source.y) for the common case.
  • If that spot is occupied (within a small bounding-box collision threshold based on the node card's min-width from styles/canvas.css and the existing 120px vertical spacing constant), it steps the candidate down by 120px and re-checks, cascading further down as needed until it finds a free slot.
  • Nodes that are far away (different x, or same x but well outside the collision threshold) never affect placement.
  • The no-source ("root add") branch now uses the same collision-avoidance logic for consistency, instead of relying solely on nodes.length.

onPickNodeType() in frontend/src/views/WorkflowBuilder.vue now delegates to this helper instead of doing the inline calculation.

Test plan

  • Wrote Vitest unit tests first in frontend/tests/unit/nodeLayout.spec.ts covering: no collision, one node occupying the candidate spot, two nodes already stacked (cascades two steps down), and unrelated far-away nodes not affecting placement — confirmed they failed before implementation (module didn't exist).
  • Implemented computeNewNodePosition and confirmed all 4 new tests pass.
  • npm run test:unit — full suite passes (3 files, 8 tests).
  • npm run check:types — no errors.
  • npm run lint — no errors.

Scoped only to this positioning logic — no changes to the "+" button's CSS/alignment, node categorization, the config modal, or flow validation.

🤖 Generated with Claude Code

@LukasHirt
LukasHirt requested a review from a team as a code owner July 24, 2026 16:02
@LukasHirt LukasHirt self-assigned this Jul 24, 2026
mzner
mzner previously approved these changes Jul 24, 2026
…apping

Extract the "+" button's new-node position calculation into a pure,
testable computeNewNodePosition() helper that checks existing node
positions and cascades downward in fixed steps until it finds a slot
that doesn't overlap another node, instead of always placing the node
at a fixed offset from its source.

Signed-off-by: Lukas Hirt <info@hirt.cz>
@LukasHirt
LukasHirt force-pushed the fix/node-placement-collision branch from 494c8fa to 8c02aac Compare July 24, 2026 20:46
@mzner
mzner self-requested a review July 24, 2026 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants