Skip to content

diagram: adjustFlows valve-mirroring is non-idempotent for off-center valves on the min-coordinate endpoint #832

Description

@bpowers

Summary

The 2-point branch of adjustFlows in src/diagram/drawing/Flow.tsx (the block commented "Re-place the valve by mirroring it around the segment", ~lines 697-722) repositions an off-center valve by mirroring it across the segment midpoint rather than preserving its fractional position along the segment. This makes the operation non-idempotent: when the dragged endpoint is the smaller-coordinate end of the flow, an off-center valve jumps to its mirror position on the very first drag frame -- even for a zero or sub-pixel move.

Concrete repro

For a horizontal flow with:

  • source cloud at x = 100
  • stock sink edge at x = 277.5
  • valve at x = 200

Calling growEndpointDrag(flow, isSource=true, {x:0, y:0}) (or any tiny delta) returns the valve at x = 177.5 instead of 200 -- a ~22.5px jump the instant the user grabs the source cloud.

The mirror case (sink cloud at the larger coordinate) is idempotent and correctly stays at 200, so the defect is specific to the moved-endpoint-is-min-coordinate geometry (a SOURCE cloud on the left of a horizontal flow, or the TOP of a vertical flow).

Root cause

The formula computes base = min(otherEnd, movedEnd) then reconstructs the valve as base + |fraction * d|. When base lands on the moved (cloud) side rather than the anchor (otherEnd) side, the absolute-offset reconstruction reflects the valve across the segment midpoint instead of preserving its fractional position.

The mirroring was intentionally added so an off-center valve stays between the two ends when a drag flips the segment orientation (the moved end crossing past otherEnd; the guard/comment references the #818-adjacent NaN case). But it is wrong for the common non-flipping case: it should preserve the valve's fraction along the segment, not reflect it.

Impact

User-visible valve/label jump at the start of dragging a source cloud (or a top-anchored vertical flow's endpoint). Minor but real. Correctness/UX polish, not a crash.

Component

src/diagram/drawing/Flow.tsx -- adjustFlows 2-point branch (shared by growEndpointDrag / growInCreationFlow in flow-attach.ts).

Suggested fix direction

Make the 2-point valve reposition preserve the fractional position along the segment (like preserveValveFraction does for multi-segment flows) rather than mirroring, while:

Warrants its own change with dedicated tests, since adjustFlows also carries the along-axis and #818 NaN-guard behavior.

How discovered

Identified while fixing #53 (flow valve/label teleports when dragging a cloud perpendicular to form an L). This is a separate, pre-existing bug in the same file, out of scope for #53: #53's scope is the perpendicular-reroute teleport (a different code path), and touching the shared adjustFlows valve formula risks the along-axis and #818 NaN-guard behavior, warranting its own change.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions