New version of checkbox has issues with multiselect = true + RowSelectionModel.selectActiveRow = true #345
Comments
SatanEnglish
changed the title
|
New version
Old version did This is without holding CTR or Shift which still do the correct funcitonality |
|
What do you mean by old vs new versions? Do you mean since Frozen feature was introduced? Also if you look at the other example-checkbox-row-select.html it does support multi-select well enough. From what I understood with how to have single versus multiple selection, is to use EDIT I also pulled latest code of slickgrid master, opened the |
|
@ghiscoding Also it doesn't seem to work how expected which was what i've shown in the gifs.
PS: Your example is working but it never has worked if you hold control and click. Here is the piece of code to fix that as I've never uploaded the fix by the looks of it. Inside the function handleKeyDown(e) {
var activeRow = _grid.getActiveCell();
if (activeRow && e.shiftKey && !e.ctrlKey && !e.altKey && !e.metaKey && (e.which == 38 || e.which == 40)) {
var selectedRows = getSelectedRows();
selectedRows.sort(function (x, y) {
return x - y
});
if (!selectedRows.length) {
selectedRows = [activeRow.row];
}
var top = selectedRows[0];
var bottom = selectedRows[selectedRows.length - 1];
var active;
if (e.which == 40) {
active = activeRow.row < bottom || top == bottom ? ++bottom : ++top;
} else {
active = activeRow.row < bottom ? --bottom : --top;
}
+ // Edited to make so that multiSelect via arrows with shift or Ctrl won't happen
+ if (active >= 0 && active < _grid.getDataLength()) {
+ _grid.scrollRowIntoView(active);
+ if (!_grid.getOptions().multiSelect) {
+ top = active;
+ bottom = active;
+ }
+ var tempRanges = rowsToRanges(getRowsRange(top, bottom));
+ setSelectedRanges(tempRanges);
+ }
e.preventDefault();
e.stopPropagation();
}
} |
|
Here is my example of how it worked / works with my changes.
Here you can see select checkboxes allows multi select clicking a row deselects all but selects single. Here is how multiselect = false. Here you can see select checkboxes only one active at a time clicking row deselects all but selects single. I've not used the second example if I need single select I always just use selectActiveRow = true and turn off checkboxes |
|
@ghiscoding @6pac The functionality of these things should now change. I've also given you the fix for if holding control or shift. (slick.rowselectionmodel.js fix) Let me know if you still don't have enough information to understand the issue here and I can give step by step what to click and do to get the functionality of this plugin back to what it use to do. Thanks |
|
I didn't even know we could use the I'll have to re-read everything another day, I'm just too tired now and I don't know what was the old way vs new way since I didn't know we could certain keyboard keys in combo with this plugin |
|
@SatanEnglish can this be close since PR #343 ? |
|
@ghiscoding However may pay to do the fix to the |
|
@ghiscoding |
|
I didn't yet, I was focusing on the other PRs first. You could create a PR if you want. |
|
@SatanEnglish The first thing I see is that, only a small piece of code is different compare to the function handleKeyDown(e) {
var activeRow = _grid.getActiveCell();
if (activeRow && e.shiftKey && !e.ctrlKey && !e.altKey && !e.metaKey && (e.which == 38 || e.which == 40)) {
// .... some code.... removed for brevity of this post
if (e.which == 40) {
active = activeRow.row < bottom || top == bottom ? ++bottom : ++top;
} else {
active = activeRow.row < bottom ? --bottom : --top;
}
+ // Edited to make so that multiSelect via arrows with shift or Ctrl won't happen
if (active >= 0 && active < _grid.getDataLength()) {
_grid.scrollRowIntoView(active);
+ if (!_grid.getOptions().multiSelect) {
+ top = active;
+ bottom = active;
+ }
var tempRanges = rowsToRanges(getRowsRange(top, bottom));
setSelectedRanges(tempRanges);
}
e.preventDefault();
e.stopPropagation();
}
}However even if I add that piece of code, it doesn't seem to change anything on my side. That is, if I enable both flags like so Here's an animated gif of that test I did, the last step I'm doing is with the Also note, you don't seem to have latest code since this line (118 for me)
|
|
@6pac |
|
My first instinct is to agree that However, looking at the linked sample, this is a very useful feature, but it probably should be called So we either view this as an overloaded flag that has slightly different meaning depending on the value of |
|
@6pac @SatanEnglish |
|
@ghiscoding + if (!_grid.getOptions().multiSelect) {
+ top = active;
+ bottom = active;
+ }The code above is for when you have {multiSelect: false, selectActiveRow: true} PS: Stops what happens in Gif in #345 (comment) |
|
@ghiscoding @6pac If you look at the code |
|
@SatanEnglish |
|
@ghiscoding |
|
@ghiscoding @6pac changed the code 6 days ago. Main change is that it will never get in there for multiSelect anymore. + if (activeRow
- if (_grid.getOptions().multiSelect && activeRow Can no longer use row selection with multiSelect which don't work for me. |
|
... not aware of changing anything recently to do with this? there are small unrelated changes being applied from time to time that may screw up line numbers tho, and the odd pull from a PR ... |
|
@6pac Thiis the link |
|
@6pac I should also look into making the checkbox plugin taking into account the grid.options.multiSelect |
|
ah yes, that was a PR |
|
feel free to change that. the PR seemed innocuous enough to me at the time, i though it was just about shift-clicking :-) |
|
@6pac FYI the changes that @SatanEnglish found in this comment was not enough to fix the entire problem (I tried it) and he did mention that there's other issues, in particular 4 conditions that needs to work. It's been a busy week on my side, so I don't have much time to help unfortunately. |
|
@6pac @ghiscoding Sorry still haven't found time to play with this again yet will try look within next couple of weeks |
|
Notes for the 4 cases for when I next get a chance to work on this again or incase someone else finds time. Need to fix up to work for all 4 cases. Should be able to select multiple rows via check box and ctrl/Shift clicking. These should highlight and select checkboxes if using ctrl/Shift. (Clicking directly on a checkbox is equivalent to holding ctrl and clicking a row) Should be able to select multiple rows via check box and ctrl/Shift clicking. NO rows should highlight If using Checkboxes should only allow one to be selected. (Needs update to checkbox plugin) If using Checkboxes should only allow one to be selected. (Needs update to checkbox plugin) May edit this if I think of more information that i've missed about the cases |
|
I am using RowSelectionModel. Can anyone help me out over here? |
|
@swatiwak |
|
@SatanEnglish I have installed slickgrid. I have imported below files as per my need. When I select any row, only last column is highlighted. If I redraw table complete row is highlighted. |
|
@swatiwak |
On examples like http://6pac.github.io/SlickGrid/examples/example-checkbox-header-row.html
If you turn on Checkbox multiselect and
RowSelecitonModel.selectActiveRowThe new version will not allow multiselect to work.
Old version would allow you to select each row by the check box and have multiple selected.
New version does not
The above is deleting the row even tho it is the same row.
The issue seems to be one of the commented out rows below.
function toggleRowSelection(row) { if (_selectedRowsLookup[row]) { _grid.setSelectedRows($.grep(_grid.getSelectedRows(), function (n) { return n != row })); } else { _grid.setSelectedRows(_grid.getSelectedRows().concat(row)); } + //_grid.setActiveCell(row, getCheckboxColumnCellIndex()); + //_grid.focus(); }The set and then focus seems to be acting like a double click on the checkbox/row
The text was updated successfully, but these errors were encountered: