-
Notifications
You must be signed in to change notification settings - Fork 11
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
feat: topology group node #2477
Changes from all commits
074c3fa
116fd69
0eee791
080cfa7
fffd64c
4c697dd
b9e505d
94e479a
a93de40
827d831
d040a48
69f504c
87af1d5
aa00349
844b9a5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,6 +13,7 @@ import { | |
TopologyDataSpecifier, | ||
TopologyEdgeInteractionHandler, | ||
TopologyEdgeRenderer, | ||
TopologyLayout, | ||
TopologyLayoutType, | ||
TopologyNode, | ||
TopologyNodeInteractionHandler, | ||
|
@@ -58,9 +59,18 @@ export class TopologyComponent implements OnChanges, OnDestroy { | |
@Input() | ||
public shouldAutoZoomToFit?: boolean = false; | ||
|
||
@Input() | ||
public draggableNodes?: boolean = true; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: this sounds like it is storing the nodes which are draggable. Could we rename it to allowNodeDrag? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, this was the property already present in the topology config, so I used the same name. do we change at both places or just input? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is fine if it is an existing term |
||
|
||
@Input() | ||
public layoutType?: TopologyLayoutType; | ||
|
||
@Input() | ||
public customLayout?: TopologyLayout; // This will override `layoutType` property | ||
|
||
@Input() | ||
public supportGroupNode?: boolean; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need this as an input or can we extract this info from our custom group node layout property? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The layout and group nodes are different IMO. The current flow looks like this.
The one thing that we can do here is we can extract this property by checking if the nodes contain a group node or not. If this works I can make the changes |
||
|
||
@ViewChild('topologyContainer', { static: true }) | ||
private readonly container!: ElementRef; | ||
|
||
|
@@ -86,9 +96,12 @@ export class TopologyComponent implements OnChanges, OnDestroy { | |
tooltipRenderer: this.tooltipRenderer, | ||
showBrush: this.showBrush, | ||
shouldAutoZoomToFit: this.shouldAutoZoomToFit, | ||
draggableNodes: this.draggableNodes, | ||
nodeInteractionHandler: this.nodeInteractionHandler, | ||
edgeInteractionHandler: this.edgeInteractionHandler, | ||
layoutType: this.layoutType, | ||
customLayout: this.customLayout, | ||
supportGroupNode: this.supportGroupNode, | ||
}); | ||
|
||
// Angular doesn't like introducing new child views mid-change detection | ||
|
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.
break up this class :)
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.
Moved the group node related code to a util class, but still it is going over around 20 lines 😑
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.
Need to break it up further. Can do it in a follow up.