Change the way checkbox-typed cells react to pressing SPACE and `EN…

…TER` keys. (#10802) * - Change the way checkbox-typed cells react to pressing SPACE and ENTER keys. - Modify the existing test cases and add new ones. * Add the changelog entry.
handsontable · Feb 23, 2024 · 4832dd2 · 4832dd2
1 parent bb2f966
commit 4832dd2
Show file tree

Hide file tree

Showing 3 changed files with 216 additions and 25 deletions.
diff --git a/.changelogs/10802.json b/.changelogs/10802.json
@@ -0,0 +1,8 @@
+{
+  "issuesOrigin": "private",
+  "title": "Changed the way checkbox-typed cells react to pressing `SPACE` and `ENTER` keys.",
+  "type": "changed",
+  "issueOrPR": 10802,
+  "breaking": false,
+  "framework": "none"
+}
diff --git a/handsontable/src/renderers/checkboxRenderer/__tests__/checkboxRenderer.spec.js b/handsontable/src/renderers/checkboxRenderer/__tests__/checkboxRenderer.spec.js
@@ -267,24 +267,25 @@ describe('CheckboxRenderer', () => {
     expect(getDataAtCell(199, 0)).toEqual(true);
   });
 
-  it('should reverse checkboxes state after hitting space, when multiple cells are selected', () => {
+  it('should reverse checkboxes state after hitting space, when multiple cells are selected and all of the cells share ' +
+    'the same value', () => {
     handsontable({
-      data: [[true], [false], [true]],
+      data: [[true], [true], [true]],
       columns: [
         { type: 'checkbox' }
       ]
     });
 
-    const afterChangeCallback = jasmine.createSpy('afterChangeCallback');
+    const afterChangeCallback1 = jasmine.createSpy('afterChangeCallback');
 
-    addHook('afterChange', afterChangeCallback);
+    addHook('afterChange', afterChangeCallback1);
 
     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(getData()).toEqual([[true], [false], [true]]);
+    expect(getData()).toEqual([[true], [true], [true]]);
 
     selectCell(0, 0, 2, 0);
 
@@ -293,18 +294,46 @@ describe('CheckboxRenderer', () => {
     checkboxes = spec().$container.find(':checkbox');
 
     expect(checkboxes.eq(0).prop('checked')).toBe(false);
-    expect(checkboxes.eq(1).prop('checked')).toBe(true);
+    expect(checkboxes.eq(1).prop('checked')).toBe(false);
     expect(checkboxes.eq(2).prop('checked')).toBe(false);
-    expect(getData()).toEqual([[false], [true], [false]]);
-    expect(afterChangeCallback.calls.count()).toEqual(1);
-    expect(afterChangeCallback).toHaveBeenCalledWith([
+    expect(getData()).toEqual([[false], [false], [false]]);
+    expect(afterChangeCallback1.calls.count()).toEqual(1);
+    expect(afterChangeCallback1).toHaveBeenCalledWith([
       [0, 0, true, false],
-      [1, 0, false, true],
+      [1, 0, true, false],
       [2, 0, true, false]
     ], 'edit');
+
+    updateData([[false], [false], [false]]);
+
+    const afterChangeCallback2 = jasmine.createSpy('afterChangeCallback');
+
+    addHook('afterChange', afterChangeCallback2);
+
+    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(getData()).toEqual([[false], [false], [false]]);
+
+    selectCell(0, 0, 2, 0);
+
+    keyDownUp(' ');
+
+    checkboxes = spec().$container.find(':checkbox');
+
+    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([[true], [true], [true]]);
+    expect(afterChangeCallback2.calls.count()).toEqual(1);
+    expect(afterChangeCallback2).toHaveBeenCalledWith([
+      [0, 0, false, true],
+      [1, 0, false, true],
+      [2, 0, false, true]
+    ], 'edit');
   });
 
-  it('should reverse checkboxes state after hitting space, when multiple non-contiguous cells are selected', () => {
+  it('should make all checkboxes checked after hitting space, when multiple cells are selected and they vary in value', () => {
     handsontable({
       data: [[true], [false], [true]],
       columns: [
@@ -323,7 +352,47 @@ describe('CheckboxRenderer', () => {
     expect(checkboxes.eq(2).prop('checked')).toBe(true);
     expect(getData()).toEqual([[true], [false], [true]]);
 
-    selectCells([[0, 0], [2, 0]]);
+    selectCell(0, 0, 2, 0);
+
+    keyDownUp(' ');
+
+    checkboxes = spec().$container.find(':checkbox');
+
+    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([[true], [true], [true]]);
+    expect(afterChangeCallback.calls.count()).toEqual(1);
+    expect(afterChangeCallback).toHaveBeenCalledWith([
+      [0, 0, true, true],
+      [1, 0, false, true],
+      [2, 0, true, true]
+    ], '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', () => {
+    handsontable({
+      data: [[true, true], [true, true]],
+      columns: [
+        { type: 'checkbox' },
+        { type: 'checkbox' },
+      ]
+    });
+
+    const afterChangeCallback1 = jasmine.createSpy('afterChangeCallback');
+
+    addHook('afterChange', afterChangeCallback1);
+
+    let checkboxes = spec().$container.find(':checkbox');
+
+    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([[true, true], [true, true]]);
+
+    selectCells([[0, 0, 0, 1], [1, 0, 1, 1]]);
 
     keyDownUp(' ');
 
@@ -332,8 +401,71 @@ describe('CheckboxRenderer', () => {
     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(getData()).toEqual([[false], [false], [false]]);
-    expect(afterChangeCallback.calls.count()).toEqual(2);
+    expect(checkboxes.eq(3).prop('checked')).toBe(false);
+    expect(getData()).toEqual([[false, false], [false, false]]);
+    expect(afterChangeCallback1.calls.count()).toEqual(2);
+
+    updateData([[true, true], [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(2).prop('checked')).toBe(false);
+    expect(checkboxes.eq(3).prop('checked')).toBe(false);
+    expect(getData()).toEqual([[true, true], [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(2).prop('checked')).toBe(true);
+    expect(checkboxes.eq(3).prop('checked')).toBe(true);
+    expect(getData()).toEqual([[false, false], [true, true]]);
+    expect(afterChangeCallback2.calls.count()).toEqual(2);
+  });
+
+  it('should make appropriate changes to the checkboxes\' states within their own respective sub-selections', () => {
+    handsontable({
+      data: [[true, false], [true, true]],
+      columns: [
+        { type: 'checkbox' },
+        { type: 'checkbox' },
+      ]
+    });
+
+    const afterChangeCallback1 = jasmine.createSpy('afterChangeCallback');
+
+    addHook('afterChange', afterChangeCallback1);
+
+    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(2).prop('checked')).toBe(true);
+    expect(checkboxes.eq(3).prop('checked')).toBe(true);
+    expect(getData()).toEqual([[true, false], [true, true]]);
+
+    selectCells([[0, 0, 0, 1], [1, 0, 1, 1]]);
+
+    keyDownUp(' ');
+
+    checkboxes = spec().$container.find(':checkbox');
+
+    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(afterChangeCallback1.calls.count()).toEqual(2);
   });
 
   it('should reverse checkboxes state after hitting enter, when multiple non-contiguous cells are selected', () => {
@@ -368,7 +500,46 @@ describe('CheckboxRenderer', () => {
     expect(afterChangeCallback.calls.count()).toEqual(2);
   });
 
-  it('should reverse checkboxes state after hitting space, when multiple cells are selected and selStart > selEnd', () => {
+  it('should reverse checkboxes state after hitting space, when multiple cells are selected and selStart > selEnd' +
+  '+ all the selected checkboxes have the same value', () => {
+    handsontable({
+      data: [[true], [true], [true]],
+      columns: [
+        { type: 'checkbox' }
+      ]
+    });
+
+    const afterChangeCallback = jasmine.createSpy('afterChangeCallback');
+
+    addHook('afterChange', afterChangeCallback);
+
+    let checkboxes = spec().$container.find(':checkbox');
+
+    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([[true], [true], [true]]);
+
+    selectCell(2, 0, 0, 0); // selStart = [2,0], selEnd = [0,0]
+
+    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(2).prop('checked')).toBe(false);
+    expect(getData()).toEqual([[false], [false], [false]]);
+    expect(afterChangeCallback.calls.count()).toEqual(1);
+    expect(afterChangeCallback).toHaveBeenCalledWith([
+      [0, 0, true, false],
+      [1, 0, true, false],
+      [2, 0, true, false]
+    ], 'edit');
+  });
+
+  it('should check all of the checkboxes in the selection after hitting space, when multiple cells are selected ' +
+  'and selStart > selEnd + the selected checkboxes differ in values', () => {
     handsontable({
       data: [[true], [false], [true]],
       columns: [
@@ -393,15 +564,15 @@ describe('CheckboxRenderer', () => {
 
     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(false);
-    expect(getData()).toEqual([[false], [true], [false]]);
+    expect(checkboxes.eq(2).prop('checked')).toBe(true);
+    expect(getData()).toEqual([[true], [true], [true]]);
     expect(afterChangeCallback.calls.count()).toEqual(1);
     expect(afterChangeCallback).toHaveBeenCalledWith([
-      [0, 0, true, false],
+      [0, 0, true, true],
       [1, 0, false, true],
-      [2, 0, true, false]
+      [2, 0, true, true]
     ], 'edit');
   });
 

diff --git a/handsontable/src/renderers/checkboxRenderer/checkboxRenderer.js b/handsontable/src/renderers/checkboxRenderer/checkboxRenderer.js
@@ -213,11 +213,15 @@ 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();
-      const changes = [];
+      let changes = [];
 
       for (let visualRow = startRow; visualRow <= endRow; visualRow += 1) {
         for (let visualColumn = startColumn; visualColumn <= endColumn; visualColumn += 1) {
           const cachedCellProperties = hotInstance.getCellMeta(visualRow, visualColumn);
+          const templates = {
+            checkedTemplate: cachedCellProperties.checkedTemplate,
+            uncheckedTemplate: cachedCellProperties.uncheckedTemplate,
+          };
 
           if (cachedCellProperties.type !== 'checkbox') {
             return;
@@ -239,18 +243,26 @@ export function checkboxRenderer(hotInstance, TD, row, col, prop, value, cellPro
 
           if (uncheckCheckbox === false) {
             if ([cachedCellProperties.checkedTemplate, cachedCellProperties.checkedTemplate.toString()].includes(dataAtCell)) { // eslint-disable-line max-len
-              changes.push([visualRow, visualColumn, cachedCellProperties.uncheckedTemplate]);
+              changes.push([visualRow, visualColumn, cachedCellProperties.uncheckedTemplate, templates]);
 
             } else if ([cachedCellProperties.uncheckedTemplate, cachedCellProperties.uncheckedTemplate.toString(), null, undefined].includes(dataAtCell)) { // eslint-disable-line max-len
-              changes.push([visualRow, visualColumn, cachedCellProperties.checkedTemplate]);
+              changes.push([visualRow, visualColumn, cachedCellProperties.checkedTemplate, templates]);
             }
 
           } else {
-            changes.push([visualRow, visualColumn, cachedCellProperties.uncheckedTemplate]);
+            changes.push([visualRow, visualColumn, cachedCellProperties.uncheckedTemplate, templates]);
           }
         }
       }
 
+      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) {
         hotInstance.setDataAtCell(changes);
       }