Various cleanup in plexus #426
Conversation
- Rename component to Digraph instead of LayeredDigraph - Revisit the default types for generics on TEdge and TVertex - Remove the label field from TVertex - Remove the data and label fields from TEdge Signed-off-by: Joe Farro <joef@uber.com>
- Rename getLocalId to getGlobalId - Remove scaledStrokeWidth - Add props factories onto Digraph Signed-off-by: Joe Farro <joef@uber.com>
Signed-off-by: Joe Farro <joef@uber.com>
tiffon
force-pushed
the
layered-digraph-package-cleanup
branch
from
9d4c5ed
to
53b9aed
Compare
Codecov Report
@@ Coverage Diff @@
## master #426 +/- ##
==========================================
- Coverage 91.68% 91.54% -0.14%
==========================================
Files 176 176
Lines 4039 4034 -5
Branches 968 966 -2
==========================================
- Hits 3703 3693 -10
- Misses 292 297 +5
Partials 44 44
Continue to review full report at Codecov.
|
…eanup Signed-off-by: Joe Farro <joef@uber.com>
There was a problem hiding this comment.
I'm confused as to why some generics default to {} and others to Record<string, unknown>. Is there a guiding principal for when one is preferred over the other? I'd think {} would be best as it forces consumers to provide types if the generic isn't supposed to be empty.
| @@ -74,7 +73,7 @@ export default class DirectedGraph<T> extends React.PureComponent< | |||
| arrowScaleDampener: undefined, | |||
| className: '', | |||
| classNamePrefix: 'plexus', | |||
| getNodeLabel: defaultGetNodeLabel, | |||
| // getNodeLabel: defaultGetNodeLabel, | |||
There was a problem hiding this comment.
Yes, the entire DirectedGraph component will be removed in another PR.
There was a problem hiding this comment.
Will resolve this in the PR that removes DirectedGraph
Signed-off-by: Joe Farro <joef@uber.com> (cherry picked from commit a4e4fa2)
|
I think the generics need a bit more refinement. I suggest this happen on a future PR as the generics are currently in pretty good shape and we have other fish to fry. Currently, the main place to supply types is on the |
Signed-off-by: Joe Farro <joef@uber.com>
Various cleanup in plexus Signed-off-by: vvvprabhakar <vvvprabhakar@gmail.com>
Which problem is this PR solving?
Various cleanup of the plexus package.
Short description of the changes
Note: This builds on the #425 (the README PR).
TEdgeandTVertexTVertexTEdgeDigraphinstead ofLayeredDigraphDigraphas static fieldsgetLocalIdtogetGlobalIdscaledStrokeWidthprop factoryDigraphby wrapping nodes and edges in pure wrappersnumberfromTVertexKey