-
Notifications
You must be signed in to change notification settings - Fork 16
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 column nesting #98
Comments
One thing to consider on this feature is sorting, In your example you show 2 headers (Employee & Quarter) and Quarter has I & II sub headers, will those be allowed to be sorted as well? Depending on the data this may become very tricky, same goes for filters and search, so usually it's easier to flatten the table instead. Also, it might be better to bisect the data per Quarter and show a table per Quarter instead? Just thinking out loud here, could be a nice addition for some edge cases for sure, but goes more towards a static table, since this kind of nesting can get extremly complex :) |
Yeah, I think so. I don't see any risks with that, if you can provide sorting path for such column (e.g. DQL path when working with Doctrine), then there should be no problem with that.
In some cases, probably. That's just a first example that came to my mind 😄 |
What's currently "blocking" me with this feature, is the column builder's API itself. Similar to many builder classes from Symfony itself, I am naming the last "build" method like a getter. For example, This, unfortunately, conflicts with the idea that
This was planned to provide a nice looking API for the nested columns: $builder
->addColumn(
$builder->createColumn('quarter', ColumnType::class) // this returns ColumnBuilderInterface
->addColumn('first', NumberColumnType::class) // this addColumn() belongs to ColumnBuilderInterface
->addColumn('second', NumberColumnType::class) // this addColumn() belongs to ColumnBuilderInterface
)
; We should either use the $builder
->addColumn(
$builder->createColumn('quarter', ColumnType::class)
->add('first', NumberColumnType::class)
->add('second', NumberColumnType::class)
)
; or name those methods with "nested" or "child" keyword: $builder
->addColumn(
$builder->createColumn('quarter', ColumnType::class)
->addNestedColumn('first', NumberColumnType::class)
->addNestedColumn('second', NumberColumnType::class)
)
; Currently, I'm liking the second option more, and it makes this possible: $builder->getColumn('quarter')->getNestedColumn('first');
$builder->getColumn('quarter')->removeNestedColumn('first'); Unfortunately, more than two-level nesting may look weird when mixing with the $builder->addColumn(
$builder->createColumn('foo')->addNestedColumn(
$builder->createColumn('bar')->addNestedColumn('baz')
)
);
// or
$columnFoo = $builder->createColumn('foo');
$columnBar = $builder->createColumn('bar');
$columnBaz = $builder->createColumn('baz');
$columnFoo->addNestedColumn($columnBar);
$columnBar->addNestedColumn($columnBaz);
$builder->addColumn($columnFoo); But I think that's the compromise we have to make. |
Currently, the
DataTableBuilderInterface::addColumn
method currently acceptsColumnBuilderInterface
as an argument, but nesting columns is not yet supported.I'm thinking of some simple, yet friendly API, similar to forms. Let's assume, that we have to render a table like that:
To achieve such hierarchy, I'm thinking on something like this:
This way, we've created a data table that consists of two columns - employee and quarter, but the latter contains two children columns - respectively first and second.
If you create a view of such data table, you should notice two header rows, with appropriate HTML attributes automatically assigned to the first one:
This corresponds to the following HTML structure:
This would require following changes:
createColumn()
toDataTableBuilderInterface
, that returnsColumnBuilderInterface
, similar toFormBuilderInterface::create()
.ColumnBuilderInterface
(addColumn()
,removeColumn()
, etc.), so API remains consistent. Naming those methodsadd
andremove
may seem more intuitive, but I think it would be confusing if used in this case:rowspan
andcolspan
HTML attributes - preferably in theColumnHeaderView->attr
array.DataTableView
to accept array ofHeaderRowView
, instead of just one.sortablejs
, and it already supports nesting! Although, we have to prevent moving nested column outside of their parent.Some breaking changes, but nothing too extreme. Probably needs more changes, but that's how I see it as today.
The text was updated successfully, but these errors were encountered: