-
Notifications
You must be signed in to change notification settings - Fork 635
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
Adjust the proxy ports for the groups that have CBN. #12574
Conversation
if (portModel.Owner is CodeBlockNodeModel) | ||
{ | ||
// Special case because code block outputs are smaller than regular outputs. | ||
return new Point2D(Left + Width, y + 6); |
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.
Assuming there is a shift in this case, is there a way we can get the port to be closer to the group edge?
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.
can this value be based on a ratio instead of an exact pixel value?
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.
@mjkkirschner I think we can, but I followed the same logic we used for node ports. https://github.com/DynamoDS/Dynamo/blob/master/src/DynamoCore/Graph/Nodes/PortModel.cs#L186
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.
@QilongTang If we adjust the margin more, it will displace ports coming from other nodes. This margin has been adjusted considering the area used for port snapping.
@@ -724,7 +724,6 @@ | |||
Grid.Row="0" | |||
Canvas.ZIndex="20" | |||
HorizontalContentAlignment="Left" | |||
Style="{StaticResource InOutPortControlStyle}" |
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.
what consequences does removing these styles have?
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.
This styling doesn't make any additional difference to other ports as this same style will be inherited from the node ports(they have this same styling)
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.
These are removed mainly so that the CBN ports won't get overlapped with other ports.
Merging this after talking to @mjkkirschner and @QilongTang. Will be creating a followup task to investigate further after the 2.13.1 hotfix. |
(cherry picked from commit c4a5305)
Purpose
These changes are to adjust the proxy port positions for the collapsed groups that have code block nodes in it.
This fix would not resolve the port design issues as we would need to have multiple controls to adjust based on the port size. In the current design, the ports get overlapped but after this fix, the ports will be stacked below each other.
Before:
After:
Declarations
Check these if you believe they are true
*.resx
filesRelease Notes
Fix the proxy ports positions for groups that have CBN's.
Reviewers
@QilongTang @SHKnudsen @Amoursol