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

Deleting node is sandbox breaks the demo if the number of nodes reaches 0 #287

Closed
pushpinder107 opened this issue Jan 11, 2020 · 1 comment · Fixed by #288
Closed

Deleting node is sandbox breaks the demo if the number of nodes reaches 0 #287

pushpinder107 opened this issue Jan 11, 2020 · 1 comment · Fixed by #288
Labels
bug enhancement improving existent functionality or performance related

Comments

@pushpinder107
Copy link
Contributor

Describe the bug
Deleting node is sandbox breaks the demo if the number of nodes reaches 0. The error message in the console is descriptive but the app crashes and the page needs to be refreshed.

To Reproduce
Steps to reproduce the behavior:

  1. Go to https://goodguydaniel.com/react-d3-graph/sandbox/index.html
  2. Click on the '-' button to delete nodes
  3. Keep deleting nodes till node count reaches 0
  4. App crashes

Expected behavior
Either stop deleting nodes when the count has reached 0 or wrap the graph in an error boundary to stop the app from crashing.

Screenshots
If applicable, add screenshots to help explain your problem.
image

Additional context
I can give a PR to fix it, would you rather add an error boundary or stop node deletion when only one is left?

@danielcaldas
Copy link
Owner

Hi @pushpinder107, thank you so much for looking into this. We were aware of this before thus the custom error. Anyways I think we can do better if you're willing to submit a PR I would suggest adding an error boundary here as you suggested:

  • When we reach the point where we only have 1 node, do not delete any more nodes when the user clicks on the - button, and keep logging the error with the same message, but maybe instead of using console.warn so that it's less noisy and alarming.

Thank you!

@danielcaldas danielcaldas added enhancement improving existent functionality or performance related bug labels Jan 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug enhancement improving existent functionality or performance related
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants