Skip to content
This repository has been archived by the owner on Jul 29, 2019. It is now read-only.

Networks: Phantom link after creating cluster and creating link to node in cluster #1218

Closed
abcbaby opened this issue Aug 24, 2015 · 21 comments

Comments

@abcbaby
Copy link

abcbaby commented Aug 24, 2015

Here is the example:

http://jsbin.com/rafari/16/edit

@AlexDM0
Copy link
Contributor

AlexDM0 commented Aug 24, 2015

Hi Dragon,

I'll see if we can fix this before the upcoming release. It should have been today, but it will probably be tormorrow.

Regards

@abcbaby
Copy link
Author

abcbaby commented Aug 24, 2015

Thx. Will wait for release.

@abcbaby
Copy link
Author

abcbaby commented Aug 24, 2015

BTW, I notice when I use jsbin to create the sample file, if I point the script file to: https://raw.githubusercontent.com/almende/vis/master/dist/vis.js, it will not load/run. I have to point to an CDN for it to load. Do you know why?

@AlexDM0
Copy link
Contributor

AlexDM0 commented Aug 24, 2015

It's in the MIME settings, you'll have to pass it through https://rawgit.com/

On the other hand, CDN is better because your issue is for a specific version of vis, if we release, your jsbin will be invalid.

Regards

@AlexDM0
Copy link
Contributor

AlexDM0 commented Aug 24, 2015

I think that for this issue, you'll have to decluster and cluster again.. I'm not sure if this will be easily fixed before the release. The easiest fix I can think of is not allowing edges to hidden nodes be shown, on the other hand, this may interfere with people using that as a cheat to redirect edges.

@abcbaby
Copy link
Author

abcbaby commented Aug 24, 2015

it's a workaround. but with simple logic in the example, it should work. It may be confusing to other users when they use it.

@AlexDM0
Copy link
Contributor

AlexDM0 commented Aug 24, 2015

I'll have to think of a nice way to do this, the edges should not need to know about the clustering to keep the modularity of the network. I'll look into it.

@abcbaby
Copy link
Author

abcbaby commented Aug 24, 2015

Let me know. Because if it gets to buggy, I may have to write my own way of clustering. Holding my separate list.

@AlexDM0
Copy link
Contributor

AlexDM0 commented Aug 24, 2015

I have reworked the clustering in the next release, greatly speeding it up for stacked clusters. The edge problems we had before are also solved, but this is a different case. There is no catch for making an edge connected to a node that is in a cluster as it stands now.

@abcbaby
Copy link
Author

abcbaby commented Aug 24, 2015

You have an excellent tool! I'm just trying to help it improve it more since it's the "de facto" here and it's now, I think, the main attraction in our organization and is spreading wildly to others org. Just for a heads up, clustering is becoming a more demanding piece here, where I will be concentrating more. So I may find a few more issues.

@AlexDM0
Copy link
Contributor

AlexDM0 commented Aug 24, 2015

I hope you do (in the sense that its difficult to test hehe)!

Glad you guys are happy with it :)

The next release should have great performance gains in the physics during initialization and stabilization all around.

@abcbaby
Copy link
Author

abcbaby commented Aug 24, 2015

I was trying to create another issue, when I encounter this one. So when you fix this issue, think about the case, when you have "cluster 1" and "cluster 2". Then you cluster them together to become "cluster 3" (which contains cluster 1 & 2). Then you create a new node and the edge points to either or both of cluster 1 and/or 2.

Since clusters can be within clusters, vis.js should be smart enough to know if the link points to one of those inner cluster. That is what we are building and will assume it to work. Hope I explained clearly here, if not. Let me know, I'll try to create a more specific example.

@abcbaby
Copy link
Author

abcbaby commented Aug 24, 2015

The reason your tool is so powerful is because I've created a search capability for them. Where you initially draw the network. Then users can then search for addition information where those information are converted to nodes and edges and update the network with the changes. It is all dynamically built. Similar to the example I have: https://github.com/dragonzone/JsSamplerApp. It's about a month old. But that's the ideal state.

@AlexDM0
Copy link
Contributor

AlexDM0 commented Aug 24, 2015

Hi Dragon,

Just so I understand what you mean, you'd expect that a cluster edge is created when a node connects to a node in a cluster right? This is exactly the same case as you're current issue right? Except the cluster will be "deeper" in the stack.

