DYN-9724: Allow Pins to Stay Until Connector is Actually Disconnected

[ivaylo-matov]

Purpose

This PR addresses DYN-9724

The changes in the code aim to improve the connector reconnection behavior so connector pins are preserved while a wire is being reconnected, instead of being dropped immediately when drag starts.

Changes:

• Reconnection + pin persistence
  • preserved pin locations during reconnection and reapplied them to newly created permanent connectors.
  • added centralized pin snapshot API on ConnectorModel: GetPinLocations()
  • reused this API in reconnection command flow to remove duplicated pin-location logic
• Transient connector behavior
  • during reconnect drag, pins are transferred off the original connector and cached on the transient connector for redraw
  • transient redraw uses cached pin locations (not transient connector pin VM ownership), keeping wire shape stable while dragging
  • added deferred redraw/batching for pin collection updates to reduce redundant redraws
• Test

Declarations

Check these if you believe they are true

• Is documented according to the standards
• The level of testing this PR includes is appropriate
• Changes to the API follow Semantic Versioning and are documented in the API Changes document.

Release Notes

Improved connector reconnection so pins are preserved through drag using non-interactive transient pins and restored on the final connector, with added safety hardening and dedicated test coverage.

Reviewers

@zeusongit
@DynamoDS/eidos

FYIs

@dnenov
@johnpierson

[github-actions]

See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-9724

[Copilot]

Pull request overview

This PR addresses DYN-9724 by implementing connector pin persistence during reconnection operations. When users perform shift-reconnections (dragging a wire to a different port), connector pins are now preserved through the operation using non-interactive transient pins during the drag, and restored as interactive pins on the final connector.

Changes:

• Added pin location snapshot and restoration logic in both DynamoCore and DynamoCoreWpf
• Introduced IsInteractive property to ConnectorPinViewModel to distinguish transient (non-interactive) from permanent (interactive) pins
• Enhanced null safety throughout connector and pin view models for transient connector scenarios
• Added comprehensive test coverage for the new reconnection behavior

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/DynamoCore/Graph/Connectors/ConnectorModel.cs	Added GetPinLocations() method to provide centralized pin location snapshot API
src/DynamoCore/Models/DynamoModelCommands.cs	Implemented pin location recording and consumption during reconnection workflows with dictionary-based tracking
src/DynamoCoreWpf/ViewModels/Core/ConnectorViewModel.cs	Added SetTransientConnectorPinPositions() and enhanced null safety for transient connector scenarios
src/DynamoCoreWpf/ViewModels/Core/ConnectorPinViewModel.cs	Introduced IsInteractive property to control pin interactivity
src/DynamoCoreWpf/Views/Core/ConnectorPinView.xaml	Bound IsHitTestVisible to IsInteractive property
src/DynamoCoreWpf/ViewModels/Core/StateMachine.cs	Updated reconnection logic to preserve and restore pins through transient connectors
src/DynamoCoreWpf/PublicAPI.Unshipped.txt	Added public API entries for IsInteractive property
test/DynamoCoreWpfTests/ConnectorViewModelTests.cs	Added new test and updated existing test to verify pin persistence behavior

[johnpierson]

Testing locally, the speed is noticeable, which feels odd.

Also, this interaction is still confusing:

[johnpierson]

With testing between @jnealb and I, it feels ok when there is 1 or 2 pins on a wire. A larger amount is noticeable and laggy, but most folks will not have this many pins (I hope).

The lag still makes me hesitant to launch this at all, but I think the gain of not losing pins is worth it. Would appreciate your thoughts @Amoursol @achintyabhat

However, the weirdness when disconnecting the port on the left side of the wire does need fixed before considering merging this.

[ivaylo-matov]

With testing between @jnealb and I, it feels ok when there is 1 or 2 pins on a wire. A larger amount is noticeable and laggy, but most folks will not have this many pins (I hope).

The lag still makes me hesitant to launch this at all, but I think the gain of not losing pins is worth it. Would appreciate your thoughts @Amoursol @achintyabhat

However, the weirdness when disconnecting the port on the left side of the wire does need fixed before considering merging this.

Sorry @johnpierson , it should be working now

[johnpierson]

Comments below, pending any potential speed changes you had @ivaylo-matov .

---

Hey @ivaylo-matov - solid work here. Testing looks good, and it works as expected when using 2-pins on a wire.

That said, I want to call out a few things we'll eventually want to address in follow-up tickets:

Follow-up Backlog Items

1. Copy/paste with pins - when copying nodes that have pinned wires, should copy node + wire + pins to the new location. Only when the pins are selected too I think.
   • This would use a transformation vector based on the new node copy’s transformation
2. Performance tuning - caching pin locations vs rebuilding on every call

Bigger Picture Discussion

One thought this feedback surfaced: why do pins only “decorate” a wire instead of acting like first-class endpoints?

If a pin could terminate the wire and become a redistribution point, you’d get a more composable model:

• the original source node stays stable
• the pin becomes a branching/anchor point you can reconnect, move, or duplicate
• downstream edits don’t force you to chase changes back to the source connection

That likely changes the graph semantics and UI expectations (and might start to look like a lightweight routing/junction node), so it’s not something to sneak into this PR. But it’s a real design direction worth discussing: should “pinning” evolve into “introduce an explicit junction” on the wire?

I'll create Jira tickets for the items above and link them to DYN-9724. Thanks for the detailed implementation!

[ivaylo-matov]

Thanks for the comments @johnpierson , all makes sense. I'll lookup into the options.

[johnpierson]

After another round of testing, this LGTM! With 1-2 pins this interaction feels snappy, and the gain of not losing pins is awesome. Thanks for getting this sorted.

[johnpierson]

If we can check the tests that are failing and modify if needed @ivaylo-matov .

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

[Copilot]

ClearTransientPinCache disposes ConnectorPinModel instances without first ensuring they are removed from DynamoSelection. Because transient pins remain clickable during drag (see ConnectorPinView.xaml.cs mouse handlers), it’s possible for a pin model to be selected and then disposed here, leaving selection containing a disposed model. Consider removing transient pin models from DynamoSelection (and any other UI selection/state) before disposing them, and/or preventing selection by making transient pins non-interactive.

Suggested change

	workspaceViewModel.Pins.Remove(pinViewModel);
	workspaceViewModel.Pins.Remove(pinViewModel);
	// Ensure transient pin models are removed from selection before disposal.
	if (pinViewModel.Model != null &&
	DynamoSelection.Instance.Selection.Contains(pinViewModel.Model))
	{
	DynamoSelection.Instance.Selection.Remove(pinViewModel.Model);
	}

[ivaylo-matov]

Fair point. I’ve added a defensive cleanup in ClearTransientPinCache to remove any transient pin models from DynamoSelection before disposal

[ivaylo-matov]

If we can check the tests that are failing and modify if needed @ivaylo-matov .

Thanks, @johnpierson

DiscardAllConnectorPinModels was iterating ConnectorPinViewCollection directly and called ConnectorModel.RemovePin. This was triggering collection-change events and mutated the collection mid-iteration.

I’ve updated it to iterate over a ToList(), and all ConnectorViewModelTests are now passing locally.

PS: Just saw that there are new tests from DYN-10130 that fail. I'm on it.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud