Skip to content

fix(vscode): apply a routing change made during a drag when the drag settles - #1330

Merged
HuiJun merged 7 commits into
developfrom
fix/diagram-routing-change-mid-drag
Oct 21, 2026
Merged

HuiJun merged 7 commits into
developfrom
fix/diagram-routing-change-mid-drag

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

What and why

Changing opensysml.diagram.routing while a drag is in progress left the old routes on screen after the drag ended. The routing message handler in editors/vscode/src/webview/diagram.ts rightly declines to redraw under a held pointer, but settle() re-lays the drawing out only when unrouted is set, and only the router's own late load set it. So a routing change during a gesture was recorded (applyRouting updated routing) and then forgotten: a release-as-click or an Escape cancel put the pre-change layout back, and the drawing stayed stale until some later render. The pan gesture (a press on empty canvas) ends before settle() is reached at all, so the same held there.

Reproduction on develop (the sequence the new tests drive):

  1. render with routing: "model" on a connection with a stated DiagramLayout::Route — the diagonal is drawn verbatim with its waypoint handle.
  2. pointerdown on a node (a gesture begins; nothing has moved).
  3. routing: "router" arrives — the handler sees gesture and does not redraw (correct), routing is now router.
  4. pointerup without movement → endGesture → unrouted is false → settle() is skipped → the model-routed drawing with its waypoint handle is still on screen. Same with Escape instead of release (cancelGesture → settle() only when moved || unrouted), and with the press on empty canvas instead of a node (the pan branch of endGesture returns before settle()).

