-
Notifications
You must be signed in to change notification settings - Fork 106
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
Implement Grid Sorting #163
Conversation
Resolve #157 |
I'm not sure why coverage drop... |
docs/table.rst
Outdated
$table->sort_by = 'name'; | ||
$table->sort_order = 'ascending'; | ||
|
||
This will highlight the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
something missing in text
src/Grid.php
Outdated
@@ -55,6 +55,12 @@ class Grid extends View | |||
public $selection = null; | |||
|
|||
/** | |||
* Table can be sorted by clicking on column headers. This will be automatically enabled |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Grid can be sorted ..." not Table
return $this->model = $this->table->setModel($model, $columns); | ||
$this->model = $this->table->setModel($model, $columns); | ||
|
||
if ($this->sortable === null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not simply setting $this->sortable=true as default when we define Grid class?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we will try and detect capability of a model to set order. E.g. if Array persistence does not support, then this will be disabled. Similar with pagination.
@@ -83,10 +83,8 @@ public function setAttr($attr, $value, $position = 'body') | |||
* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no description of new $attr parameter
When grid is associated with a model that supports order, it will automatically make itself sortable. You can | ||
override this behaviour by setting $sortable property to `true` or `false`. | ||
|
||
Additionally you may set list of sortable fields to a sortable property if you wish that your grid would be |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see how this is implemented - "set list of sortable fields ..."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you are right it's not, but we will need to have PR into agile data to make it work like this. A to-do before release.
$tag was ignored anyway.
Table will now have controls for displaying sorting (non-interractive) and Grid takes it to the next level by enabling dynamic sorting.
Too bad mouse is invisible...