Selective shift + click output connections grabs connectors for selected nodes only #9585
Conversation
|
All tests passing on the self-serve. It originally showed 3 failing tests but that was because #9580 was not pulled into this branch. After pulling in the latest additions from master I verified these do pass with the fix. |
|
one comment thne LGTM, thank you for all the detailed comments |
| if (numOfConnectors == 0) return; | ||
|
|
||
| activeStartPorts = new PortModel[numOfConnectors]; | ||
|
|
||
| for (int i = 0; i < numOfConnectors; i++) | ||
| { | ||
| ConnectorModel connector = selectedPort.Connectors[i]; | ||
| ConnectorModel connector = selectedConnectors[i]; | ||
| connectorsForDeletion.Add(connector); |
There was a problem hiding this comment.
connectorsForDeletion seems a dup to me after the refactor? What do you think
There was a problem hiding this comment.
I think it's still required as removing it will break undo/redo. Below we call CurrentWorkspace.SaveAndDeleteModels(connectorsForDeletion) where SaveAndDeleteModels() is a method in UndoRedo.cs which is a partial class of the WorkspaceModel. Several new and old recorded tests break as well.
There was a problem hiding this comment.
I understand before the refactor it is required, but after the refactor, it contains exactly all the connectors in selectedConnectors.
Can we call code below to preserve the undo/redo behavior?
CurrentWorkspace.SaveAndDeleteModels(selectedConnectors);
There was a problem hiding this comment.
I think we can do this:
CurrentWorkspace.SaveAndDeleteModels(selectedConnectors.ToList<ModelBase>());Since connectorsForDeletion = List<ModelBase> while selectedConnectors = List<ConnectorModel> and ConnectorModel is derived from ModelBase
|
LGTM to me now. Thank you for addressing the comment |
Purpose
This PR pulls in the work originally completed by @yeexinc in #7900 and adds several tests that leverage recorded commands. The recorded tests verify the proper connectors are grabbed when a
shift+clickoperation occurs. Only selected downstream nodes that are connected to the given output port will be removed uponshift+clicking. Testing coverage also verifies the graph remains in a known state after several undo/redo commands are conducted after using selective shift-click operations. The last test performs a large series ofshift+clicksandctrl+clickswith and without selected nodes to test these commands in successive combinations verifying the graph's final state produce a specific numeric combination. All tests pass on the self-serve CI.Declarations
Check these if you believe they are true
*.resxfilesReviewers
@mjkkirschner @QilongTang