Skip to content

Commit

Permalink
Modify the change from #10802 to not distinguish between sub-selectio…
Browse files Browse the repository at this point in the history 
…ns when modifying the checkboxes' state by hitting space/enter.
  • Loading branch information
@jansiegel
jansiegel committed Feb 27, 2024
1 parent 4d6ac27 commit 810eb21
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 34 deletions.
48 changes: 25 additions & 23 deletions handsontable/src/renderers/checkboxRenderer/__tests__/checkboxRenderer.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -370,8 +370,8 @@ describe('CheckboxRenderer', () => {
], 'edit');
});

it('should reverse checkboxes state within sub-selection after hitting space, when multiple non-contiguous cells are ' +
'selected and all of the cells in their respective selection share the same value', () => {
it('should reverse checkboxes state after hitting space, when multiple non-contiguous cells are ' +
'selected and all of the cells in the entire selection share the same value', () => {
handsontable({
data: [[true, true], [true, true]],
columns: [
Expand Down Expand Up @@ -405,37 +405,38 @@ describe('CheckboxRenderer', () => {
expect(getData()).toEqual([[false, false], [false, false]]);
expect(afterChangeCallback1.calls.count()).toEqual(2);

updateData([[true, true], [false, false]]);
updateData([[false, false], [false, false]]);

const afterChangeCallback2 = jasmine.createSpy('afterChangeCallback');

addHook('afterChange', afterChangeCallback2);

checkboxes = spec().$container.find(':checkbox');

expect(checkboxes.eq(0).prop('checked')).toBe(true);
expect(checkboxes.eq(1).prop('checked')).toBe(true);
expect(checkboxes.eq(0).prop('checked')).toBe(false);
expect(checkboxes.eq(1).prop('checked')).toBe(false);
expect(checkboxes.eq(2).prop('checked')).toBe(false);
expect(checkboxes.eq(3).prop('checked')).toBe(false);
expect(getData()).toEqual([[true, true], [false, false]]);
expect(getData()).toEqual([[false, false], [false, false]]);

selectCells([[0, 0, 0, 1], [1, 0, 1, 1]]);

keyDownUp(' ');

checkboxes = spec().$container.find(':checkbox');

expect(checkboxes.eq(0).prop('checked')).toBe(false);
expect(checkboxes.eq(1).prop('checked')).toBe(false);
expect(checkboxes.eq(0).prop('checked')).toBe(true);
expect(checkboxes.eq(1).prop('checked')).toBe(true);
expect(checkboxes.eq(2).prop('checked')).toBe(true);
expect(checkboxes.eq(3).prop('checked')).toBe(true);
expect(getData()).toEqual([[false, false], [true, true]]);
expect(getData()).toEqual([[true, true], [true, true]]);
expect(afterChangeCallback2.calls.count()).toEqual(2);
});

it('should make appropriate changes to the checkboxes\' states within their own respective sub-selections', () => {
it('should check all the checkboxes in the enteire selection after hitting space, when multiple non-contiguous cells ' +
'are selected and they vary in value', () => {
handsontable({
data: [[true, false], [true, true]],
data: [[true, true], [true, false]],
columns: [
{ type: 'checkbox' },
{ type: 'checkbox' },
Expand All @@ -449,10 +450,10 @@ describe('CheckboxRenderer', () => {
let checkboxes = spec().$container.find(':checkbox');

expect(checkboxes.eq(0).prop('checked')).toBe(true);
expect(checkboxes.eq(1).prop('checked')).toBe(false);
expect(checkboxes.eq(1).prop('checked')).toBe(true);
expect(checkboxes.eq(2).prop('checked')).toBe(true);
expect(checkboxes.eq(3).prop('checked')).toBe(true);
expect(getData()).toEqual([[true, false], [true, true]]);
expect(checkboxes.eq(3).prop('checked')).toBe(false);
expect(getData()).toEqual([[true, true], [true, false]]);

selectCells([[0, 0, 0, 1], [1, 0, 1, 1]]);

Expand All @@ -462,15 +463,16 @@ describe('CheckboxRenderer', () => {

expect(checkboxes.eq(0).prop('checked')).toBe(true);
expect(checkboxes.eq(1).prop('checked')).toBe(true);
expect(checkboxes.eq(2).prop('checked')).toBe(false);
expect(checkboxes.eq(3).prop('checked')).toBe(false);
expect(getData()).toEqual([[true, true], [false, false]]);
expect(checkboxes.eq(2).prop('checked')).toBe(true);
expect(checkboxes.eq(3).prop('checked')).toBe(true);
expect(getData()).toEqual([[true, true], [true, true]]);
expect(afterChangeCallback1.calls.count()).toEqual(2);
});

it('should reverse checkboxes state after hitting enter, when multiple non-contiguous cells are selected', () => {
it('should reverse checkboxes state after hitting enter, when multiple non-contiguous cells are selected and they ' +
'share the same value', () => {
handsontable({
data: [[false], [true], [true]],
data: [[true], [true], [true]],
columns: [
{ type: 'checkbox' }
]
Expand All @@ -482,21 +484,21 @@ describe('CheckboxRenderer', () => {

let checkboxes = spec().$container.find(':checkbox');

expect(checkboxes.eq(0).prop('checked')).toBe(false);
expect(checkboxes.eq(0).prop('checked')).toBe(true);
expect(checkboxes.eq(1).prop('checked')).toBe(true);
expect(checkboxes.eq(2).prop('checked')).toBe(true);
expect(getData()).toEqual([[false], [true], [true]]);
expect(getData()).toEqual([[true], [true], [true]]);

selectCells([[0, 0], [2, 0]]);

keyDownUp('enter');

checkboxes = spec().$container.find(':checkbox');

expect(checkboxes.eq(0).prop('checked')).toBe(true);
expect(checkboxes.eq(0).prop('checked')).toBe(false);
expect(checkboxes.eq(1).prop('checked')).toBe(true);
expect(checkboxes.eq(2).prop('checked')).toBe(false);
expect(getData()).toEqual([[true], [true], [false]]);
expect(getData()).toEqual([[false], [true], [false]]);
expect(afterChangeCallback.calls.count()).toEqual(2);
});

Expand Down
35 changes: 24 additions & 11 deletions handsontable/src/renderers/checkboxRenderer/checkboxRenderer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,9 @@ export function checkboxRenderer(hotInstance, TD, row, col, prop, value, cellPro
*/
function changeSelectedCheckboxesState(uncheckCheckbox = false) {
const selRange = hotInstance.getSelectedRange();
const changesPerSubSelection = [];
let changes = [];
let changeCounter = 0;

if (!selRange) {
return;
Expand All @@ -213,7 +216,6 @@ export function checkboxRenderer(hotInstance, TD, row, col, prop, value, cellPro
for (let key = 0; key < selRange.length; key++) {
const { row: startRow, col: startColumn } = selRange[key].getTopStartCorner();
const { row: endRow, col: endColumn } = selRange[key].getBottomEndCorner();
let changes = [];

for (let visualRow = startRow; visualRow <= endRow; visualRow += 1) {
for (let visualColumn = startColumn; visualColumn <= endColumn; visualColumn += 1) {
Expand Down Expand Up @@ -252,20 +254,31 @@ export function checkboxRenderer(hotInstance, TD, row, col, prop, value, cellPro
} else {
changes.push([visualRow, visualColumn, cachedCellProperties.uncheckedTemplate, templates]);
}

changeCounter += 1;
}
}

if (!changes.every(([, , cellValue]) => cellValue === changes[0][2])) {
changes = changes.map(
([visualRow, visualColumn, , templates]) => [visualRow, visualColumn, templates.checkedTemplate]
);
} else {
changes = changes.map(([visualRow, visualColumn, cellValue]) => [visualRow, visualColumn, cellValue]);
}
changesPerSubSelection.push(changeCounter);
changeCounter = 0;
}

if (changes.length > 0) {
hotInstance.setDataAtCell(changes);
}
if (!changes.every(([, , cellValue]) => cellValue === changes[0][2])) {
changes = changes.map(
([visualRow, visualColumn, , templates]) => [visualRow, visualColumn, templates.checkedTemplate]
);
} else {
changes = changes.map(([visualRow, visualColumn, cellValue]) => [visualRow, visualColumn, cellValue]);
}

if (changes.length > 0) {
// TODO: This is workaround for handsontable/dev-handsontable#1747 not being a breaking change.
// Technically, the changes don't need to be split into chunks when sent to `setDataAtCell`.
changesPerSubSelection.forEach((changesCount) => {
const changesChunk = changes.splice(0, changesCount);

hotInstance.setDataAtCell(changesChunk);
});
}
}

Expand Down

0 comments on commit 810eb21

Please sign in to comment.