DYN-9724: Allow Pins to Stay Until Connector is Actually Disconnected#16909
DYN-9724: Allow Pins to Stay Until Connector is Actually Disconnected#16909ivaylo-matov wants to merge 8 commits intoDynamoDS:masterfrom
Conversation
…onnect Co-authored-by: Ivo Petrov <ivaylo-matov@users.noreply.github.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-9724
There was a problem hiding this comment.
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
IsInteractiveproperty toConnectorPinViewModelto 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 |
| public ConnectorPinViewModel(WorkspaceViewModel workspaceViewModel, ConnectorPinModel model) | ||
| { | ||
| this.WorkspaceViewModel = workspaceViewModel; | ||
| this.model = model; | ||
| InitializeCommands(); | ||
| model.PropertyChanged += OnPinPropertyChanged; | ||
| ZIndex = ++StaticZIndex; // places the pin on top of all nodes/notes | ||
|
|
||
| DynamoSelection.Instance.Selection.CollectionChanged += SelectionOnCollectionChanged; | ||
|
|
||
| AddPinToGroupIfConnectedNodesInSameGroup(); | ||
| } |
There was a problem hiding this comment.
The 'IsInteractive' property should be initialized to 'true' by default in the constructor to ensure non-transient pins created through normal workflows are interactive. Currently, the field defaults to false, which means regular pins would incorrectly be non-interactive unless explicitly set. Consider adding 'isInteractive = true;' in the ConnectorPinViewModel constructor, or alternatively, initialize the field with 'private bool isInteractive = true;'.
There was a problem hiding this comment.
in ConnectorPinViewModel, IsInteractive is already initiated with
private bool isInteractive = true;
we also explicitly set IsInteractive = !isTransientPin in AddConnectorPinViewModel
|
Testing locally, the speed is noticeable, which feels odd. Also, this interaction is still confusing: |
johnpierson
left a comment
johnpierson
left a comment
There was a problem hiding this comment.
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
|
|
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:
Core reconnection/pin persistence
ConnectorModel(GetPinLocations) and reused it from both core and WPF reconnection flowsTransient connector/pin behavior
IsInteractivetoConnectorPinViewModelIsInteractiveinConnectorPinView.xamlIsInteractive = false;permanent pins remain interactive.Added a dedicated test:
ShiftReconnectionUsesNonInteractiveTransientPinsAndPersistsPinsthat verifies transient connector is used during drag and pins persist/recover as interactive on the final connectorDeclarations
Check these if you believe they are true
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