DYN-8986 Fix graphs opening in a dirty state #16262
Conversation
There was a problem hiding this comment.
See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-8986
There was a problem hiding this comment.
Pull Request Overview
This PR ensures that setting HasUnsavedChanges only occurs once group text boxes are actually rendered, preventing the workspace from being marked dirty during graph load.
- Wraps
HasUnsavedChangesassignment in an ActualHeight/ActualWidth check for group title and description text boxes - Applies the same guard to both
GroupTextBox_OnTextChangedandGroupDescriptionTextBox_TextChangedhandlers
Comments suppressed due to low confidence (2)
src/DynamoCoreWpf/Views/Core/AnnotationView.xaml.cs:240
- Add a comment explaining that this check distinguishes implicit initialization changes from explicit user edits, clarifying why we only mark the workspace dirty after rendering.
if (GroupTextBox.ActualHeight > 0 && GroupTextBox.ActualWidth > 0)
src/DynamoCoreWpf/Views/Core/AnnotationView.xaml.cs:240
- Add unit or integration tests to verify that
HasUnsavedChangesremainsfalseduring initialization (when dimensions are zero) and becomestrueon actual user edits.
if (GroupTextBox.ActualHeight > 0 && GroupTextBox.ActualWidth > 0)
| SetTextMaxWidth(); | ||
| SetTextHeight(); | ||
| ViewModel.WorkspaceViewModel.HasUnsavedChanges = true; | ||
| if (GroupTextBox.ActualHeight > 0 && GroupTextBox.ActualWidth > 0) |
There was a problem hiding this comment.
if the textbox has custom group name and description already, won't the actual H & W be greater than 0. We don't want to set the HasUnsavedChanges to true in that case.
There was a problem hiding this comment.
H & W are 0 only at initialization, at which point the group title and description are set (custom or default)
There was a problem hiding this comment.
I'm confused, are the height and width greater than zero only after the text is changed?
There was a problem hiding this comment.
Yes, at-least at initialization, when WPF is creating groups.
The group text is bind to AnnotationText, which is set to the stored value when the control is initialized, that action should not trigger HasUnsaved changes, because we are initializing the group for the first time.
I tried capturing this by IsLoaded and IsReady events, but at this point none of them worked, so I needed a way to differentiate between an explicit action by the user to change the text versus group text initialization on workspace load. So I found that H & W are 0 at this point which won't be the case when any user action would trigger it.
aparajit-pratap
left a comment
There was a problem hiding this comment.
I'm curious how you isolated the graph being marked dirty due to the group text changing?
Put a breakpoint in HasUnsavedChanges setter, and tracked the call stack |
Purpose
When opening large graphs the workspace appears to become dirty, and that kicks off the backup process in the background, which adds delay to the loading process.
This seems to happen when groups are being loaded and setting their text values makes the graph dirty, so for a fix checking if the group has been rendered when TextChange event is triggered, to differentiate from an explicit change and an implicit one at initialization at which point group has not been actually rendered.
Declarations
Check these if you believe they are true
*.resxfilesRelease Notes
Fix graphs opening in a dirty state
Reviewers
@DynamoDS/eidos