data-table-bundle icon indicating copy to clipboard operation
data-table-bundle copied to clipboard

Add column nesting

Open Kreyu opened this issue 1 year ago • 3 comments

Currently, the DataTableBuilderInterface::addColumn method currently accepts ColumnBuilderInterface 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:

|            |          Quarter           |
|  Employee  | ----------- | ------------ |
|            |      I      |      II      |
| ---------- | ----------- | ------------ |
|  John Doe  |     100     |      50      |
|  Jane Doe  |     200     |      75      |

To achieve such hierarchy, I'm thinking on something like this:

namespace App\DataTable\Type;

use Kreyu\Bundle\DataTableBundle\Column\Type\ColumnType;
use Kreyu\Bundle\DataTableBundle\Column\Type\NumberColumnType;
use Kreyu\Bundle\DataTableBundle\Column\Type\TextColumnType;
use Kreyu\Bundle\DataTableBundle\Type\AbstractDataTableType;
use Kreyu\Bundle\DataTableBundle\DataTableBuilderInterface;

class ProductDataTableType extends AbstractDataTableType
{
    public function buildDataTable(DataTableBuilderInterface $builder, array $options): void
    {
        $builder
            ->addColumn('employee', TextColumnType::class)
            ->addColumn(
                $builder->createColumn('quarter', ColumnType::class)
                    ->addColumn('first', NumberColumnType::class)
                    ->addColumn('second', NumberColumnType::class)
            )
        ;

    }
}

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:

/** @var \Kreyu\Bundle\DataTableBundle\DataTableView $view */

$view->headerRows[0]['employee']->vars['attr']; // ['rowspan' => 2]
$view->headerRows[0]['quarter']->vars['attr'];  // ['colspan' => 2]

This corresponds to the following HTML structure:

<thead>
    <tr>
        <th rowspan="2">Employee</th>
        <th colspan="2">Quarter</th>
    </tr>
    <tr>
        <th>I</th>
        <th>II</th>
    </tr>
</thead>

This would require following changes:

  1. Adding createColumn() to DataTableBuilderInterface, that returns ColumnBuilderInterface, similar to FormBuilderInterface::create().
  2. Adding column-related methods methods to the ColumnBuilderInterface (addColumn(), removeColumn(), etc.), so API remains consistent. Naming those methods add and remove may seem more intuitive, but I think it would be confusing if used in this case:
$builder
    ->addColumn('employee', TextColumnType::class)
    ->addColumn(
        $builder->createColumn('quarter', ColumnType::class)
            ->add('first', NumberColumnType::class) // "add" what? 
            ->add('second', NumberColumnType::class) // "add" what?
    )
;
  1. Adding a mechanism to automatically apply rowspan and colspan HTML attributes - preferably in the ColumnHeaderView->attr array.
  2. Changing DataTableView to accept array of HeaderRowView, instead of just one.
  3. Some small changes in themes to render multiple headers
  4. Something with personalization - the built-in scripts are using 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.

Kreyu avatar Jun 19 '24 14:06 Kreyu

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 :)

CMH-Benny avatar Jun 27 '24 15:06 CMH-Benny

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?

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.

Also, it might be better to bisect the data per Quarter and show a table per Quarter instead?

In some cases, probably. That's just a first example that came to my mind 😄

Kreyu avatar Jun 28 '24 06:06 Kreyu

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, ColumnBuilderInterface currently has a method named getColumn() that creates a ColumnInterface (finishes a building process). This is also the case for DataTableBuilderInterface::getDataTable(), FilterBuilderInterface::getFilter(), and so on, and I think it should stay like that.

This, unfortunately, conflicts with the idea that ColumnBuilderInterface should contain following methods to work on the child columns:

  • getColumn(string $name): ColumnBuilderInterface :x:
  • addColumn(ColumnBuilderInterface|string $column, ?string $type = null, array $options = []): ColumnBuilderInterface
  • and so on...

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 add, get, remove methods (I really don't like this) similar to FormBuilderInterface:

$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 DataTableBuilder::createColumn(), as there are no createNestedColumn(), as it would be totally unnecessary:

$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.

Kreyu avatar Jul 13 '24 19:07 Kreyu