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

Allow for custom key, children and toggled fields #12

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Allow for custom key, children and toggled fields #12

wants to merge 8 commits into from

Conversation

bkniffler
Copy link
Contributor

This will allow to optionally set keyField, childrenField and toggleField.

<TreeBeard keyField={'id'} childrenField={'items'} toggledField={'toggled'}/>

Also functions are possible like:

<TreeBeard keyField={x=>x.y.id} childrenField={x=>x.childs} toggledField={x=>true}/>

@bkniffler bkniffler changed the title Allow for custom key and children fields Allow for custom key, children and toggled fields Dec 2, 2015
@alexcurtis
Copy link
Collaborator

@bkniffler. I'm in two minds with this one. Yes, I agree it would be cool to be able to plug in any data model and render it straight away. But I do think thats a job for an external mapper and having a single, concrete data interface on treebeard separates out the concerns. Do you think data field definitions are really necessary?

@bkniffler
Copy link
Contributor Author

@alexcurtis , I understand your point of having a single interface is clean and easier to maintain, but I think these are necessary. Especially with big datasets (and trees are often used on big datasources for better UX) in mind, iterating tree data (recursively) is expensive, mapping the data (probably over and over again) to fit it to the tree component seems like something that could easily be avoided.

A generic component shouldn't be opinionated on the data structure. And with only 3 core properties your component will ever really need on data items, flexible property getters seem like a good fit.

Think about this, and never forget people will use your component for all sort of edge cases you would probably never implement yourself, so flexibility is always a big plus. I don't mind leaving this open and getting more opinions on that matter (maybe open an issue?), its not like I currently need these changes.

@alexcurtis
Copy link
Collaborator

@bkniffler I'll leave this open. We can revisit it at some point.

@fredriklindell
Copy link

+1

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

Successfully merging this pull request may close these issues.

None yet

3 participants