Skip to content

[DS 2.0] Use the measured source-node height for self-loop edges - #121

Open
librowski wants to merge 2 commits into
design-system-2.0from
ds2-canvas-edges
Open

librowski wants to merge 2 commits into
design-system-2.0from
ds2-canvas-edges

Conversation

@librowski

@librowski librowski commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

Problem

LabelEdge computed a source-node height to place the label of a self-connecting edge but did not pass it to SelfConnectingEdge, whose nodeHeight prop defaulted to 0: the loop was drawn as if the node had no height while the label was positioned as if it had. The height itself was read from sourceNode.height, which xyflow 12 sets only when a change explicitly requests attributes - normal ResizeObserver updates write measured.height.

A first version read the height through getNode(); a browser check (review of 8.09) showed that a snapshot read does not follow a later remeasurement: after adding a branch to a Decision node the loop and label stayed at the old height until an unrelated rerender.

Change

  • The source node's height is read from the React Flow store through a shared useSelfLoopNodeHeight(source, target) hook (exported): measured.height ?? height ?? 0, 0 for non-self edges. A one-line comment records the xyflow fact behind the order (measured.height on every remeasure, height only when attributes are set explicitly).
  • LabelEdge passes the value to SelfConnectingEdge so loop and label stay in lockstep. SelfConnectingEdge itself now reads the same hook when nodeHeight is omitted, so a custom edge that delegates to it (as dashed-edge.tsx suggests) no longer draws the loop for a zero-height node; the prop's JSDoc says when to pass a value.
  • Specs: measured height, fallback to an explicit height on a realistic { measured: {}, height } node, unknown node, a regular edge never touching nodeLookup, and SelfConnectingEdge with and without nodeHeight.
  • Changeset: @workflowbuilder/sdk minor (SelfConnectingEdge changes its default behaviour; useSelfLoopNodeHeight is a new export).

Not covered by an automated test: the live remeasurement itself (the spec exercises the selector with a mocked store; a real React Flow harness would be a new test layer).

Out of scope, to settle with design

SELF_CONNECTING_EDGE_LABEL_OFFSET is documented as the distance from the node's top edge, but the loop is offset from the port by nodeHeight + 100, so the apex sits nodeHeight / 2 + 100 above the top when ports are vertically centred. Whether that geometry is the requirement is a design question; the constant and its doc are unchanged here.

Edge corner radius has three values today: 16px in code (EDGE_CURVE_RADIUS), 2px in the token export (canvas-edge-corner-radius), 18px in the Figma vectors. Adopting the export would visibly straighten every corner; not changed here.

…and label

LabelEdge subscribes to the source node's height in the React Flow store
(measured.height, then height, then 0) instead of reading a getNode()
snapshot, so the loop path and its label follow every remeasurement -
xyflow 12 writes ResizeObserver results to measured.height only. The same
value is passed to SelfConnectingEdge, whose nodeHeight prop previously
defaulted to 0.
@librowski
librowski marked this pull request as ready for review September 14, 2026 06:41
Comment thread packages/sdk/src/features/diagram/edges/label-edge/label-edge.tsx Outdated
Comment thread packages/sdk/src/features/diagram/edges/label-edge/label-edge.tsx
Comment thread packages/sdk/src/features/diagram/edges/label-edge/label-edge.spec.tsx Outdated
Comment thread packages/sdk/src/features/diagram/edges/label-edge/label-edge.tsx
Review follow-up. The measured-height selector moves to a shared
useSelfLoopNodeHeight hook (exported), so SelfConnectingEdge reads the
source node's measured height when nodeHeight is omitted instead of
drawing the loop for a zero-height node; LabelEdge keeps passing its own
value so loop and label stay in lockstep. Specs cover the regular-edge
early exit, the realistic measured-but-empty fixture and both paths of
the new default.
@librowski
librowski removed this pull request from stack #125 September 15, 2026 10:24
@librowski
librowski added this pull request to stack #151 September 15, 2026 10:24
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