-
Notifications
You must be signed in to change notification settings - Fork 90
Topology map component #633
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
Conversation
@skateman bump |
eslint.yaml
Outdated
@@ -35,7 +35,7 @@ rules: | |||
vars-on-top: 2 | |||
no-debugger: 2 | |||
no-cond-assign: 2 | |||
no-console: 2 | |||
no-console: 0 |
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.
YAGNI 😉
0835e64
to
2749404
Compare
@Hyperkid123 the zooming is just wrong, it should zoom the distance among the nodes, but not the nodes itself. Check out my PR... |
f0a661d
to
f515fdd
Compare
@skateman Yep i know, i'am having some issues zooming in combination with dragging (v3 / v4 inconsistencies). I know how it is supposed to be, but until i have to make i cooperate first 😆 |
c790cf8
to
b1608cf
Compare
b1608cf
to
154c418
Compare
b3a011b
to
76896a9
Compare
bc5f96a
to
9e76e16
Compare
@skateman ping |
@skateman can you take a look at this please? |
e56bfd4
to
bf39ae3
Compare
* <li>size: node radius; default value is 17</li> | ||
* <li>iconType: string that can be one of <b>'fonticon', 'fileicon'</b>. icon is chosen based on iconType value. If no type has been chosen, default icon will be used.</li> | ||
* <li>iconClass: if iconType is <b>fonticon</b>, node should have icon css class. eg: <b>'fa fa-info'</b></li> | ||
* <li>path: if iconType is <b>fileicon</b>, this attribute specifies path to image file. eg: <b>'/some/path/to/image/image.png'</b></li> |
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't you just use fonticon
or fileicon
directly? The priority should be fileicon
first...as in trees.
selectNode: '<', | ||
tooltipStyle: '<?', | ||
}, | ||
templateUrl: 'charts/topology-map.html', |
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.
The html file should be located at charts/topology-map/topology-map.html
Hi @Hyperkid123, I find it strange that there is no hover or selection css/treatment. |
Are there any plans for multi-select? Ctrl+Right-Click? or selection by rectangle? |
726b7af
to
23793b4
Compare
@Hyperkid123 I found a solution for the IE11 fonticon issue. The fonticonService hack we have in ui-components should be enough, you just have to convert the |
@skateman thanks. I will check it as soon as i can |
505709f
to
3ea548a
Compare
@dtaylor113 can you review this please? |
023ac67
to
8cc3e02
Compare
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.
Hi, LGTM. small nit, pls include a blurb in the description about how zooming works and that it zooms to where ever the mouse is hovering in the chart.
Also, you will need to change your commit msg to adhere to https://github.com/patternfly/angular-patternfly#feature-release.
Since this is a new feature I would recommend a Minor release: "feat(pfTopologyMap): Initial Implementation"
00aa142
to
ed549f2
Compare
ping @dtaylor113 |
ed549f2
to
8823892
Compare
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.
LGTM, thanks!
8823892
to
54517f7
Compare
ping @dtaylor113 can you merge this please? (Just rebased to remove conflict) |
Description
Goal is to create new topology graph component. New component will not have any internal logic such as searching or filtering. This should be done in parent component.
Instead of SVG it uses canvas for rendering. This should increase performance for graphs with big amount of nodes.
Please ignore changes to eslint and package.json, before final commit i will revert it back to original state. I only wanted to make my life easier.
PR Checklist
dragging nodes while zooming resets position to 0, 0. Have to solve that.node font iconnode image iconnode custom opacity, fill, border and icon colorlinks dashed line optionutlization indicationnode tooltip on hoverData format
Component has these parameters: nodes, edges, select-node, multi-select-nodes, select-edge, multi-select-edges, tooltip-style, show-node-labels, show-edge-labels
Nodes data format
Array of objects. Only
id
is mandatory parameterEdges data format
Array of objects. Each edge have two mandatory attributes source and target.
Select node(s)
User can select one or more nodes. Single node is selected by clicking.
Multiple nodes are selected by click + ctrl/shift key
Select edge(s)
User can select one or more nodes. Single node is selected by clicking.
Multiple edges are selected by click + ctrl/shift key
Tooltip style
Object with styling properties for tooltips. Tooltip style is not mandatory attribute. None of the styling properties is mandatory. If some property is not specified, default value will be used.
Tooltips will only be shown for nodes with
title
property.Zooming
Component also supports semantic zooming. Only distance between nodes is growing/shrinking, but node size remains the same. Canvas will zoom towards the position of the mouse cursor.