Skip to content

DYN-8631 - Block UndoRedo Operation During Cluster Prediction#16157

Merged
johnpierson merged 9 commits intoDynamoDS:masterfrom
johnpierson:undoRedoBlock
Apr 23, 2025
Merged

DYN-8631 - Block UndoRedo Operation During Cluster Prediction#16157
johnpierson merged 9 commits intoDynamoDS:masterfrom
johnpierson:undoRedoBlock

Conversation

@johnpierson
Copy link
Copy Markdown
Member

@johnpierson johnpierson commented Apr 21, 2025

Purpose

This PR aims to address the issue, DYN-8631, by disabling the undo/redo process while iterating through cluster results. Additionally, this PR enables preselection of the nodes created by the cluster.

20250421-clusterUndoBlock

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

@chubakueno, @QilongTang,

FYIs

@Jingyi-Wen , @Amoursol , @achintyabhat

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-8631

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.

Some comments then LGTM

Comment thread src/DynamoCore/Graph/Workspaces/UndoRedo.cs Outdated
Comment thread src/DynamoCore/Graph/Workspaces/UndoRedo.cs Outdated
Comment thread src/DynamoCore/Graph/Workspaces/UndoRedo.cs Outdated
var node = PortViewModel.NodeViewModel;

//unlock undo/redo
node.WorkspaceViewModel.Model.UndoRedoLocked = false;
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.

This is fine for now, we could also make it more granular in the future, similar to how undorecorder that the period of the lock is maintained by a living cycle of an object instead of bool setting by the coder. Something to think about

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.

Let me take a look at this. I think I might have some ideas.

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 might file a task for "As a dev, I want to add a new run state to dynamo graphs for 'is autocompleting'" or something along those lines.

Comment thread src/NodeAutoCompleteViewExtension/ViewModels/NodeAutoCompleteBarViewModel.cs Outdated
Copy link
Copy Markdown
Contributor

@chubakueno chubakueno 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
Copy link
Copy Markdown
Contributor

Thank you @johnpierson Will take another look soon

@johnpierson johnpierson requested a review from BogdanZavu April 23, 2025 14:56
get { return ((null != undoRecorder) && undoRecorder.CanUndo); }
get
{
return ((null != undoRecorder) && undoRecorder.CanUndo) && !IsUndoRedoLocked;
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.

Looks like an extra set of bracket there can be removed..

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.

Fixed!

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 with one comment

@johnpierson johnpierson merged commit cdafe82 into DynamoDS:master Apr 23, 2025
21 checks passed
@johnpierson johnpierson deleted the undoRedoBlock branch April 23, 2025 15:49
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.

3 participants