Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Feature/interactive_physical_layout #4

Merged
merged 8 commits into from
Nov 22, 2016
Merged

Feature/interactive_physical_layout #4

merged 8 commits into from
Nov 22, 2016

Conversation

jbrekelmans
Copy link
Contributor

@jbrekelmans jbrekelmans commented Nov 1, 2016 •
edited
Loading

1/3 Pull requests on repositories Netflix/vizceral, Netflix/vizceral-example and Netflix/vizceral-react with the following features around "optimizing layout manually":

  1. Repositioning of nodes by dragging them
  2. Interactive Particle System simulation (nodes are particles and edges are spring forces) because it's awesome and allow for good automatic layout on some graphs
  3. Resetting layout if Vizceral canvas always height = 0 #2 get's out of hand
  4. Layout algorithm that respects positional metadata in the JSON graph files and otherwise lays out nodes in a circle
  5. Export (well, Console.log) current node positions to be used as positional metadata once a satisfactory manual layout has been created

How to run:

  1. Clone the branch feature/interactive_physical_layout of repositories jbrekelmans/vizceral, jbrekelmans/vizceral-example and jbrekelmans/vizceral-react locally
  2. Run command 'npm link' in vizceral
  3. Run command 'npm link vizceral' in vizceral-react
  4. Run command 'npm link' in vizceral-react
  5. Run command 'npm link vizceral-react' in vizceral-example
  6. Make sure a graph (e.g. in sample_data.json) with 10-20 nodes is loaded for best results using the default Particle System settings

@jbrekelmans jbrekelmans changed the title Initial commit Feature/interactive_physical_layout Nov 2, 2016
jrsquared
jrsquared reviewed Nov 3, 2016
View reviewed changes
src/vizceral.jsx
@@ -200,18 +217,18 @@ Vizceral.defaultProps = {
connectionHighlighted: () => {},
definitions: {},
filters: [],
graphsUpdated: () => {},
graphsUpdated: () => undefined,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the change? These have default empty functions so they are just no-ops if the user does not provide listeners to things they do not care about.

Copy link
Contributor Author

@jbrekelmans jbrekelmans Nov 4, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the moment I thought all of those lambda expressions were creating functions that returned empty POJO's, but I learnt this is not the case. I'll undo them since I prefer the old syntax

@brekelj1
Undo introducing more verbose syntax for semantically equivalent expr…
b4ab6f5 
…essions
@brekelj1
Change the condition for calling updateData (removing regions now als…
df65d30 
…o results in updateData being called)

Added more verbosity, coming from a better safe than sorry/defensive coding perspective
@jbrekelmans
Copy link
Contributor Author

jbrekelmans commented Nov 16, 2016 •
edited
Loading

Hey Justin!

I've changed the condition for calling updateData so it works for me. Of course I would like to know whether it works for other people as well and what the design here is.

The main difference is I call updateData if regions are removed and I do not accept non-falsy
~'nodes' that are not arrays. ~
What you'll additionally see is my defensive style of coding preferring
verbosity/confidence/debuggability over having less lines of code and performance (not to say
performance improvements are really significant in this code path).

Merged with master and adopted your much simpler condition for calling updateData.

Kind regards,

Jasper Brekelmans

@brekelj1
Merge commit 'c6e081396a5bd7ac23c673629a1a23376f13e2e4' into feature/…
1ef35fd 
…interactive_physical_layout

# Conflicts:
#	src/vizceral.jsx
@brekelj1
Fix bug introduced in merging from master
2f34a1c
@brekelj1
Added property for repositioning of nodes by dragging, fix potential …
040c0ff 
…issue where defaults for allowDraggingOfNodes and showLabels are not passed to Visceral initially. Remove code I forgot to remove
@jrsquared jrsquared merged commit 040c0ff into Netflix:master Nov 22, 2016
@jbrekelmans jbrekelmans deleted the feature/interactive_physical_layout branch December 6, 2016 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jrsquared jrsquared jrsquared left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jbrekelmans @jrsquared @brekelj1