Skip to content

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

Merged
merged 1 commit into from
Apr 18, 2018
Merged

Conversation

Hyperkid123
Copy link
Contributor

@Hyperkid123 Hyperkid123 commented Sep 27, 2017

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

  • use canvas for rendering
  • zooming
  • dragging
    • dragging nodes while zooming resets position to 0, 0. Have to solve that.
  • select node(s)
  • select edge(s)
  • minimap
  • Specify correct data format
  • Handle different nodes styling
    • node font icon
    • node image icon
    • node custom opacity, fill, border and icon color
    • links dashed line option
    • utlization indication
    • node tooltip on hover
  • Code styling

Data 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 is an array of nodes to render.
  • Edges is array of links between nodes.
  • Select-node is a function which will return clicked node as a parameter if passed function.
  • Multi-select-nodes is a function which will return array of selected nodes. Multiple nodes can be selected by shift/ctrl + clicking.
  • Select-edge is a function which returns clicked edge.
  • Multi-select-edges is a function which returns array of selected edges. Multiple edges can be selected by shift/ctrl + clicking.
  • Tooltip-style is a object with styling properties of node titles
  • Show-node-label is a boolean. If true, all node titles will be shown without hovering over nodes.
  • Show-edge-labels is a boolean. If true, all edge titles will be shown without hovering over nodes.

Nodes data format

Array of objects. Only id is mandatory parameter

nodes = [
  //node with all attributes
  {
    id: 'nodeId1',
    title: 'nodeTitle',
    size: 17, //node radius,
    fileicon: this attribute specifies path to image file. eg: '/some/path/to/image/image.png'. File icon has higher priority than fonticon.
    fonticon: css class of node icon eg: 'fa fa-info'
    fill: '#FFFFFF', //color of node background
    borderColor: '#000000', //color of node border
    iconColor: '#FFFFFF', //color of fontIcon
    opacity: 〈0,1〉, //node opacity
    utilization: 〈0,100〉, //border chart around the node
  },
  {
    id: 'nodeId2',
  }, //example of node without any attributes
]

Edges data format

Array of objects. Each edge have two mandatory attributes source and target.

edges = [
  {
    source: 'nodeId1', //id of source node in nodes array
    target: 'nodeId2', //id of target node in nodes array
    lineStyle: ['dashed'], //style of line
    title: 'edge title', //edge tool tip
  }
]

Select node(s)

User can select one or more nodes. Single node is selected by clicking.

selectNode = function(node){
   console.log('selected node from graph: ', node);
 };

Multiple nodes are selected by click + ctrl/shift key

multiSelect = function(array) {
  console.log('selected nodes from graph: ', array);
}

Select edge(s)

User can select one or more nodes. Single node is selected by clicking.

selectEdge = function(edge){
   console.log('selected edge from graph: ', edge);
 };

Multiple edges are selected by click + ctrl/shift key

multiSelectEdge = function(array) {
  console.log('selected edges from graph: ', array);
}

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.

tooltip-style = {
 size: 12,
 font: 'Arial',
 textColor: 'white',
 background: 'rgba(0, 0, 0, .5)',
 borderColor: 'red',
 borderWidth: 10
}

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.

@Hyperkid123
Copy link
Contributor Author

@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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

YAGNI 😉

@Hyperkid123 Hyperkid123 force-pushed the topology-map branch 4 times, most recently from 0835e64 to 2749404 Compare September 30, 2017 13:27
@skateman
Copy link
Member

skateman commented Oct 1, 2017

@Hyperkid123 the zooming is just wrong, it should zoom the distance among the nodes, but not the nodes itself. Check out my PR...

@Hyperkid123 Hyperkid123 force-pushed the topology-map branch 2 times, most recently from f0a661d to f515fdd Compare October 1, 2017 14:48
@Hyperkid123
Copy link
Contributor Author

@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 😆

@Hyperkid123 Hyperkid123 force-pushed the topology-map branch 2 times, most recently from c790cf8 to b1608cf Compare October 2, 2017 10:03
@cdcabrera cdcabrera added the bug label Oct 2, 2017
@cdcabrera cdcabrera removed the bug label Oct 2, 2017
@Hyperkid123 Hyperkid123 force-pushed the topology-map branch 2 times, most recently from b3a011b to 76896a9 Compare October 9, 2017 09:23
@Hyperkid123 Hyperkid123 changed the title [WIP]Topology map component Topology map component Oct 15, 2017
@Hyperkid123 Hyperkid123 force-pushed the topology-map branch 3 times, most recently from bc5f96a to 9e76e16 Compare October 16, 2017 09:33
@Hyperkid123
Copy link
Contributor Author

@skateman ping

@Hyperkid123
Copy link
Contributor Author

@skateman can you take a look at this please?

* <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>
Copy link
Member

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',
Copy link
Member

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

@dtaylor113
Copy link
Member

Hi @Hyperkid123, I find it strange that there is no hover or selection css/treatment.

@dtaylor113
Copy link
Member

Are there any plans for multi-select? Ctrl+Right-Click? or selection by rectangle?

@Hyperkid123 Hyperkid123 force-pushed the topology-map branch 7 times, most recently from 726b7af to 23793b4 Compare February 1, 2018 12:03
@skateman
Copy link
Member

skateman commented Feb 1, 2018

@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 XXXX hex code to YYYY decimal and pass it further as &#YYYY;. Haven't tried it as I'm on my phone now, but it looks promising 😉

@Hyperkid123
Copy link
Contributor Author

@skateman thanks. I will check it as soon as i can

@Hyperkid123 Hyperkid123 force-pushed the topology-map branch 3 times, most recently from 505709f to 3ea548a Compare February 5, 2018 14:52
@Hyperkid123
Copy link
Contributor Author

@dtaylor113 can you review this please?

@Hyperkid123 Hyperkid123 force-pushed the topology-map branch 2 times, most recently from 023ac67 to 8cc3e02 Compare March 1, 2018 08:22
Copy link
Member

@dtaylor113 dtaylor113 left a 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"

@Hyperkid123 Hyperkid123 force-pushed the topology-map branch 5 times, most recently from 00aa142 to ed549f2 Compare March 5, 2018 10:16
@Hyperkid123
Copy link
Contributor Author

ping @dtaylor113

dtaylor113
dtaylor113 previously approved these changes Mar 12, 2018
Copy link
Member

@dtaylor113 dtaylor113 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@Hyperkid123
Copy link
Contributor Author

Hyperkid123 commented Apr 18, 2018

ping @dtaylor113 can you merge this please? (Just rebased to remove conflict)

@dtaylor113 dtaylor113 merged commit 58e32de into patternfly:master Apr 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants