Skip to content

DYN-8494 - Group Cluster Node and Wire Creation into One Undo#16148

Merged
QilongTang merged 9 commits intoDynamoDS:masterfrom
johnpierson:dyn8494-checkConnections
Apr 18, 2025
Merged

DYN-8494 - Group Cluster Node and Wire Creation into One Undo#16148
QilongTang merged 9 commits intoDynamoDS:masterfrom
johnpierson:dyn8494-checkConnections

Conversation

@johnpierson
Copy link
Copy Markdown
Member

@johnpierson johnpierson commented Apr 16, 2025

Purpose

This PR expands upon DYN-8494 by grouping actions within the cluster creation into one undo group. It also disallows undo of clusters that were previewed, but not used.

Also related to DYN-8632

Behavior Before:
20250415-cluster-Before

Behavior Now:
20250416-cluster-After
20250416-cluster-After2

Declarations

Check these if you believe they are true

  • The codebase is in a better state after this PR
  • Is documented according to the standards
  • The level of testing this PR includes is appropriate
  • User facing strings, if any, are extracted into *.resx files
  • All tests pass using the self-service CI.
  • Snapshot of UI changes, if any.
  • Changes to the API follow Semantic Versioning and are documented in the API Changes document.
  • This PR modifies some build requirements and the readme is updated
  • This PR contains no files larger than 50 MB

Release Notes

N/A

Reviewers

@BogdanZavu (after vacation) @QilongTang @chubakueno

FYIs

@Amoursol @Jingyi-Wen

@github-actions github-actions Bot changed the title DYN8494 - Group Cluster Node and Wire Creation into One Undo DYN-: DYN8494 - Group Cluster Node and Wire Creation into One Undo Apr 16, 2025
Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@johnpierson johnpierson changed the title DYN-: DYN8494 - Group Cluster Node and Wire Creation into One Undo DYN-8494 - Group Cluster Node and Wire Creation into One Undo Apr 16, 2025
RecordUndoModels(wsViewModel.Model, newNodesAndWires);
}

private void RecordUndoModels(WorkspaceModel workspace, List<ModelBase> undoItems)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Saw a similar implementation somewhere else, maybe we dont have to repeat it and can put this as a utility function in the DynamoUtility class/project?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went ahead and reused the method from the DynamoModel (where it is currently).



RecordUndoGraphLayout(workspace, isGroupLayout, reuseUndoRedoGroup);
//only record graph layout undo when it is not node autocomplete
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious what does this mean?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we are using the layout method that exists in Dynamo already, it adds an undo for the auto layout. So if we skip it when triggered from node autocomplete it helps minimize the undos

dynamoViewModel.Model.ExecuteCommand(new DynamoModel.CreateNodeCommand(Guid.NewGuid().ToString(), typeInfo.FullName, xoffset, node.NodeModel.Y, false, false));

//disallow the node creation command from the undo group, we group node creation and wires below
wsViewModel.Model.UndoRecorder.PopFromUndoGroup();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh interesting, I guess with this we can keep the current implementation with commands?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! :)

Copy link
Copy Markdown
Contributor

@QilongTang QilongTang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@QilongTang QilongTang merged commit 5727e38 into DynamoDS:master Apr 18, 2025
24 checks passed
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.

2 participants