Feature/multi column sort #86
Conversation
Replace @todo form deprecated reason
Set default value for $sortedFields when $sorter is null
To enable, simply call `enableDataTables` on a Component. Developer Notes: table-main.js is built from table.js using webpack, as such some dependencies are missing since Dewdrop does not currently track node module dependencies. In the mean time you can test this by running the following: npm install webpack npm install datatables.net npm install datatables.net-bs npm install imports-loader npm install jquery-param Also you may want to make your dewdrop directory that is in bower_components be a symbolic link to the corresponding one in this directory. ToDo: Add ability to globally enable DataTables (not just via Component)
|
I added support for DataTables, still need to include webpack and get it checked out by others before merging. Waiting on development of others before committing node modules for DataTables and other dependencies to avoid conflicts. |
|
Note to self: Update the Contributing Wiki after this is merged as to how to use Yarn. |
Moved DataTables logic into a listing handler, Index page now uses methods on the Listing to render critical sections such as the table and pagination.
To enable DataTables in your application:
For entire application:
Pimple::getInstance()['listing-handler'] = new Listing\DataTables();
For a component:
public function getListing()
{
if (!this->listing) {
$this->listing = new Listing($adminListing, $primaryKey, $request, new Listing\DataTables();
}
return $this->listing;
}
|
Should I go ahead and merge this into master? |
| /** | ||
| * Get the field the Select is currently sorted by. | ||
| * | ||
| * @deprecated Use getSortedFields() instead. |
There was a problem hiding this comment.
Curious what the plan is for eventually removing these @deprecated methods (e.g., version 2.0.0)?
There was a problem hiding this comment.
I am unaware of any roadmap for Dewdrop, would be happy to help start one. These methods are still supported at line 254
|
|
||
| $out .= <<<HTML | ||
| <script type="text/javascript"> | ||
| var INDEX_TABLE; |
There was a problem hiding this comment.
Does this approach limit us to one table per request/page? I'm not sure offhand if we're already limited in this way, but I'd hesitate to introduce such a limitation.
There was a problem hiding this comment.
It does, we already are limited, at least on the frontend. The backend supports prefixes for multiple table views. There are few existing implementations of that and those are already custom. This entire feature is essentially opt in, so updating to it should introduce any unexpected behaviors.
Dewdrop/View/Helper/Table.php
Outdated
| $out = ''; | ||
| $sortedFields = ($sorter) ? $sorter->getSortedFields() : []; | ||
|
|
||
| //var_dump($sortedFields); |
Dewdrop/View/Helper/Table.php
Outdated
| $out .= '</th>'; | ||
| } | ||
|
|
||
| //die; |
There was a problem hiding this comment.
This, and the other comment about //var_dump have been resolved in 7057b71
package.json
Outdated
| @@ -0,0 +1,27 @@ | |||
| { | |||
| "name": "dewdrop", | |||
| "version": "1.27.0", | |||
There was a problem hiding this comment.
Do we need to flag this for updating immediately prior to, or as part of, the release process?
There was a problem hiding this comment.
That was added by Yarn, I think I can just remove the version
|
I'm very much looking forward to the availability of these features in Dewdrop. It would be nice if we had a CI system that would support feature branches to aid in determining whether the related changes are stable and ready for merging. Perhaps this feature branch is already being successfully used by one or more projects? I'm also curious if any projects that are not using the DataTables functionality successfully operate using this feature branch, or if there are any backward incompatibilities or risks that should be addressed? |
|
@darbyfelton I am not aware of any project using this currently, there should not be any backwards compatibility issues. We may want to put in the release notes how to enable these new features: To enable DataTables in your application: For entire application: For a component: |
|
I found an issue when using this with listings that have bulk selections. So that will need to be updated before merging. |
|
Also need to add support for SortableListingInterface. Currently it just shows the sort index, not the sorting grabber. |
This is the work for #61