Skip to content
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

DYN-4449 Undo in group casues weird visual artifacts #15067

Merged
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 19 additions & 4 deletions src/DynamoCoreWpf/ViewModels/Core/ConnectorViewModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public partial class ConnectorViewModel : ViewModelBase
public List<Point[]> BezierControlPoints { get; set; }

/// <summary>
/// Property tracks 'X' location from mouse poisition
/// Property tracks 'X' location from mouse position
/// </summary>
public double PanelX
{
Expand Down Expand Up @@ -119,7 +119,7 @@ public Point MousePosition
}

/// <summary>
/// This WatchHoverViewModel controls the visibility and behaviour of the WatchHoverIcon
/// This WatchHoverViewModel controls the visibility and behavior of the WatchHoverIcon
/// which appears when you hover over this connector.
/// </summary>
public ConnectorAnchorViewModel ConnectorAnchorViewModel
Expand Down Expand Up @@ -537,7 +537,7 @@ public PreviewState PreviewState

/// <summary>
/// Toggle used to turn Connector PreviewState to the correct state when a pin is selected.
/// Modelled after connector preview behaviour when a node is selected.
/// Modelled after connector preview behavior when a node is selected.
/// </summary>
public bool AnyPinSelected
{
Expand Down Expand Up @@ -1371,7 +1371,7 @@ private void HandlePinModelChanged(object sender, NotifyCollectionChangedEventAr
/// <summary>
/// Removes all connectorPinViewModels/ connectorPinModels. This occurs during 'dispose'
/// operation as well as during the 'PlaceWatchNode', where all previous pins corresponding
/// to a connector are cleareed.
/// to a connector are cleared.
/// </summary>
/// <param name="allDeletedModels"> This argument is used when placing a WatchNode from ConnectorAnchorViewModel. A reference
/// to all previous pins is required for undo/redo recorder.</param>
Expand Down Expand Up @@ -1426,9 +1426,24 @@ public void Redraw()
this.Redraw(this.ConnectorModel.End.Center);
}

this.SetCollapsedByNodeViewModel();
RaisePropertyChanged(nameof(ZIndex));
}

/// <summary>
/// Evaluates whether both nodes associated with a connector are collapsed, if so, collapses the connector itself.
/// This is to address DYN-4449. Connectors are only recorded in the Undo stack when they are connected.
/// Consequently, if a group is collapsed and then moved, performing an Undo operation will not restore
/// the connector to its state at the time the move was recorded.
/// </summary>
private void SetCollapsedByNodeViewModel()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems like you could write a unit test for the Redraw method that makes sure IsCollapsed is true when it should be.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please see the test included. hope that's okay.

{
if (this.Nodevm.IsCollapsed && this.NodeEnd.IsCollapsed)
{
this.IsCollapsed = true;
}
}

/// <summary>
/// Recalculate the connector's points given the end point
/// </summary>
Expand Down
Loading