Prezent / prezent-grid-bundle

Integrate prezent-grid in Symfony
MIT License
6 stars 3 forks source link

No default sort field #10

Open seydu opened 6 years ago

seydu commented 6 years ago

When a grid is displayed the first time there is no active sort column. This is because the sortable extension relies only on the request to decide what sortable field is active.

We will need more than the request to make that decision. We can add optional is_default_sort and default_sort_order. When there is no sort field defined by the request these default values will be used. When will those default values be set? When building the grid or right before generating the view.

In a controller for instance or grid builder service:

$defaultSortColumn =  ....;
foreach ($gridColumns as $column => $columnConfig) {
    $columnOptions = $columnConfig['options'];
    if($column == $defaultSortColumn) {
        $columnOptions['is_default_sort'] = true;
        $columnOptions['default_sort_order'] = $orderDirection;
    }
    $gridBuilder->addColumn($column, $columnConfig['type'], $columnOptions);
}
sandermarechal commented 6 years ago

This is tricky. If there is no sorting information in the request, then the grid is unsorted. I suggest that, if you want a default sorting, then the controller (or a listener) should add this information in the request. In my controllers I usually do this like so:

class MyController extends Controller
{
    public function indexAction(Request $request)
    {
        $sortField = $request->get('sort_by', 'my_default_sort_column');
        $sortOrder = $request->get('sort_order', 'my_default_sort_order'); // e.g. "ASC"

        // Ensure that the correct field is active
        $request->query->set('sort_by', $sortField);
        $request->query->set('sort_order', $sortOrder);

        // Now, use $sortField and $sortOrder to build some kind of query
        $items = ...;

        // Create the grid
        $grid = $this->get('grid_factory')->createGrid(MyGridType::class);

        return [
            'grid' => $grid->createView(),
            'items' => $items,
        ];
    }
}

Maybe this is better solved by improving the documentation?

sandermarechal commented 6 years ago

Alternatively, if you really want to do it in the grid itself, it is not hard to make an extension that does this. I would suggest setting it as a grid option though, not as a column option. That makes it easier to reuse the grids with different default sortings. E.g:

$grid = $this->get('grid_factory')->createGrid(MyGridType::class, [
    'default_sort_field' => 'some_field',
    'default_sort_order' => 'ASC',
]);

Then, the grid extension simply looks at those two values and sets them on the current request if the sort_field and sort_order are missing.

seydu commented 6 years ago

I agree with you on that. Deciding what column is the default sort belongs to the grid. The grid level has an overview of all columns, can determine if no column is active by user input and activate the default sort column.

I would even go further and remove determining if a column is the active sorted one from the column extension.

i have integrated the library into a Laravel application in 2016. I had an extension that made the grid accessible from a column. Sortable columns would grab the grid and ask if they are currently sorted. Now I have the feeling that this was not a clean solution (columns accessing the grid). What do you think?

I had a Composite Design Pattern approach that allowed to have nested grids (for instance One to Many associations used child class grid to display associated elements).

The other issue I remember running into was that tying the logic of determining active sorted columns to the request made it almost impossible to have many grids in a request. When I moved that logic into the grid, it removed the dependency to the request. The grid expected an attribute for sorted column. If it was not set it would apply the default sort column.

sandermarechal commented 6 years ago

Now I have the feeling that this was not a clean solution (columns accessing the grid). What do you think?

I think this may have been cleaner when you leave it to the grid, not the column. But hey, if it works for you!

The other issue I remember running into was that tying the logic of determining active sorted columns to the request made it almost impossible to have many grids in a request.

That is actually a very valid complaint. I rarely use multiple grids on the same page. I will give this some thought.