Initial components for supporting layers of graph elements in plexus #416
Conversation
Signed-off-by: Joe Farro <joef@uber.com>
Signed-off-by: Joe Farro <joef@uber.com>
tiffon
changed the title
Codecov Report
@@ Coverage Diff @@
## master #416 +/- ##
==========================================
- Coverage 91.46% 91.41% -0.06%
==========================================
Files 174 174
Lines 3915 3915
Branches 897 897
==========================================
- Hits 3581 3579 -2
- Misses 296 298 +2
Partials 38 38
Continue to review full report at Codecov.
|
Signed-off-by: Joe Farro <joef@uber.com>
|
@everett980 I finished the TODO for this; it's ready for review. |
| @@ -31,7 +31,7 @@ export function matches(path: string) { | |||
| return Boolean(matchPath(path, ROUTE_MATCHER)); | |||
| } | |||
|
|
|||
| export function getUrl(query?: Object | null | undefined) { | |||
| export function getUrl(query?: { [key: string]: unknown } | null | undefined) { | |||
There was a problem hiding this comment.
Record<string, unknown>?
|
|
||
| export type TFromGraphStateFn<T = {}, U = {}> = (input: TExposedGraphState<T, U>) => TAnyProps | null; | ||
|
|
||
| export type TPropsFactory<PropNames extends string, FactoryFn extends TPropFactoryFn> = { |
There was a problem hiding this comment.
Why does PropNames extends string? Should/could this just be <PropNames: string | string[]>?
| }; | ||
|
|
||
| type TStandaloneEdgesLayer<T = {}, U = {}> = TEdgesLayer<T, U> & { | ||
| layerType: TLayerType; |
There was a problem hiding this comment.
What would HTML edges look like?
| Done = 'Done', | ||
| } | ||
|
|
||
| export type TLayerType = 'html' | 'svg'; |
There was a problem hiding this comment.
why do we need both the TLayerType and the ELayerType? It seems as though the enum should be sufficient?
| /> | ||
| </div> | ||
| </div> | ||
| <h1>LayeredDigraph with standalone layers</h1> |
There was a problem hiding this comment.
If possible, the distinction between this and the previous graph should be more apparent. It seems like one has a layersGroup that contains a single html nodes layer, meanwhile they other just has that single html nodes layer as a standalone.
This example should showcase the value provided by layersGroups.
| constructor(props: TLayeredDigraphProps<T, U>) { | ||
| super(props); | ||
| const { edges, vertices, zoom: zoomEnabled } = props; | ||
| if (Array.isArray(edges) && edges.length && Array.isArray(vertices) && vertices.length) { |
There was a problem hiding this comment.
what should this do if there aren't edges and vertices?
| } | ||
| if (zoomEnabled) { | ||
| this.zoomManager = new ZoomManager(this.onZoomUpdated); | ||
| } else { |
There was a problem hiding this comment.
I don't think this else is necessary based on the assignment before the constructor.
| this.setState({ sizeVertices }); | ||
| const { layout } = layoutManager.getLayout(edges, sizeVertices); | ||
| // const { positions, layout } = layoutManager.getLayout(edges, sizeVertices); | ||
| // positions.then(this._onPositionsDone); |
There was a problem hiding this comment.
I'm not well versed in the LayoutManager, what is the difference between this and the next line? Is this commented out to implement/enable later?
|
|
||
| private setSizeVertices = (senderKey: string, sizeVertices: TSizeVertex<T>[]) => { | ||
| const { edges, layoutManager, measurableNodesKey: expectedKey } = this.props; | ||
| if (senderKey !== expectedKey) { |
There was a problem hiding this comment.
is this implemented to prevent multiple measurable layers?
| }; | ||
|
|
||
| private renderLayers() { | ||
| const { classNamePrefix, layers: topLayers } = this.props; |
There was a problem hiding this comment.
I'm looking at topLayers v layer the latter of which can be a group of layers. Can that group of layers have more groups of layers within it? or is it exclusively a one or two depth array?
…egertracing#416) Initial components for supporting layers of graph elements in plexus Signed-off-by: vvvprabhakar <vvvprabhakar@gmail.com>
Which problem is this PR solving?
Adding the first couple of components for supporting layers of graph elements in plexus.
Short description of the changes
Add
LayeredDigraph,HtmlLayersGroup,MeasurableNodesLayerandMeasurableHtmlNodeto plexus.Also, update TypeScript to
3.5.3.From the demo:
TODO:
MeasurableNodesLayeras a standalone layer, i.e. not as a layer within aHtmlLayersGroupNode layout comparison
↓
LayeredDigraph(component under development)↓
DirectedGraph(the current component)