Fix: applyRouting now marks the layout on screen as predating the routing change (unrouted = true) when a gesture holds the screen, so the gesture's end re-lays out through the existing settle() path; the pan branch of endGesture settles an unrouted layout like the other gestures do. Every gesture records the routing it began under, and panelOverrides() reads that while a gesture is in progress, so a drag's preview stays on the layout the drag started from rather than switching modes under the held node. panTo records the viewport as it scrolls the container (the scroll listener hears of it a task later than the pan's end might redraw), so a redraw at a pan's end, released or cancelled, puts the pan's offsets back; a drag's preview is laid out on the canvas the drag began on (layoutCanvas with held covers the held layout's canvas), so a preview that draws less no longer shrinks the scrollable canvas and clamps the offsets under the pointer, and a cancelled drag comes back where it was. unrouted's comment now says "the router loaded or the routing changed mid-gesture". No behaviour changes when the routing message carries the routing already in force (applyRouting returns before touching anything, as before) or when no gesture is in progress (the handler redraws at once, as before).

The panel's markup (the controls and regions its script binds by id) moves from the html template in editors/vscode/src/diagram.ts to editors/vscode/src/markup.ts, which the template and the new test both read, so the test loads the script into the markup the extension serves rather than a copy of it.

In-flight overlap

Open PRs touching editors/vscode/src/webview: #1320 (related elements), #1328 (api-json bridge; its webview delta is develop's own code from a stale base). Neither touches applyRouting, settle, unrouted or the routing message handler. #1318 (diagram navigation) merged into develop while this PR was open and is merged in here; its pan gesture is covered by the fix. PR-less remote branches touching the webview: fix/webview-collapsed-ports (layout/canvas/autolayout, no change to diagram.ts) and docs/landing-runtime-environment-label (layout.test.ts only). No overlap with this change.

How it was verified

New editors/vscode/src/webview/diagram.test.ts loads the panel script into the markup the extension serves it in (jsdom, faked acquireVsCodeApi, libavoid loaded from node_modules) and drives it through the message and pointer events: a routing change during a drag is applied on release, on Escape, and at the end of a pan; a drag's preview keeps the routing it began under until the drag ends; a pan cancelled after a routing change is redrawn where it left the canvas; a drag's preview and the layout put back after Escape keep the canvas the drag began on (and a layoutCanvas case: held covers the held canvas, and still grows beyond it); and a routing message that leaves the routing as it is redraws nothing. The six regression tests fail without the fix (stale waypoint handles after the gesture, the preview switching modes mid-drag, scrollLeft back at 0, a preview canvas smaller than the held one) and pass with it. jsdom.d.ts gains the url and pretendToBeVisual constructor options: the panel's message handler accepts only its own origin, and its viewport work is scheduled in animation frames.

Extension gate (editors/vscode, as the VS Code extension CI job runs it), on the head merged with current develop:

npm run build       → exit 0
npm run typecheck   → exit 0
npm test            → ℹ tests 380 / pass 380 / fail 0
python3 scripts/changelog.py check → exit 0
python3 scripts/check-doc-ids.py   → "No internal work-item labels in reader-facing documentation"

No Go code is touched.

Checklist

  • make test and make lint pass locally — not applicable: no Go code touched; the extension's own gate (build, typecheck, test) passes
  • Tests added or updated for the change
  • Documentation extended where it already covers the surface (see CONTRIBUTING.md) — no documented surface changes
  • Changelog entry added as changes/unreleased/diagram-routing-change-mid-drag.fixed.md, not as an edit to CHANGELOG.md
  • baselines regenerated and make docs-counts run if a gate count moved — no gate count moved
  • No internal work-item labels (waves, slices, F4, K5) in the body, docs, or changelog

Link to Devin session: https://nasa-jpl-demo.devinenterprise.com/sessions/a63fa6cf47fa4ce29879754d525288bd
Open in Devin Desktop: https://nasa-jpl-demo.devinenterprise.com/desktop/session/a63fa6cf47fa4ce29879754d525288bd?variant=devin
Requested by: @HuiJun

…settles

Co-Authored-By: jason.han <hanhuijun@gmail.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

  • Disable automatic comment, CI, and merge conflict monitoring

devin-ai-integration Bot and others added 2 commits October 21, 2026 00:25
…g-change-mid-drag

Co-Authored-By: jason.han <hanhuijun@gmail.com>
…ttles

The panel's tests now load its script into the markup the extension serves,
shared through src/markup.ts, so the controls the script binds at load are the
served ones and not a copy that drifts from them.

Co-Authored-By: jason.han <hanhuijun@gmail.com>
devin-ai-integration[bot]

This comment was marked as resolved.

@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

Native VS Code end-to-end comparison of this head (f26e199) against develop (c5a59e1), each packaged and installed fresh, on the same model with a stated DiagramLayout::Route, with the pointer really held while opensysml.diagram.routing changed.

Gesture settlement: before and after

On the branch, a node release without movement, Escape, a stationary empty-canvas release and a moved-pan release all switch from the stated diagonal with its one waypoint handle to the router's orthogonal route with none. The webview's event log confirms the routing message arrived while the pointer was held, with no render in between.

Develop: stale after an empty-canvas release Branch: rerouted on release
Develop keeps the stated route after a stationary pan The branch reroutes on release

develop is stale in every settlement case. Its node-release case is masked ~78 ms later by the render the click's source reveal triggers; Escape and both pan cases stay stale.

Pan preservation and controls
  • At 195% zoom, a moved-pan release reroutes and keeps its scroll offsets (scrollLeft 417.5, scrollTop 0).
  • A routing change with no pointer held redraws at once on both builds.
  • A drag that moves a node saves the node's new position (67, 67) and the redraw after it follows the new routing; the connection's stated waypoint (250, 150) is untouched, on both builds.
  • Gestures that move nothing edit nothing: both final models are byte-identical to their fixtures.
  • Not exercised: vertical overflow in a moved pan.

…where it left the canvas

Co-Authored-By: jason.han <hanhuijun@gmail.com>
devin-ai-integration[bot]

This comment was marked as resolved.

… keeps its viewport

Co-Authored-By: jason.han <hanhuijun@gmail.com>
devin-ai-integration[bot]

This comment was marked as resolved.

… the scroll is not clamped

Co-Authored-By: jason.han <hanhuijun@gmail.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

Native VS Code run of dd25007 (VSIX rebuilt and installed; pointer really held; routing changed through the setting; no render between pointerdown and the routing message in any measured gesture).

Node drag whose preview draws a smaller canvas, cancelled with Escape — 195% zoom, the Target node dragged left from scrollLeft 354.1666564941406, model → router while held, moved again, Escape. scrollLeft at start, first move, routing message, second move, Escape and once settled: 354.1666564941406 every time; the SVG stayed 1082.03125 × 564.453125 (viewBox 0 0 554 289) throughout, and passive mutation/scroll sampling saw no clamp. Escape rerouted and left the document unchanged. On the previous head the preview shrank the canvas and the browser clamped scrollLeft to 65, which the redraw after Escape kept.

Held preview and the layout put back after Escape, both at the starting offsets

Same drag released — preview and offsets unchanged through pointerup; the saved edit moved Target to (229, 200) and left the Link's stated waypoint (250, 150) alone; the render of the saved edit then drew the smaller canvas (viewBox 0 0 383 289) and scrollLeft clamped to 125.83332824707031, as a committed layout may.

Held preview keeps its routing — model-start drag, model → router while held, second move: the stated diagonal and its waypoint handle stayed; Escape applied router routing and cancelled the edit. Moved pan + Escape at 381%: rerouted with both offsets kept at (983.3333129882812, 297.5), also once settled.

Recording of the run

@HuiJun
HuiJun merged commit b60a18c into develop Oct 21, 2026
21 checks passed
@HuiJun
HuiJun deleted the fix/diagram-routing-change-mid-drag branch October 21, 2026 06:30
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.

1 participant