[DYN-7843] Resize collapsed groups#16203
Conversation
There was a problem hiding this comment.
See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-7843
jasonstratton
left a comment
There was a problem hiding this comment.
I believe I have been able to follow the logic and it makes sense. LGTM. Approved!
|
DynamoSelfServe failed. Attempting to run it again. |
|
It failed again. I will take a look w/ Reddy in the morning |
There was a problem hiding this comment.
Pull Request Overview
This PR introduces support for resizing collapsed groups in the application by adding a bottom‐right resize thumb and tracking separate width/height adjustments for collapsed and expanded states. Key changes include adding a new thumb control style for collapsed groups, updating event handlers and layout measurement methods to support dynamic size updates, and serializing additional group adjustment properties for state restoration.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/DynamoCoreWpf/Views/Core/AnnotationView.xaml.cs | Removed SizeChanged event subscriptions and added logic to handle resizing via IsVisibleChanged and DragDelta events. |
| src/DynamoCoreWpf/Views/Core/AnnotationView.xaml | Introduced a new thumb style (GroupResizeThumbStyle) and updated layout bindings for collapsed group dimensions. |
| src/DynamoCoreWpf/ViewModels/Core/AnnotationViewModel.cs | Added tracking of separate adjustment values for collapsed and expanded states and updated connector redraw logic. |
| src/DynamoCoreWpf/UI/Converters.cs | Updated color conversion logic to adjust thumb foreground based on contrast ratio. |
| Other files (SerializationConverters.cs, PublicAPI.Unshipped.txt, WorkspaceModel.cs, AnnotationModel.cs) | Extended serialization and model properties to support the new collapsed resize functionality. |
Comments suppressed due to low confidence (1)
src/DynamoCoreWpf/Views/Core/AnnotationView.xaml.cs:47
- Removing the SizeChanged event handler could potentially miss updates when the rectangle's size changes without a visibility change. Verify that relying solely on IsVisibleChanged provides consistent updates, or consider a combined approach to ensure reliability.
this.CollapsedAnnotationRectangle.SizeChanged += CollapsedAnnotationRectangle_SizeChanged;
| portControl.UpdateLayout(); | ||
|
|
There was a problem hiding this comment.
Calling UpdateLayout during drag events might impact performance if the port control contains many items; consider optimizing or caching these measurements where feasible.
| portControl.UpdateLayout(); |
There was a problem hiding this comment.
@ivaylo-matov This review seems relevant enough to be considered. Can you take a look and cache or limit the layout update here? This has been our focus for performance improvement, and we would like to avoid such calls, if possible.
There was a problem hiding this comment.
sure, will take a look
There was a problem hiding this comment.
@zeusongit , you are right.
After testing, I found that UpdateLayout() was unnecessary and have removed it.
I've also combined the two measurement methods into a single one to avoid iterating over each portControl twice.
| { | ||
| public object Convert(object value, Type targetType, object parameter, CultureInfo culture) | ||
| { | ||
| var controlType = parameter as string; |
There was a problem hiding this comment.
[nitpick] Consider documenting the expected string values for the controlType parameter (e.g., 'Thumb') to clarify its usage for future maintainers.
There was a problem hiding this comment.
I've updated the XML doc summary to clarify that controlType expects a string such as "Thumb" to determine which color to use
This reverts commit e2a8ee6.
Purpose
This PR aims to address DYN-7843
This change allows users to resize a collapsed group from the bottom-right corner, similarly to how resizing works when the group is expanded.
Please see the changes below:
Declarations
Check these if you believe they are true
*.resxfilesRelease Notes
Latest changes allow users to resize groups while collapsed.
Reviewers
@DynamoDS/eidos
@jasonstratton
FYIs
@achintyabhat
@dnenov