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

Add a checking callback before deleting a node. #18

Open
gintau opened this issue Mar 14, 2014 · 3 comments
Open

Add a checking callback before deleting a node. #18

gintau opened this issue Mar 14, 2014 · 3 comments

Comments

@gintau
Copy link
Contributor

gintau commented Mar 14, 2014

Hello @sderickson,

According to CodeCombat #507, components should care about their dependencies before being deleted. Currently the deleting action is handled by treema, but dependencies are found by thang editor. So I'm thinking about adding a callback (passed by constructor) in onDeletePressed() or in removeSelectedNodes(). If the callback exists and return false, then treema should prevent deletion from proceeding. How do you think?

@sderickson
Copy link
Contributor

I don't think the callbacks should affect behavior. Adding a delete callback could still work, though. I'd say:

  1. Have remove mark itself as changed.
  2. Have broadcastChanges also call a 'delete' callback, providing all nodes that were deleted.

Then in CodeCombat, we can have the editor get all the info it needs to also delete any other nodes which were dependent on the deleted node. How's that sound?

@gintau
Copy link
Contributor Author

gintau commented Mar 14, 2014

I agree that callbacks should not affect behavior, but there are still some issues to consider. One is that some errors are raised if components depended by others are deleted, which prevent editor from working correctly. By your mean, we can delete a component and its dependents, but the errors would still appear.

Now I'm trying to find a way to alert users and prevent them from deleting the components which has dependents. Of course, the best case is to do this without injecting something which affects default behavior.

@sderickson
Copy link
Contributor

Hmm, good point. The only other solution I can think of that would prevent removal from happening would be to create an abstract method canRemove which can be overridden. But that's not much of a better solution. Okay, let's go with the callback and see how it goes as a system. I suppose the error will be shown through the Treema itself?

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

No branches or pull requests

2 participants