Initial Pipeline Viz React Component #2340
Conversation
…ong/pipeline_viz_graph
…ong/pipeline_viz_graph
…ong/pipeline_viz_graph
emilyzhong
changed the title
emilyzhong
changed the title
| from: nodeData.id, | ||
| to: END_NODE_ID, | ||
| color: { | ||
| color: STAGE_BG_COLOR, |
There was a problem hiding this comment.
you may want to consider some sort of className based coloring instead of directly setting styles here
|
Looks great. A lot of work here... I think we can make an effort to make it a bit more modular. We can discuss that offline. |
|
This is not a blocker but for the future I would lean towards more popular libraries if possible (re: ajainarayanan/react-pan-zoom). Package hijacking is always possible https://blog.npmjs.org/post/180565383195/details-about-the-event-stream-incident |
|
looks pretty good 👍 see last comments |
|
ok and see Travis for an out of memory error T_T |
emilyzhong
changed the title
| const START_NODE_ID = -1; | ||
| const END_NODE_ID = -2; | ||
|
|
||
| class PipelineViz extends React.Component { |
There was a problem hiding this comment.
Have you considered a function component with hooks? It looks like it could handle all of this, and be a little cleaner, though I don't want to assume.
There was a problem hiding this comment.
Just curious @dcwither , what criteria triggered you to make that suggestion?
There was a problem hiding this comment.
- use of a
componentDidMountwhen acomponentDidUpdateis probably also necessary (solved by dependencies in theuseEffecthook). - multiple pieces of state that are modified independently, also some that could benefit from useReducer
- I think it would also help identify things that are being mutated that shouldn't be
There was a problem hiding this comment.
Good points!
However this is just the first pass on this component with basic functionality, so I am not sure we want to go with a functional component. But we will definitely keep that in mind.
…ong/pipeline_viz_graph
…ong/pipeline_viz_graph
…/idseq-web into ezhong/pipeline_viz_graph
* migrate projectselect, thresholdfiltertag, sampletypesearchbox * migrate files from common * migrate common/heatmap * migrate pinSampleSelector * fix pr notes
Initial Pipeline Viz React Component
Initial component for pipeline visualization
Notes
Functionality/features very likely to change once final designs are in
Tests
Go to
samples/[sample id]/stage_results