Conversation
librowski
requested review from
lukasz-jazwa,
piotrblaszczyk and
szymon-t-sc
as code owners
September 7, 2026 12:05
librowski
marked this pull request as draft
September 7, 2026 12:07
…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
force-pushed
the
ds2-canvas-edges
branch
from
September 9, 2026 16:49
d6920b9 to
a62b3a9
Compare
librowski
marked this pull request as ready for review
September 14, 2026 06:41
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.
piotrblaszczyk
approved these changes
Sep 15, 2026
librowski
removed this pull request from stack #125
September 15, 2026 10:24
librowski
added this pull request to stack #151
September 15, 2026 10:24
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
LabelEdgecomputed a source-node height to place the label of a self-connecting edge but did not pass it toSelfConnectingEdge, whosenodeHeightprop defaulted to0: 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 fromsourceNode.height, which xyflow 12 sets only when a change explicitly requests attributes - normal ResizeObserver updates writemeasured.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
useSelfLoopNodeHeight(source, target)hook (exported):measured.height ?? height ?? 0,0for non-self edges. A one-line comment records the xyflow fact behind the order (measured.heighton every remeasure,heightonly when attributes are set explicitly).LabelEdgepasses the value toSelfConnectingEdgeso loop and label stay in lockstep.SelfConnectingEdgeitself now reads the same hook whennodeHeightis omitted, so a custom edge that delegates to it (asdashed-edge.tsxsuggests) no longer draws the loop for a zero-height node; the prop's JSDoc says when to pass a value.heighton a realistic{ measured: {}, height }node, unknown node, a regular edge never touchingnodeLookup, andSelfConnectingEdgewith and withoutnodeHeight.@workflowbuilder/sdkminor (SelfConnectingEdgechanges its default behaviour;useSelfLoopNodeHeightis 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_OFFSETis documented as the distance from the node's top edge, but the loop is offset from the port bynodeHeight + 100, so the apex sitsnodeHeight / 2 + 100above 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.