Skip to content
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

No default sort field #10

Open
seydu opened this issue Sep 10, 2018 · 4 comments
Open

No default sort field #10

seydu opened this issue Sep 10, 2018 · 4 comments

Comments

@seydu
Copy link
Contributor

seydu commented Sep 10, 2018

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
Copy link
Member

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
Copy link
Member

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
Copy link
Contributor Author

seydu commented Oct 16, 2018

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
Copy link
Member

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.

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

No branches or pull requests

2 participants