Remove duplicates from the expandedNodeMap array #1031
Conversation
…ds further testing
| @@ -82,7 +82,7 @@ export default class Graph { | |||
| this.nodeMap[eNode.id] = eNode | |||
| this._nodes.push(eNode) | |||
| this.expandedNodeMap[node.id] = this.expandedNodeMap[node.id] | |||
| ? this.expandedNodeMap[node.id].concat([eNode.id]) | |||
| ? [...new Set(this.expandedNodeMap[node.id].concat([eNode.id]))] | |||
There was a problem hiding this comment.
so I have to investigate a little more but I have removed duplicates at this stage and it seems to work. I have to look into the nodes parameter a bit as to why it includes the multiples of the node relationships. It may be that thats where the change is required.
There was a problem hiding this comment.
I have tracked back to https://github.com/neo4j/neo4j-browser/blob/master/src/shared/services/bolt/boltMappings.js#L214 and tried to put in a reduce here to remove duplicates but since this function is used in several places, it seems to cause other issues. So to narrow the scope to the particular problem, Im going to stick with my initial solution for now.
There was a problem hiding this comment.
I pulled it down locally and the bug still seems to be there...
There was a problem hiding this comment.
so we can either can look to go ahead with this work to keep the scope smaller, or I can leave a comment on #1003 with a link to this PR (as the context of where Hugo is making the changes could potentially be a better fit than this work). What do you think @oskarhane?
There was a problem hiding this comment.
I think this change makes sense 👍
fixes: #905
There seems to be an issue where the
expandedNodeMapcontains duplicates, so that as the code recursively loops over theexpandedNodeMaparray to remove nodes when a node is collapsed, it falls into an issue where the node is not found the second time round during the recursion and throws an error.I'm not sure of where to place a test for this as there didn't seem to be any e2e tests for expanding/closing nodes unless Im mistaken, so any input on that would be appreciated. The other option would be to open a unit test file but I don't want to start cluttering the code base with unnecessary unit test files if they're not needed.
Before - shows the duplicates in the
expandedNodeMap:Before Video
After - shows no duplicates in the
expandedNodeMap:After video
changelog: Fixes viz bug when expand -> dismiss -> expand nodes.