Skip to content

Commit

Permalink
DYN-4449 Undo in group casues weird visual artifacts (#15067)
Browse files Browse the repository at this point in the history
* DYN-4449 fixed

connectors in groups remain collapsed on undo

* DYN-4449-update

second PR attempt

* DYN-4449-Unit test added
  • Loading branch information
ivaylo-matov authored Apr 3, 2024
1 parent a0984a2 commit ee93174
Show file tree
Hide file tree
Showing 3 changed files with 405 additions and 4 deletions.
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()
{
if (this.Nodevm.IsCollapsed && this.NodeEnd.IsCollapsed)
{
this.IsCollapsed = true;
}
}

/// <summary>
/// Recalculate the connector's points given the end point
/// </summary>
Expand Down
52 changes: 52 additions & 0 deletions test/DynamoCoreWpfTests/AnnotationViewModelTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1427,6 +1427,58 @@ public void CanToggleVisibilityOfAllNodesInAGroup()
Assert.AreEqual(true, ViewModel.CurrentSpaceViewModel.HasUnsavedChanges);
}

[Test]
public void ConnectorsRemainCollapsedOnUndoCollapsedGroup()
{
// Arrange
var parentGroupName = "parentGroup";

OpenModel(@"core\annotationViewModelTests\connectorsRemainCollapsedAfterUndo.dyn");

var parentGroupVM = ViewModel.CurrentSpaceViewModel.Annotations.FirstOrDefault(x => x.AnnotationText == parentGroupName);
var connectors = ViewModel.CurrentSpaceViewModel.Connectors;

// Assert group exists and connectors are expanded
Assert.IsNotNull(parentGroupVM);
Assert.AreEqual(0, connectors.Count(c => c.IsCollapsed));

// Act
// Collapse the parent group
parentGroupVM.IsExpanded = false;

// Assert connectors are collapsed
Assert.AreEqual(2, connectors.Count(c => c.IsCollapsed));

// Select the all nodes from parent group and move them
DynamoSelection.Instance.Selection.Clear();
parentGroupVM.SelectAll();

// Perform move operation
var initialGroupX = parentGroupVM.Left;
var initialGroupY = parentGroupVM.Top;

var operation = DynamoModel.DragSelectionCommand.Operation.BeginDrag;
var command = new DynamoModel.DragSelectionCommand(new Point2D(100, 100), operation);

ViewModel.ExecuteCommand(command);

operation = DynamoModel.DragSelectionCommand.Operation.EndDrag;
command = new DynamoModel.DragSelectionCommand(new Point2D(300, 300), operation);

ViewModel.ExecuteCommand(command);

// Assert the group has moved and connectors remain collapsed
Assert.AreNotEqual(initialGroupX, parentGroupVM.Left);
Assert.AreNotEqual(initialGroupY, parentGroupVM.Top);
Assert.AreEqual(2, connectors.Count(c => c.IsCollapsed));

// Undo move operation
ViewModel.UndoCommand.Execute(null);

// Assert connectors remain collapsed post-undo
Assert.AreEqual(2, connectors.Count(c => c.IsCollapsed));
}

#endregion
}
}
Loading

0 comments on commit ee93174

Please sign in to comment.