diff --git a/src/js/modules/SelectRow/SelectRow.js b/src/js/modules/SelectRow/SelectRow.js index beb557a3d..4c6971d52 100644 --- a/src/js/modules/SelectRow/SelectRow.js +++ b/src/js/modules/SelectRow/SelectRow.js @@ -258,7 +258,7 @@ export default class SelectRow extends Module{ if(Array.isArray(rowMatch)){ if(rowMatch.length){ rowMatch.forEach((row) => { - change = this._selectRow(row, true, true); + change = this._selectRow(row, true); if(change){ changes.push(change); @@ -269,11 +269,11 @@ export default class SelectRow extends Module{ } }else{ if(rowMatch){ - this._selectRow(rowMatch, false, true); + this._selectRow(rowMatch, false); } - } + } } - + //select an individual row _selectRow(rowInfo, silent, force){ //handle max row count diff --git a/test/unit/modules/SelectRow.spec.js b/test/unit/modules/SelectRow.spec.js index de16ee76d..60c0050dc 100644 --- a/test/unit/modules/SelectRow.spec.js +++ b/test/unit/modules/SelectRow.spec.js @@ -388,6 +388,53 @@ describe("SelectRow module", () => { expect(result).toBe(false); }); + it("should enforce selectableRows limit when selectRows is called (issue #4881)", () => { + // Reproduces https://github.com/tabulator-tables/tabulator/issues/4881 + // selectRow (public API) was ignoring the selectableRows limit because + // selectRows() passed force=true to _selectRow, bypassing the cap. + + // Limit to a single selected row, no rolling reselection assumptions needed + mockTable.options.selectableRows = 1; + mockTable.options.selectableRowsRollingSelection = true; + + selectRowMod.selectedRows = []; + + mockTable.rowManager.findRow.mockImplementation((arg) => { + if (typeof arg === 'object' && arg !== null) return arg; + return mockRows.find(r => r.data.id === arg); + }); + + // Call selectRow for each row individually, mirroring the jsfiddle in the issue + selectRowMod.selectRows(mockRows[0]); + selectRowMod.selectRows(mockRows[1]); + selectRowMod.selectRows(mockRows[2]); + + // With selectableRows = 1, no more than one row should ever be selected + expect(selectRowMod.selectedRows.length).toBe(1); + // With rolling selection on, the most recently requested row should be the one selected + expect(selectRowMod.selectedRows).toContain(mockRows[2]); + expect(selectRowMod.selectedRows).not.toContain(mockRows[0]); + expect(selectRowMod.selectedRows).not.toContain(mockRows[1]); + }); + + it("should not exceed selectableRows limit when selectRows is passed an array (issue #4881)", () => { + mockTable.options.selectableRows = 2; + mockTable.options.selectableRowsRollingSelection = false; + + selectRowMod.selectedRows = []; + + mockTable.rowManager.findRow.mockImplementation((arg) => { + if (typeof arg === 'object' && arg !== null) return arg; + return mockRows.find(r => r.data.id === arg); + }); + + // Pass all three rows at once; only the first two should be selected + selectRowMod.selectRows([mockRows[0], mockRows[1], mockRows[2]]); + + expect(selectRowMod.selectedRows.length).toBe(2); + expect(selectRowMod.selectedRows).not.toContain(mockRows[2]); + }); + it("should handle rolling selection when max rows is reached", () => { // Set maximum of 2 selectable rows with rolling selection mockTable.options.selectableRows = 2;