aslagle / reactive-table

A reactive table designed for Meteor
https://atmospherejs.com/aslagle/reactive-table
Other
328 stars 138 forks source link

Feature Child Row Tmpl (Sub-Tables) #403

Open ClarenceL opened 8 years ago

ClarenceL commented 8 years ago

Responding to comments raised in: https://github.com/aslagle/reactive-table/pull/296

Would you be able to use the tmpl option and write an external template with the expand button and helpers for the subtable?

This is an updated feature set to provide fully customizable expanded rows, and a fallback to reactive-table style child tables, it provides a simple setting to enable the expand icon on any column, as well as helpers for the subtable if desired.

With this you can easily nest entire "reactive-tables" within each other, this is pretty handy and allows me to achieve many layers of tables

Please see separate readme file: CHILD_TABLE_README.md

U54 commented 8 years ago

Edit:

When creating a custom expand button I would receive the following errors in my console when clicking the button:

Uncaught TypeError: this.expandChildren is not a function Uncaught TypeError: this.collapseChildren is not a function

I was able to fix this by changing this.collapseChildren(); and this.expandChildren(); as shown in the CHILD_TABLE_README.md to this._collapseChildren(); and this._expandChildren(); (note the '_').

Once again thanks for making this, it's really helping me out so far and I'm sure it will help a lot of other people too! Hope to see it integrated as an official feature or become some kind of addon package if possible.

ClarenceL commented 8 years ago

Ah sorry yes, I added the underscore to avoid conflicts but didn't update the documentation, I think there's a better way to do this but for now it'll do.

And yes I just built a triple nested reactive-table so it was sorely needed on my end.

aslagle commented 8 years ago

Hi @ClarenceL, thanks a lot for working on this!

This is a very useful feature and I like the way it's going, but I do have some more comments.

First, I'd like a demo app in the examples directory that someone can run to see what an expanded child looks like - either a new app or incorporate this functionality into one of the existing apps.

Second, I think the options could be simplified quite a bit. Since this is an advanced feature, I think most people who use it will want to be able to customize it. It's important to keep the options flexible but not as important to make it easy to get set up. It's ok if people need to write more code to use it, as long as they're able to configure it to work however they'd like.

So on that note, do you need to have the children.fields option for a nested reactive table? Couldn't someone use tmpl for that and put a reactive table in their template, if there was good documentation of how to do it? They'd have more control over other table options and it would be easier to maintain this code if there was only one option.

Similarly, I'm not sure expandButton needs to be an option. The helpers could be added to the regular tmpl option, and people could put their own expand button in their template. Then they'd have more control over how the expandButton appears with respect to the content, and could show or hide it depending on what the content is.

Finally, you could consider making the visibleChildren ReactiveVar an option that can be passed into the table settings. I'm not sure if this makes sense or not, but it would give people more flexibility than using the built-in helpers - for example, if you wanted to have a button outside the table that collapses or expands all rows, or if you wanted to make expanding one row automatically collapse all the others, you could modify the reactive var directly.

Thanks again for doing this, let me know what you think!

ClarenceL commented 8 years ago

Thanks for the comments, I will definitely look at them shortly, briefly skimming you're totally right about improving the ReactiveVar for the visibleChildren, I found in my testing that if the data changed reactively then this ReactiveVar was re-created, therefore any expanded rows would close, this is quite jarring.

I think the only solution is to allow the developer to pass in a session key they'd like to use to track these, and as a Session var it would persist after a data reactive update.

aslagle commented 8 years ago

For the other arguments, I've used ReactiveVar so the developer can set up a reactive function that persists the changes - to a Session variable, local storage, a database, or wherever else they want.

aslagle commented 8 years ago

You don't need to use a Session key, you just need to let the developer have access to the ReactiveVar.

juliomac commented 7 years ago

Hi @ClarenceL and @aslagle , this is quite an awesome package! Thanks for that!

I find very interesting the Child Table feature. Is the merge resolved? It would be a awesome addition to the package!

ClarenceL commented 7 years ago

I've unfortunately stopped working with Meteor so I don't know when I'll have time to review the comments and merge this properly.