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

Better deletion of Famous Nodes. #20

Open
wants to merge 2 commits into
base: mixed-mode
Choose a base branch
from

Conversation

trusktr
Copy link

@trusktr trusktr commented Aug 27, 2015

I thin this is better because it does cleanup inside the parent node as well.

It might be better to call removeChild on the parent since that does some cleanup in the parent, and it also calls dismount of the child. What do you think?
…de onto some other node in a custom class that extends react-famonus Node.
@@ -42,7 +42,7 @@ class Node extends React.Component {
}

famousDelete() {
this._famousNode.dismount();
this._famousNode.getParent().removeChild(this._famousNode);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pilwon This throws if the node has no parent. Maybe we should check if the node has a parent first?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pilwon By the way, I've ben using react-famous on two projects. It's really nice (except for all the bugs in Famous itself right now)!

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

Successfully merging this pull request may close these issues.

1 participant