[MRG] Initial Release #3
Conversation
Need to improve prop typing in react component file
…ency, added more description
Example in Usage-2.py
|
Notable Problems Noticed: Can't select "preset" Layout inside callbacks. Please refer to #6 |
TODO: testing background, multiple bugs seem to exist
|
@Bachibouzouk Please feel free to review too :) |
|
@matthewchan15 Updated as per your recommendations. Thank you for reviewing! |
shammamah-zz
left a comment
shammamah-zz
left a comment
There was a problem hiding this comment.
Awesome job, Xing! Just a few things I noticed. Let me know if anything is unclear.
| parentData = parent.data(); | ||
| } else { | ||
| parentData = null; | ||
| } |
There was a problem hiding this comment.
| } | |
| let parentData = null; | |
| if (parent) { | |
| parentData = parent.data(); | |
| } |
There was a problem hiding this comment.
It's just a way of rewriting the code that you already have. Not a super big deal, but this way you have a "default" value :)
There was a problem hiding this comment.
@shammamah It makes sense! In this particular case i believe it doesn't affect how the app work since the subsequent if/else statement forces the variable to be assigned to a defined value. However I'd love to read more about the differences between null and undefined; feel free to share me an article/blog post about it :)
| with open('demos/data/sample_network.txt', 'r') as f: | ||
| data = f.read().split('\n') | ||
|
|
||
| edges = data[:750] |
There was a problem hiding this comment.
Why only 750 lines? Is the dataset too big to render?
There was a problem hiding this comment.
Yes, above 750 edges it's harder to visualize the graph
There was a problem hiding this comment.
You may want to make a note of this (like in a comment) so that your users can understand - otherwise they might be confused about why 750
There was a problem hiding this comment.
Ok will do
Edit: Just committed proposed comments
|
Thank you @matthewchan15 @shammamah for the review! Really helped making the source code better :) |
|
Thank you all for the great reviews! I will be merging now. next milestone: dash cytoscape v0.1.0: a stable, well-documented, community-aware version |
Autodeploy on: https://dash-cytoscape.herokuapp.com/
Please review:
Cytoscape.react.jsusage-elements.pyusage-events.pyusage-stylesheet.pyThanks!