[DYN-8893] Improvements to group behavior#16334
Conversation
There was a problem hiding this comment.
See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-8893
There was a problem hiding this comment.
Pull Request Overview
This PR improves group behavior by addressing double-click handling and accidental group selection issues.
- Adds a double-click event handler in annotation views to create a code block node when clicking within a group's model area.
- Introduces a new method in WorkspaceViewModel to handle annotation double clicks by creating a new code block node and updating associated groups.
- Adds tracking of original annotation group positions to prevent unintentional grouping when groups overlap.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/DynamoCoreWpf/Views/Core/AnnotationView.xaml.cs | Adds double-click event handling to trigger code block creation within groups. |
| src/DynamoCoreWpf/ViewModels/Core/WorkspaceViewModel.cs | Implements HandleAnnotationDoubleClick to create and add a new code block node to a group. |
| src/DynamoCoreWpf/ViewModels/Core/StateMachine.cs | Introduces tracking for original positions of dragged annotation groups and prevents accidental grouping on click. |
| src/DynamoCoreWpf/PublicAPI.Unshipped.txt | Updates public API list to expose the new HandleAnnotationDoubleClick method. |
Comments suppressed due to low confidence (1)
src/DynamoCoreWpf/ViewModels/Core/StateMachine.cs:788
- The variable 'dragedGroups' appears to be misspelled; consider renaming it to 'draggedGroups' for improved clarity and consistency with naming conventions.
});
| var updated = annotation.Nodes.ToList(); | ||
| updated.Add(newNode); | ||
| annotation.Nodes = updated; |
There was a problem hiding this comment.
[nitpick] If annotation.Nodes is a mutable collection, consider adding the new node directly instead of creating a new list to avoid unnecessary memory allocation. If immutability is required by design, this approach is acceptable.
| var updated = annotation.Nodes.ToList(); | |
| updated.Add(newNode); | |
| annotation.Nodes = updated; | |
| annotation.Nodes.Add(newNode); |
There was a problem hiding this comment.
annotation.Nodes is IEnumerable, not a mutable collection
RobertGlobant20
left a comment
There was a problem hiding this comment.
LGTM with two comments
Purpose
This PR aims to address DYN-8893.
Adding the search bar to the group context menu will be handled in a separate PR.
Before:
After:
Declarations
Check these if you believe they are true
*.resxfilesRelease Notes
Improvements to group behavior:
Reviewers
@DynamoDS/eidos
@jasonstratton
FYIs
@achintyabhat
@dnenov