primefaces / primereact

The Most Complete React UI Component Library
https://primereact.org
MIT License
6.9k stars 1.05k forks source link

DataTable multiple selection with metakey doesn't work with checkbox column #2065

Closed AuthorProxy closed 3 years ago

AuthorProxy commented 3 years ago

I updated from "primereact": "^6.1.0", to "primereact": "^6.3.2",

Before I had a table with selectionMode="multiple" and metaKey, and columns with <Column selectionMode="multiple" /> and it worked like this:

When I click on checkboxes - it selects multiple rows When I click a rows without shift key - it selects only a single row and a checkbox in that row

In new version I tried different setups and cannot figure a way how to achieve this, looks like there no way now to do this. When I set selectionMode="multiple" and metaKey - it works as expected with shift key, but when I add checkbox column <Column selectionMode="multiple" /> - it disable metakey and clicking on the rows always select multiple items.

Expected behaviour checkbox column and metakey options that is worked on "primereact": "^6.1.0": Not holding shift or ctrl: When clicking on a row outside of the checkbox, single row should be selected and single checkbox should be checked When clicking on the checkboxes, multiple rows should be selected

Can you confirm a bug or give a solution?

AuthorProxy commented 3 years ago

Can you confirm @mertsincan ?

mertsincan commented 3 years ago

Hi @AuthorProxy,

This is by design. But, you can do this using onRowSelect callback. Please try; https://codesandbox.io/s/stoic-feather-x8yb1?file=/src/demo/DataTableSelectionDemo.js

Best Regards,

AuthorProxy commented 3 years ago

@mertsincan hi, your sample doesn't work. I implement something like this, but there are a lot of troubles and boilerplate code:

  1. can you add type: "checkbox" | "row" to onSelect handler, the same as it implemented on the onRowSelect handler
  2. also all selected values (not just row value) to onRowSelect, now need to add too much boilerplate to add this
  3. top SelectAll checkbox doesn't call onRowSelect event handler
  4. e.type doesn't show TD around checkbox as checkbox type, so... it's not easy to implement this with current by design
  5. data: null, checked: false some strange props from onSelectionChange that exists only on the top selectAll checkbox but doesn't exist on any other checkboxes;
  6. onCellSelect-onCellUnselect - have not called at all on my quick testing

I understand that you have a lot of tasks, but can you add some of this futures to make it more customizable or add some factories for extending\replacing default selection logic

mertsincan commented 3 years ago

Hi @AuthorProxy, Sorry for the typo! :( Please use onRowSelect instead of onSelect in the above sample. https://codesandbox.io/s/stoic-feather-x8yb1?file=/src/demo/DataTableSelectionDemo.js

  1. Done! Please use onRowSelect callback. type DataTableSelectType = 'row' | 'cell' | 'checkbox' | 'radio' | 'all';
  2. Added onAllRowsSelect and onAllRowsUnselect callbacks to DataTable; https://github.com/primefaces/primereact/issues/2093
  3. Added onAllRowsSelect and onAllRowsUnselect callbacks to DataTable; https://github.com/primefaces/primereact/issues/2093
  4. Unfortunately, I didn't understand this problem.
  5. Done! Good catch!
  6. Checkbox selection doesn't support cell selection mode. Therefore, onCellSelect-onCellUnselect will never be called.

Thanks a lot for the feedback!

AuthorProxy commented 3 years ago

@mertsincan here is full working functionality, you can check how I implemented it now, and maybe you'll get some useful thoughts for future design: https://codesandbox.io/s/frosty-jang-bi4o8?file=/src/demo/DataTableSelectionDemo.js

  1. I mean to add not just value to event, but also selectedValues property or smth like this
  2. just want to say that I cannot fully rely on the type="checkbox", because the space around the checkbox (inside current td) indicated as type="row", but I am sure it should be checkbox too, if you implement custom selection logic, because it's not easy to point mouse directly to the center of checkbox