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

Node does not move/update when given coordinates or new coordinates #295

Closed
Kav91 opened this issue Jan 24, 2020 · 5 comments · Fixed by #353
Closed

Node does not move/update when given coordinates or new coordinates #295

Kav91 opened this issue Jan 24, 2020 · 5 comments · Fixed by #353

Comments

@Kav91
Copy link
Contributor

Kav91 commented Jan 24, 2020

Describe the bug
If a node is on the map already, and it is given a X or Y coordinate, it will not update to its new location. Likewise if it already had an X/Y coordinate, and received new coordinates it will not update.
If you update the name of the Node however, it will update.

To Reproduce
Use the sandbox, generate a node, give it an x and/or y coordinate and save it.
It will not move to its new location, it will only move if the id attribute is updated as it thinks it is a new node most likely.

Expected behavior
When a node is given coordinates or new coordinates it should update and move into the appropriate location

@Kav91
Copy link
Contributor Author

Kav91 commented Jan 24, 2020

I see the issue is because the X and Y coordinates are in this white list: https://github.com/danielcaldas/react-d3-graph/blob/master/src/components/graph/graph.helper.js#L36

@danielcaldas is there any harm if we remove the X and Y coordinates from this whitelist?

@danielcaldas
Copy link
Owner

danielcaldas commented Mar 1, 2020

Safer would be to conditionaly skip the restore of x and y by allowing the client to provide the NODE_PROPS_WHITELIST. I think it could be a potential solution where you can defined what properties you want to keep from the state regardless the fact that they are being mutated.

On the other hand we could just do a dirty checking agains the x and y values provided by the client, and in case they change we can update them to the most recent client values of x and y.

Something along these lines...

nodes: data.nodes.map(n => {
        let { x, y } = state.nodes[n.id];
        const node = state.nodes[n.id] ? { ...n, ...pick(state.nodes[n.id], NODE_PROPS_WHITELIST) } : { ...n };
        const { x: clientX , y: clientY } = n;
        
        if (clientX) {
            node.x = clientX;
        }
        
        if (clientY) {
            node.y = clientY;
        }
        
        return node;
    }

I have currently no time to look at it, would you be up to giving it a try and submit a PR @Kav91 ?

Will scale priority according to the number of requests for this.

@BradleyBrown19
Copy link

This would be very useful!

@TranquilMarmot
Copy link
Contributor

I've run into this as well; when a user moves a node around, the new position is persisted behind-the-scenes.

I also have undo/redo functionality and undo-ing a node's movement doesn't properly update the graph.

@piotrdelikat
Copy link

Being able to easily pass custom NODE_PROPS_WHITELIST would be helpful enough for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants