[WIP] Converted topology graph to an angular component #75
Conversation
| @@ -46,7 +46,7 @@ | |||
| "copy-webpack-plugin": "4.0.1", | |||
| "coveralls": "^2.11.15", | |||
| "css-loader": "^0.26.0", | |||
| "d3": "^4.4.1", | |||
| "d3": "^4.9.1", | |||
There was a problem hiding this comment.
Will this d3 version 4.x cause issues with patternfly c3 use of d3 version 3.x. Can they coexist without issue?
There was a problem hiding this comment.
I think this should not be a problem as this is packed as a component, but @himdel can tell you more.
There was a problem hiding this comment.
AFAIK the d3 object is in global namespace.
There was a problem hiding this comment.
Well, we are already using ui-components with manageiq and for now there were no issues ...
There was a problem hiding this comment.
ui-classic forces d3 to be ~3.5.0 (3.5.5 right now) anyway ;)
Does this PR actually depend on d3 4+?
There was a problem hiding this comment.
So.. current dependency constraints:
- kubernetes-topology-graph needs >=3.5.0, <=3.5.5 - but I assume we will drop
kubernetes-topology-graphwith the new topology component, right? patternfly-sassneeds ~3.5.17 - so we'll need to update patternfly to a version compatible with d3 4patternfly-sassneeds c3 ~0.4.11, which depends on d3 ~3.5.0 - so, we'll need to update patternfly to a version compatible with new enough c3 - but even the newest has that dependnecy, so either the dependency is obsolete or we can't move to d3 4 yet
There was a problem hiding this comment.
c3js/c3#1648 is the c3 issue on d3 4 compatibility - short answer - waiting for c3 1.0.0
There was a problem hiding this comment.
v4 was a complete change in architecture from v3 so they are quite different even from a API standpoint.
There was a problem hiding this comment.
So, we pretty much have to put this in a separate bundle, ok.
Which means we'll need a webpack config change here to not include topology in the dist bundle, and make a separate topology-only bundle...
.. and if we're doing that, maybe it's time to separate each component into its own bundle (dialog-editor, gtl, site-switcher, toolbar and topology, I guess), abandon the everything-bundle and include just the right ones in miq. (Then you'll need to put d3-4 and topology in a separate webpacker pack on the ui-side, and that's it.)
Cc @karelhala ^
There was a problem hiding this comment.
OR i can take a look for a v3-compatible solution 😉
|
I'm curious about the motivation to use |
|
@mtho11 mostly because of the performance gained by not touching the DOM. Based on these measurements it should be better up to 4K. Also it won't kill the browser's debugger if you open it at the elements panel during an animation. I'm thinking about having both SVG and canvas versions, but the semantic zooming would be much more complicated to implement in SVG. |
|
Ah ok, thanks for the reply @skateman! |
| margin-left: 0; | ||
| margin-right: 0; } | ||
|
|
||
| .miq-small-tile .card-view-pf { |
There was a problem hiding this comment.
why are the card related stuff relevant for topology?
There was a problem hiding this comment.
They are not, this is a generated file which changed from single-line to multiline.
(@skateman you may want to run webpack -p to recompile these..)
- Use <canvas> for rendering instead of <svg> - Angularize tooltips and the context menu - Declaring the input as an HTML attribute - Declaring the filters as angular components - Semantic zooming with optional minimap - Determine fonticon font/char from CSS classes - Automatically centerized icons/images
|
is this replacing the original |
|
@abonas we'll have a meeting about this on Monday where the direction will be converge to merging all the good stuff into a single implementation 😉 |
|
@skateman : are you going to merge the changes here into Patternfly? Shall we close this PR? |
<canvas>for rendering instead of<svg>