gyrocode / jquery-datatables-checkboxes

Checkboxes is an extension for the jQuery DataTables library that provides universal solution for working with checkboxes in a table.
https://www.gyrocode.com/projects/jquery-datatables-checkboxes/
MIT License
150 stars 61 forks source link

Use rowId for unique value of checkbox column #112

Closed d42ohpaz closed 4 years ago

d42ohpaz commented 4 years ago

Looking at the Known Limitations, it states:

Column containing checkboxes must have unique data. Using columns.data option set to null for the column containing checkboxes will result in unexpected behavior.

I would like to request that absent a value in columns.data(), then fallback to the rowId. If rowId is not available, then use the internal row number that is provided by datatables.

Thank you!

nikitul commented 4 years ago

@9ae8sdf76 , @mpryvkin Hello there. I made some bug fixes and some improvements including some of this request. Source: here. I supposed that if rowId is provided, it should be used. If not, is used cell.data(). I didn't used internal row number because that it could be changed using RowReorder or by adding new rows... and is a lot of extra work that could be avoided using a rowId.

d42ohpaz commented 4 years ago

Thank you for the efforts!

@nikitul do you want to create the PR, or should I?

nikitul commented 4 years ago

@9ae8sdf76 Any time. Please send me (or here) some feedback. I don't know what's a PR (I am newbie with Github) so... do it :)

d42ohpaz commented 4 years ago

@nikitul no worries; we all start somewhere. Feel free to read up on the Standard Fork and Pull Request Workflow to get more familiar with the process.

nikitul commented 4 years ago

@9ae8sdf76 Thank you.

mpryvkin commented 4 years ago

I am not sure I understand the problem.

If you want to use row ID provided by DT_RowId or another data property for the checkbox column, just use 'data': 'DT_RowId' option for your column definition. For example:

'columnDefs': [
    {
        'targets': 0,
        'checkboxes': true,
        'data': 'DT_RowId'
    }
],

This doesn't require any code changes as suggested by the pull request #114.

There are cases where checkbox can be used in multiple columns. I would rather not blindly revert to use row ID if cell data is missing and would prefer to either:

Regarding quoted limitation of the script to require unique data, I am planning to remove this limitation in future by allowing column with checkboxes to not have any data, if all is needed is the state of the checkbox.

nikitul commented 3 years ago

Sorry for this (very) late response. 2020 was very busy one... Those changes I suggested in this pull request were because in some cases it is needed to get the whole data of the checked rows. And if DataTable settings are "serverSide": true, all the current page checked rows data is gone when the page is changed and all we can get is the checked ID's. That is why I suggested also to create columns().checkboxes.ids() for returning checked id's and columns().checkboxes.data() functions for returning checked row's data .

Your 'data': 'DT_RowId' suggestion does not solve this problem id I want checked DT_RowId and cell.data() and row.data() on the same DataTable instance. It only allows to get row ID's or cell's data.

My goal was to:

That is why I made that amount of changes.

There is a bug thought (in my version of code)... When Ajax is used to supply the data and "serverSide": false and checked columns settings are 'checkboxes': {selectRow': false}, if I check the selectAll checkbox, only the visualized rows (pages) are selected (because not all rows are created in DOM).

I will try to update my version with your last changes and remove that bug.

Best Regards

nikitul commented 3 years ago

THANK YOU!

Your "1.2.13-dev" version solved all my big problems! Now it works sooooo nice with fixed columns (except dt.fixedColumns().cellIndex()), and when I use DataTable settings"serverSide": true, I capture the selected cell with 'selectCallback': function(cellNodes, isSelected){}. I don't know why I don't used this function before. There is a little problem with dt.fixedColumns().cellIndex(). That piece of code should be replaced because is deprecated. See [here](https://datatables.net/reference/api/fixedColumns().cellIndex()). I will create another Issue for that, describing it.

Thanks Again!