Regards

@abcbaby
Copy link
Author

abcbaby commented Aug 24, 2015

Yes, you'll have to create a while loop to dig through all the inner cluster to see if that edge points to one of the nodes in any of those cluster level. I was just afraid you only think about 1 level down, where it can be X levels.

@AlexDM0
Copy link
Contributor

AlexDM0 commented Aug 24, 2015

I agree with you that this should be done. I don't think it will make this release though. Probably the one after that, hopefully that one will be released next week, and the current develop version this week.

@abcbaby
Copy link
Author

abcbaby commented Aug 24, 2015

That's fine. I know it'll pretty sophisticated. As long as we identified it and it is being worked on. It should all be good.

@mojoaxel
Copy link
Member

mojoaxel commented Nov 5, 2016

This is still an issue in v4.17.0!

@AlexDM0
Copy link
Contributor

AlexDM0 commented Feb 11, 2017 via email

@AoDev
Copy link
Contributor

AoDev commented Feb 11, 2017

@AlexDM0 Thanks for the answer. Sorry, before I commented on the wrong issue because I had another opened in the browser. #1291 . But it's all related.

I wish to know more about what is the responsibility of what.

  • What is this.body ?

  • What is this.body.nodes versus this.body.data.nodes ?

  • Also clusterNode.containedNodes is I guess used for what you mentioned.

  • Why is it possible to used different kind of data sources?

  {
    nodes: [some nodes],
    edges: [some edges]
  }

vs

  {
    nodes: new vis.DataSet([some nodes]),
    edges: new vis.DataSet([some edges])
  }

The reason being that it's possible to listen and react on DataSet changes. That's how I get the cluster node to replicate the changes in its "containedNodes". But I wouldn't be able to do that if it's not a dataSet.

@AlexDM0
Copy link
Contributor

AlexDM0 commented Feb 11, 2017 via email

yotamberk pushed a commit that referenced this issue Aug 8, 2017
* First working version of updating clustered edge

* Added fix for #1315 as well

* Enable unit testing for module Network

Adds mocks for certain components, in order to let module `Network` be run in unit tests.

Changes:

- Create a mock object for `hammer.js` when running browserless. The inspiration is taken from [here](visgl/deck.gl#658)
- Create a mock function for `window.requestAnimationFrame()` when running browserless in `network/modules/CanvasRenderer.js`
- Added unit test for `Network` to show that unit testing for it now works
- Fixed naming of container in `test/Graph3d.test.js`

Since `hammer.js` is also used in other modules, this fix is potentially an enabler for full-module unit tests for those as well.

* Cleanup unit test Network

* Added unit test for fix issue #1218

* Adding test for #1315 - Interim save

* Completed unit test and fixes for #1315

* Added fixes for #1291

* Added unit test for #1219

* Added header comment for Clustering.js, small fixes

* Fix for unit test

* Added example networks to unit test

* Fix error in loading disassemblerExample

* Network unit test final fixes

* Fixes for linting

* Fixed essential typo

* Fixed linting error

* Fixed unit test

* Fixed unit test again
primozs pushed a commit to primozs/vis that referenced this issue Jan 3, 2019
)

* First working version of updating clustered edge

* Added fix for almende#1315 as well

* Enable unit testing for module Network

Adds mocks for certain components, in order to let module `Network` be run in unit tests.

Changes:

- Create a mock object for `hammer.js` when running browserless. The inspiration is taken from [here](visgl/deck.gl#658)
- Create a mock function for `window.requestAnimationFrame()` when running browserless in `network/modules/CanvasRenderer.js`
- Added unit test for `Network` to show that unit testing for it now works
- Fixed naming of container in `test/Graph3d.test.js`

Since `hammer.js` is also used in other modules, this fix is potentially an enabler for full-module unit tests for those as well.

* Cleanup unit test Network

* Added unit test for fix issue almende#1218

* Adding test for almende#1315 - Interim save

* Completed unit test and fixes for almende#1315

* Added fixes for almende#1291

* Added unit test for almende#1219

* Added header comment for Clustering.js, small fixes

* Fix for unit test

* Added example networks to unit test

* Fix error in loading disassemblerExample

* Network unit test final fixes

* Fixes for linting

* Fixed essential typo

* Fixed linting error

* Fixed unit test

* Fixed unit test again
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants