Skip to content
This repository has been archived by the owner on Jan 16, 2023. It is now read-only.

Removal Of 'terminal' attribute in children #11

Closed
alexcurtis opened this issue Dec 2, 2015 · 5 comments
Closed

Removal Of 'terminal' attribute in children #11

alexcurtis opened this issue Dec 2, 2015 · 5 comments
Labels
🤝 help wanted ❓ question Further information is requested

Comments

@alexcurtis
Copy link
Collaborator

This could be derived from the children attribute. If children is undefined, then it is a terminal node. If children is [empty] array then it is a potential parent, that needs to load in its children. If children has elements, then it is definitely a parent node. Can anyone see any issues with this?

@alexcurtis alexcurtis added ❓ question Further information is requested 🤝 help wanted labels Dec 2, 2015
@bkniffler
Copy link
Contributor

It makes sense for your core module. But you should really consider to outsource the toggle functionality to your decorator and join together header/toggle, as this would easily allow us to customize the behavior.

@alexcurtis
Copy link
Collaborator Author

Maybe a good step in the right direction is for treebeard to determine this and pass it as a prop to the decorators. Then we can look at the decorators as a separate issue. What I'm trying to achieve here is simplifying down the data input as much as possible, so getting Treebeard setup is a snap for people in general. At the moment, the data bit feels a little contrived.

@bkniffler
Copy link
Contributor

Separate those properties, that you need for core functionality (children, toggled) and those additional for default out-of-the-box behavior (name, active, ...). Allow for a custom .id property used as a key instead of 'index' for building the tree. Allow for renaming core properties by this occasion, like items instead of children.

Don't forget that there will always be the users who want their tree state managed by treebeard and those who want to manage it themselves, which currently works okay if you suppress the default onToggle with e.preventDefault(), so there is no setState({toggled: true}).

@bkniffler
Copy link
Contributor

Also it would be great to use something like http://jnuno.com/tree-model-js/ to store the tree data, but a node will have its toggled inside of a property model in that case. So maybe you could allow overwriting the toggled by a function passed as a prop like:

function isToggled(node){
   return node.model.toggled;
}
<TreeBeard isToggled={isToggled}/>

@alexcurtis
Copy link
Collaborator Author

I'll add in the id -> key option as I think that is definitely a good idea. We have to make sure we aren't making it over complicated with some of these things.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🤝 help wanted ❓ question Further information is requested
Development

No branches or pull requests

2 participants