| CARVIEW |
Navigation Menu
-
Notifications
You must be signed in to change notification settings - Fork 658
DYN-8986 Fix graphs opening in a dirty state #16262
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this 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-8986
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
| @@ -237,7 +237,10 @@ private void GroupTextBox_OnTextChanged(object sender, TextChangedEventArgs e) | |||
|
|
|||
| SetTextMaxWidth(); | |||
| SetTextHeight(); | |||
| ViewModel.WorkspaceViewModel.HasUnsavedChanges = true; | |||
| if (GroupTextBox.ActualHeight > 0 && GroupTextBox.ActualWidth > 0) | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm confused, are the height and width greater than zero only after the text is changed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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