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

[Sandbox] Fail to remove links / nodes in the live demo example #212

Closed
LonelyPrincess opened this issue Aug 2, 2019 · 6 comments · Fixed by #216
Assignees

Comments

@LonelyPrincess
Copy link
Collaborator

@LonelyPrincess LonelyPrincess commented Aug 2, 2019

Describe the bug
The sandbox example breaks when the user tries to remove an item by using the - links found in the editable Graph Data section. This applies to both nodes or links.

To Reproduce
Steps to reproduce the behavior:

  1. Go to the sandbox page
  2. Expand the links or nodes property in the Graph Data section of the page
  3. Click the - button next to the link / node that you wish to remove
  4. See error

Expected behavior
The selected node or link should dissappear from the editable data view and the graph shall be updated accordingly.

Additional context
When trying to remove a link, the following error is displayed at the developer's console:

react-dom.production.min.js?f6dd:188 TypeError: Cannot read property 'source' of null
    at eval (graph.helper.js?28ec:219)
    at Array.find (<anonymous>)
    at _loop (graph.helper.js?28ec:219)
    at _validateGraphData (graph.helper.js?28ec:216)
    at Object.initializeGraphState (graph.helper.js?28ec:324)
    at Graph.componentWillReceiveProps (Graph.jsx?fe1a:436)
    at sg (react-dom.production.min.js?f6dd:150)
    at Sg (react-dom.production.min.js?f6dd:178)
    at yh (react-dom.production.min.js?f6dd:202)
    at zh (react-dom.production.min.js?f6dd:203)

If the item you're trying to delete is a node, then the following error is shown instead:

react-dom.production.min.js?f6dd:188 TypeError: Cannot read property 'x' of null
    at eval (Sandbox.jsx?dd0e:238)
    at Array.map (<anonymous>)
    at Sandbox.decorateGraphNodesWithInitialPositioning (Sandbox.jsx?dd0e:236)
    at Sandbox.render (Sandbox.jsx?dd0e:314)
    at Lg (react-dom.production.min.js?f6dd:167)
    at Sg (react-dom.production.min.js?f6dd:180)
    at yh (react-dom.production.min.js?f6dd:202)
    at zh (react-dom.production.min.js?f6dd:203)
    at Wh (react-dom.production.min.js?f6dd:212)
    at Zh (react-dom.production.min.js?f6dd:211)
@danielcaldas

This comment has been minimized.

Copy link
Owner

@danielcaldas danielcaldas commented Aug 8, 2019

Now that I'm reading the description of the issue with more attention, this is not actually a bug. If you remove a node you must ensure that the respective links, that refer that node are also removed. But removing a link tough, should not fail... even if some node becomes orphan afaik, the graph should not break.

This is the behavior that is implemented in the Sandbox for the - button.

image

I'll look into it with more detail, can't promise that I'm come out with a solution in short notice tough.

@danielcaldas

This comment has been minimized.

Copy link
Owner

@danielcaldas danielcaldas commented Aug 8, 2019

Actually, I think I got it! Somehow react-editable-json-tree is giving us null instead of completely removing the element from our data. Is this expected behavior of your library @oxyno-zeta?

image

@oxyno-zeta

This comment has been minimized.

Copy link

@oxyno-zeta oxyno-zeta commented Aug 8, 2019

Hello @danielcaldas ,
Yes the remove button first remove the object and put null at the same place. After, you can remove the null item.
This mecanism was done to allow changing from object to another type like Array, string, boolean, ... Otherwise, no type change was possible :).
I'm sorry that this breaks your demo.
Tell me if you have any questions.

@danielcaldas

This comment has been minimized.

Copy link
Owner

@danielcaldas danielcaldas commented Aug 8, 2019

All good! Your docs are awesome @oxyno-zeta! I'll be using onBeforeRemoveGraphData and onFullyUpdate functions to handle this properly in our sandbox.

@oxyno-zeta

This comment has been minimized.

Copy link

@oxyno-zeta oxyno-zeta commented Aug 8, 2019

Thanks :)
Maybe the onDeltaUpdate (https://github.com/oxyno-zeta/react-editable-json-tree#ondeltaupdate) is better because there is a delta object that can help you to detect null objects and where are they no ?

@danielcaldas

This comment has been minimized.

Copy link
Owner

@danielcaldas danielcaldas commented Aug 8, 2019

Fixed with #216

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.