edhager
commented
5 years ago This change breaks how the selector is supposed to work.
Take a look at the first grid on the selector manual test page: https://github.com/SitePen/dgrid/blob/master/test/Selector.html. That grid has an allowSelect method that only allows a row to be selected if the col1 property of the data item is not "note".
In that grid, rows with ids 3, 6, 10 and 13 should not be selectable. Click the select-all checkbox at the top to see this in action.
The checkbox input is a bit odd when it comes to setting readonly and disabled when compared to a text input. Clicking on a checkbox changes its clicked state but does not, by default, change the value of the input.
- Checkbox disabled: user cannot change the clicked state.
- Checkbox readonly: user can change the clicked state.
Here is a checkbox to play with: https://jsbin.com/kozemoxagi/1/edit?html,output
With the changes in this PR, I can go to the selector test page, click on the checkbox in the row with id "3" and see the checkbox changing state. That makes me think those rows are selectable when they are not.
So I cannot merge this PR into master but if you want to override the default rendering of the selector checkbox, you can do so by adding a selector property to the column definition that is a function. The documentation describes this functionality: https://github.com/SitePen/dgrid/blob/master/doc/components/mixins/Selector.md.
Also, the selector manual test page contains an example:
Disabled elements eat click events at least in Firefox. A read-only element serves the original purpose and allows clicking on the element itself.