feat: topology group node #2477
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #2477 +/- ##
==========================================
- Coverage 82.99% 82.20% -0.79%
==========================================
Files 925 927 +2
Lines 20639 20860 +221
Branches 3262 3310 +48
==========================================
+ Hits 17129 17149 +20
- Misses 3389 3574 +185
- Partials 121 137 +16 ☔ View full report in Codecov by Sentry. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| @@ -1,3 +1,4 @@ | |||
| /* eslint-disable max-lines */ | |||
There was a problem hiding this comment.
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.
Need to break it up further. Can do it in a follow up.
projects/observability/src/shared/components/topology/d3/d3-topology.ts
Outdated
Show resolved
Hide resolved
projects/observability/src/shared/components/topology/d3/d3-topology.ts
Outdated
Show resolved
Hide resolved
projects/observability/src/shared/components/topology/d3/d3-topology.ts
Outdated
Show resolved
Hide resolved
projects/observability/src/shared/components/topology/d3/d3-topology.ts
Outdated
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
projects/observability/src/shared/components/topology/d3/d3-topology.ts
Outdated
Show resolved
Hide resolved
...ects/observability/src/shared/components/topology/d3/interactions/drag/topology-node-drag.ts
Outdated
Show resolved
Hide resolved
projects/observability/src/shared/components/topology/utils/topology-converter.ts
Outdated
Show resolved
Hide resolved
Unit Test Results 4 files 316 suites 45m 16s ⏱️ Results for commit fffd64c. |
| @@ -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.
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.
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.
It is fine if it is an existing term
| public customLayout?: TopologyLayout; // This will override `layoutType` property | ||
|
|
||
| @Input() | ||
| public supportGroupNode?: boolean; |
There was a problem hiding this comment.
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.
The layout and group nodes are different IMO. The current flow looks like this.
- We create group nodes like we create entity nodes using the data that we get from the backend.
- We pass this data to the topology.
- Now topology handles group nodes based on the property.
- custom layout (with some business logic) is passed from the parent component and used to position the nodes on some layout logic. The layout may or may not care about the group nodes.
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
Screen.Recording.2023-10-19.at.11.15.16.PM.mov