jdhenke / celestrium

A javascript library to visualize graphs.
MIT License
2 stars 1 forks source link

Minimap #89

Closed willzeng closed 10 years ago

willzeng commented 10 years ago

This data-set agnostic and passive minimap plugin is very basic (it only displays the centerNode and neighbors).

Future updates: it would make more sense for the MiniMap to respond directly to the GraphView rather than to go through NodeSelection. The sort of code I was thinking to implement this would be the following placed in init():

      @graphView = instances['GraphView']
      @graphView.on "enter:node", (nodeEnterSelection) =>
        nodeEnterSelection.on("click", (datum, index) =>
          console.log "datum: ", datum
          @mostRecentNode = datum
          @update()
          )

The reason I haven't implemented this yet is that defining this second on("click") function overwrites the functionality of the one in NodeSelection causing that to break.

Suggestions on how to get around this?

This is a bit related to the issue mentioned earlier in https://github.com/jdhenke/celestrium/pull/81 of either

where the second seems like the way to go.

jdhenke commented 10 years ago

It's funny, I ran into a yesterday #88 where I couldn't add another callback to tick, so my solution was to trigger a tick event off the GraphView plugin, for which any number of listeners could register.

So in general, the trick is to have GraphView create the actual event handler used by d3, which propagates the events we care about through GraphView so multiple listeners can subscribe to it through GraphView.

I'm not sure this is great design, because it could lead to many different types of events going through GraphView but it gets the job done. Good naming conventions can help, like instead of just tick, it should probably be, d3:node:enter:tick or something. More important than what the convention is probably just that there is a convention.

willzeng commented 10 years ago

Does this seem to do the trick: https://github.com/willzeng/celestrium/commit/355f65f8a4309508e3dc652e67f7dda4eb38e8bc

GraphView now creates event handlers for both click (trigger is "node:enter:click") and double click (trigger is "node:enter:dblclick") that are listened for in NodeSelection and MiniMap

jdhenke commented 10 years ago

:+1: this is a much better way of doing things overall.

i'll go through the code more in detail soon.

jdhenke commented 10 years ago

apologies in advance for anything that may seem nit-picky

sorry in advance for every comment, haha.

jdhenke commented 10 years ago

Do you have an example repo which uses this plugin for reference? Feel free to fork celestrium-example and add to the random.html interface.

In terms of naming conventions, I like node:enter:click convention - what was your rationale behind it?

willzeng commented 10 years ago

I'm in no way wedded to the enter:node:click setup, but the rationale was simply that this is the trigger produced by a click on the node created with trigger enter:node

In other words, it is somewhat clear that the enter:node:click trigger is only available following a enter:node trigger.

jdhenke commented 10 years ago

I like that rationale.