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.

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

implement remove edge from clustering #21

Open
micahstubbs opened this issue Sep 3, 2018 · 7 comments
Open

implement remove edge from clustering #21

micahstubbs opened this issue Sep 3, 2018 · 7 comments
Labels
bug Something isn't working

Comments

@micahstubbs
Copy link
Member

micahstubbs commented Sep 3, 2018

remove edge from clustering

steps to reproduce

cd visjs-network
# run a local server
live-server

then visit the example (you may need to update the port to match the port that your local server is running on)
http://127.0.0.1:51702/examples/network/other/clusteringByZoom.html

open the browser console (cmd+j on a mac) and notice the error message:

screen shot 2018-09-02 at 9 51 30 pm

@micahstubbs micahstubbs added the bug Something isn't working label Sep 3, 2018
@JHeinzde
Copy link

Hello Micah,

I would consider trying to implement this, but I am relatively unexperienced, so it may take a while for me to implement this. Let me know your thoughts on this.

Best Regards,

Jonathan

@micahstubbs
Copy link
Member Author

@JHeinzde go for it! I'd be happy to review a PR that fixes this bug.

@Tyler-Maclachlan
Copy link
Contributor

@micahstubbs @JHeinzde Hey guys, I'm getting this error a lot with clusters, can someone point me in the right direction to fix it, or is this almost implemented?

@micahstubbs
Copy link
Member Author

@Tyler-Maclachlan perhaps so. can you share a screenshot or some error text from the error that you are seeing? That will help us know how best to help 😄

@Tyler-Maclachlan
Copy link
Contributor

@micahstubbs,

I had the same error as in the screenshot above. Here is how I've fixed it for my project:

if (shouldBeClustered === this._isClusteredEdge(edge.id)) {
        return // all is well
      }

      if (shouldBeClustered) {
        // add edge to clustering
        let clusterFrom = this._getClusterNodeForNode(edge.fromId)
        if (clusterFrom !== undefined) {
          this._clusterEdges(this.body.nodes[edge.fromId], edge, clusterFrom)
        }

        let clusterTo = this._getClusterNodeForNode(edge.toId)
        if (clusterTo !== undefined) {
          this._clusterEdges(this.body.nodes[edge.toId], edge, clusterTo)
        }

        // TODO: check that it works for both edges clustered
        //       (This might be paranoia)
      } else {
        delete this._clusterEdges[edgeId]
        this._restoreEdge(edge)
        // This should not be happening, the state should
        // be properly updated at this point.
        //
        // If it *is* reached during normal operation, then we have to implement
        // undo clustering for this edge here.
        // throw new Error('remove edge from clustering not implemented!')
      }

@micahstubbs
Copy link
Member Author

ah hah! thanks for that snippet @Tyler-Maclachlan. I'll try this out locally and see if I can reproduce the fix.

@Tyler-Maclachlan
Copy link
Contributor

@micahstubbs cool, will you let me know if you merge these changes and my previous PR #66, because we've currently forked the library to carry on working.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants