Split Port Views and ViewModels#12062
Conversation
|
@QilongTang I will fix the merge conflict but I think the IsProxy should be a property on the model and not a wrapper class. |
# Conflicts: # src/DynamoCoreWpf/ViewModels/Core/AnnotationViewModel.cs # src/DynamoCoreWpf/ViewModels/Core/PortViewModel.cs
| public bool SetConnectorsVisibility | ||
| { | ||
| get => areConnectorsHidden; | ||
| set | ||
| { | ||
| areConnectorsHidden = value; | ||
| RaisePropertyChanged(nameof(SetConnectorsVisibility)); | ||
| } | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Enables or disables the Hide Wires button on the node output port context menu. | ||
| /// </summary> | ||
| public bool HideWiresButtonEnabled | ||
| { | ||
| get => hideWiresButtonEnabled; | ||
| set | ||
| { | ||
| hideWiresButtonEnabled = value; | ||
| RaisePropertyChanged(nameof(HideWiresButtonEnabled)); | ||
| } |
| { | ||
| InPorts.Clear(); | ||
| List<ProxyPortViewModel> newPortViewModels; | ||
| List<PortViewModel> newPortViewModels; |
There was a problem hiding this comment.
sorry @saintentropy - why did you make all these changes related to proxy type.
There was a problem hiding this comment.
I think instead of additional type, @saintentropy is using a property on PortViewModel to indicate if it is a proxy.. Given there is no new property in ProxyPortViewModel yet, this should be OK. But in the future if we need to store additional info, this may not scale. But I am fine with it for now, How about you @mjkkirschner?
There was a problem hiding this comment.
@mjkkirschner @QilongTang I maid the change as the purpose of the new wrapper class ProxyPortViewModel was just to provide a type check and did not have a different feature set. Because there are now essentially two base classes for ports it seemed to make more sense to add the proxy status as a property (for the is proxy check) and keep the classes a bit simpler in the code.
| if (_port.Connectors.Count < 1 || _node.WorkspaceViewModel.Connectors.Count < 1) return false; | ||
|
|
||
| // Attempting to get a relevant ConnectorViewModel via matching NodeModel GUID | ||
| ConnectorViewModel connectorViewModel = _node.WorkspaceViewModel.Connectors |
There was a problem hiding this comment.
too bad theres no map of connectors.
| return connectorViewModel.IsCollapsed; | ||
| } | ||
|
|
||
| public DelegateCommand MouseLeftButtonDownOnContextCommand |
There was a problem hiding this comment.
same questions as in inport
| private bool areConnectorsHidden; | ||
| private string showHideWiresButtonContent = ""; | ||
| private bool hideWiresButtonEnabled; | ||
| protected static SolidColorBrush portBorderBrushColor = new SolidColorBrush(Color.FromArgb(255, 204, 204, 204)); |
There was a problem hiding this comment.
appears some of these are repeated in the other new classes...
There was a problem hiding this comment.
There are other colors but they are specific for inputs not dup
# Conflicts: # src/DynamoCoreWpf/ViewModels/Core/AnnotationViewModel.cs
QilongTang
left a comment
QilongTang
left a comment
There was a problem hiding this comment.
LGTM, the regressions should be fixed on master already
|
seems there is still a conflict |
|
@saintentropy There is one regression on this PR, once it's fixed, should be good to go |
|
Oops, there is also merge conflicts to address again |
6 participants
Purpose
The purpose of this PR is to split up the Port View and ViewModels. As the difference between In Ports and Out Ports has grown in UX and Code complexity, it is putting a high burden on implementation and a net drag on memory and performance of the Workspace View. This PR allows the port views to be implemented and styled independently. This PR should maintain feature capability with existing 2.13 WIP. This does introduce a 10% gain in start up time and similar for memory allocation.
Declarations
Check these if you believe they are true
*.resxfilesReviewers
@mjkkirschner @OliverEGreen @QilongTang
FYIs
@SHKnudsen