fix: mouse cell selection with active editor (#1382)

* fix: handle null pointer event

* fix: maintain propagation only if editor is on current cell

.eslintignore

@@ -8,7 +8,6 @@
 *.map
 *.md
 *.zip
-*.spec.ts
 **/**/*.json
 **/**/*.js
 **/__tests__/*.*

.eslintrc

@@ -11,7 +11,7 @@
     "plugin:@typescript-eslint/recommended",
     "plugin:cypress/recommended"
   ],
-  "plugins": ["@typescript-eslint", "node"],
+  "plugins": ["@typescript-eslint", "node", "jest"],
   "root": true,
   "env": {
       "browser": true,
@@ -53,6 +53,8 @@
     "no-prototype-builtins": [0],
     "semi": "off",
     "keyword-spacing": "error",
-    "space-before-blocks": "error"
+    "space-before-blocks": "error",
+    "jest/no-focused-tests": "error",
+    "jest/no-identical-title": "error"
   }
 }

package.json

@@ -73,6 +73,7 @@
     "eslint": "^8.56.0",
     "eslint-plugin-cypress": "^2.15.1",
     "eslint-plugin-import": "^2.29.1",
+    "eslint-plugin-jest": "^27.6.3",
     "eslint-plugin-node": "^11.1.0",
     "flatpickr": "^4.6.13",
     "husky": "^9.0.10",

packages/common/src/core/__tests__/slickGrid.spec.ts

@@ -4341,6 +4341,13 @@ describe('SlickGrid core file', () => {
         expect(() => grid.getCellFromEvent(event)).toThrow('SlickGrid getCellFromNode: cannot get cell - slick-cell');
       });
 
+      it('should return null if either the native event or passed in event is not set', () => {
+        grid = new SlickGrid<any, Column>(container, data, columns, { ...defaultOptions, enableCellNavigation: true });
+
+        expect(grid.getCellFromEvent(null as any)).toBeNull();
+        expect(grid.getCellFromEvent(new SlickEventData(null))).toBeNull();
+      })
+
       it('should return null when clicked cell is not a slick-cell closest ancestor', () => {
         grid = new SlickGrid<any, Column>(container, data, columns, { ...defaultOptions, enableCellNavigation: true });
         const secondRowSlickCells = container.querySelectorAll('.slick-row:nth-child(1)');

packages/common/src/core/slickGrid.ts

@@ -5048,6 +5048,10 @@ export class SlickGrid<TData = any, C extends Column<TData> = Column<TData>, O e
    */
   getCellFromEvent(evt: Event | SlickEventData) {
     const e = evt instanceof SlickEventData ? evt.getNativeEvent() : evt;
+    if (!e) {
+      return null;
+    }
+
     const targetEvent: any = (e as TouchEvent).touches ? (e as TouchEvent).touches[0] : e;
 
     const cellNode = (e as Event & { target: HTMLElement; }).target.closest('.slick-cell');

packages/common/src/extensions/__tests__/slickCellRangeSelector.spec.ts

@@ -179,19 +179,22 @@ describe('CellRangeSelector Plugin', () => {
     jest.spyOn(gridStub, 'getCellFromEvent').mockReturnValue({ cell: 2, row: 3 });
     jest.spyOn(gridStub, 'canCellBeSelected').mockReturnValue(true);
     jest.spyOn(gridStub, 'getCellFromPoint').mockReturnValue({ cell: 4, row: 5 });
+    jest.spyOn(gridStub, 'getActiveCell').mockReturnValue({ cell: 2, row: 3 });
     const focusSpy = jest.spyOn(gridStub, 'focus');
     const scrollSpy = jest.spyOn(gridStub, 'scrollCellIntoView');
     jest.spyOn(plugin.onBeforeCellRangeSelected, 'notify').mockReturnValue({
       getReturnValue: () => true
     } as any);
+    const initEvent = new Event('dragInit');
+    const popagationSpy = jest.spyOn(initEvent, 'stopImmediatePropagation');
 
     plugin.init(gridStub);
     const decoratorShowSpy = jest.spyOn(plugin.getCellDecorator(), 'show');
 
     const scrollEvent = addVanillaEventPropagation(new Event('scroll'));
     gridStub.onScroll.notify({ scrollTop: 10, scrollLeft: 15, grid: gridStub }, scrollEvent, gridStub);
 
-    const dragEventInit = addVanillaEventPropagation(new Event('dragInit'));
+    const dragEventInit = addVanillaEventPropagation(initEvent);
     gridStub.onDragInit.notify({ offsetX: 6, offsetY: 7, row: 1, startX: 3, startY: 4 } as any, dragEventInit, gridStub);
 
     const dragEventStart = addVanillaEventPropagation(new Event('dragStart'));
@@ -206,6 +209,7 @@ describe('CellRangeSelector Plugin', () => {
     expect(decoratorShowSpy).toHaveBeenCalled();
     expect(plugin.getCurrentRange()).toEqual({ start: { cell: 4, row: 5 }, end: {} });
     expect(scrollSpy).not.toHaveBeenCalled();
+    expect(popagationSpy).toHaveBeenCalled();
   });
 
   it('should handle drag in bottom right canvas with decorator showing dragging range', () => {
@@ -701,4 +705,102 @@ describe('CellRangeSelector Plugin', () => {
       done();
     }, 7);
   });
+
+  it('should maintain propagation only if editor is on current cell', () => {
+    mockGridOptions.frozenRow = 2;
+    const divCanvas = document.createElement('div');
+    const divViewport = document.createElement('div');
+    divViewport.className = 'slick-viewport';
+    divCanvas.className = 'grid-canvas-bottom grid-canvas-left';
+    divViewport.appendChild(divCanvas);
+    jest.spyOn(gridStub, 'getActiveViewportNode').mockReturnValue(divViewport);
+    jest.spyOn(gridStub, 'getActiveCanvasNode').mockReturnValue(divCanvas);
+    jest.spyOn(gridStub, 'getDisplayedScrollbarDimensions').mockReturnValue({ height: 200, width: 155 });
+    jest.spyOn(gridStub, 'getAbsoluteColumnMinWidth').mockReturnValue(47);
+    jest.spyOn(gridStub, 'getCellFromEvent').mockReturnValue({ cell: 2, row: 3 });
+    jest.spyOn(gridStub, 'canCellBeSelected').mockReturnValue(true);
+    jest.spyOn(gridStub, 'getCellFromPoint').mockReturnValue({ cell: 4, row: 5 });
+    jest.spyOn(gridStub.getEditorLock(), 'isActive').mockReturnValue(true);
+    const focusSpy = jest.spyOn(gridStub, 'focus');
+    const scrollSpy = jest.spyOn(gridStub, 'scrollCellIntoView');
+    jest.spyOn(plugin.onBeforeCellRangeSelected, 'notify').mockReturnValue({
+      getReturnValue: () => true
+    } as any);
+
+    jest.spyOn(gridStub, 'getActiveCell').mockReturnValue({ row: 3, cell: 2 });
+    plugin.init(gridStub);
+
+    const decoratorShowSpy = jest.spyOn(plugin.getCellDecorator(), 'show');
+
+    const scrollEvent = addVanillaEventPropagation(new Event('scroll'));
+    gridStub.onScroll.notify({ scrollTop: 10, scrollLeft: 15, grid: gridStub }, scrollEvent, gridStub);
+
+    const initEvent = new Event('dragInit');
+    const propagationSpy = jest.spyOn(initEvent, 'stopImmediatePropagation');
+    const dragEventInit = addVanillaEventPropagation(initEvent);
+    gridStub.onDragInit.notify({ offsetX: 6, offsetY: 7, row: 1, startX: 3, startY: 4 } as any, dragEventInit, gridStub);
+
+    const dragEventStart = addVanillaEventPropagation(new Event('dragStart'));
+    gridStub.onDragStart.notify({ offsetX: 6, offsetY: 7, row: 1, startX: 3, startY: 4 } as any, dragEventStart, gridStub);
+
+    const dragEvent = addVanillaEventPropagation(new Event('drag'));
+    dragEvent.pageX = 0;
+    dragEvent.pageY = 0;
+    gridStub.onDrag.notify({ startX: 3, startY: 4, range: { start: { cell: 2, row: 3 }, end: { cell: 4, row: 5 } }, grid: gridStub } as any, dragEvent, gridStub);
+
+    expect(focusSpy).toHaveBeenCalled();
+    expect(decoratorShowSpy).toHaveBeenCalled();
+    expect(plugin.getCurrentRange()).toEqual({ start: { cell: 4, row: 5 }, end: {} });
+    expect(scrollSpy).not.toHaveBeenCalled();
+    expect(propagationSpy).not.toHaveBeenCalled();
+  });
+
+  it('should stop propagation if the editor is not on the current cell', () => {
+    mockGridOptions.frozenRow = 2;
+    const divCanvas = document.createElement('div');
+    const divViewport = document.createElement('div');
+    divViewport.className = 'slick-viewport';
+    divCanvas.className = 'grid-canvas-bottom grid-canvas-left';
+    divViewport.appendChild(divCanvas);
+    jest.spyOn(gridStub, 'getActiveViewportNode').mockReturnValue(divViewport);
+    jest.spyOn(gridStub, 'getActiveCanvasNode').mockReturnValue(divCanvas);
+    jest.spyOn(gridStub, 'getDisplayedScrollbarDimensions').mockReturnValue({ height: 200, width: 155 });
+    jest.spyOn(gridStub, 'getAbsoluteColumnMinWidth').mockReturnValue(47);
+    jest.spyOn(gridStub, 'getCellFromEvent').mockReturnValue({ cell: 3, row: 3 });
+    jest.spyOn(gridStub, 'canCellBeSelected').mockReturnValue(true);
+    jest.spyOn(gridStub, 'getCellFromPoint').mockReturnValue({ cell: 4, row: 5 });
+    jest.spyOn(gridStub.getEditorLock(), 'isActive').mockReturnValue(true);
+    const focusSpy = jest.spyOn(gridStub, 'focus');
+    const scrollSpy = jest.spyOn(gridStub, 'scrollCellIntoView');
+    jest.spyOn(plugin.onBeforeCellRangeSelected, 'notify').mockReturnValue({
+      getReturnValue: () => true
+    } as any);
+
+    jest.spyOn(gridStub, 'getActiveCell').mockReturnValue({ row: 3, cell: 2 });
+    plugin.init(gridStub);
+
+    const decoratorShowSpy = jest.spyOn(plugin.getCellDecorator(), 'show');
+
+    const scrollEvent = addVanillaEventPropagation(new Event('scroll'));
+    gridStub.onScroll.notify({ scrollTop: 10, scrollLeft: 15, grid: gridStub }, scrollEvent, gridStub);
+
+    const initEvent = new Event('dragInit');
+    const propagationSpy = jest.spyOn(initEvent, 'stopImmediatePropagation');
+    const dragEventInit = addVanillaEventPropagation(initEvent);
+    gridStub.onDragInit.notify({ offsetX: 6, offsetY: 7, row: 1, startX: 3, startY: 4 } as any, dragEventInit, gridStub);
+
+    const dragEventStart = addVanillaEventPropagation(new Event('dragStart'));
+    gridStub.onDragStart.notify({ offsetX: 6, offsetY: 7, row: 1, startX: 3, startY: 4 } as any, dragEventStart, gridStub);
+
+    const dragEvent = addVanillaEventPropagation(new Event('drag'));
+    dragEvent.pageX = 0;
+    dragEvent.pageY = 0;
+    gridStub.onDrag.notify({ startX: 3, startY: 4, range: { start: { cell: 2, row: 3 }, end: { cell: 4, row: 5 } }, grid: gridStub } as any, dragEvent, gridStub);
+
+    expect(focusSpy).toHaveBeenCalled();
+    expect(decoratorShowSpy).toHaveBeenCalled();
+    expect(plugin.getCurrentRange()).toEqual({ start: { cell: 4, row: 5 }, end: {} });
+    expect(scrollSpy).not.toHaveBeenCalled();
+    expect(propagationSpy).toHaveBeenCalled();
+  });
 });

packages/common/src/extensions/__tests__/slickCellSelectionModel.spec.ts

@@ -191,9 +191,11 @@ describe('CellSelectionModel Plugin', () => {
     ]);
   });
 
-  it('should return False when onBeforeCellRangeSelected is called and getEditorLock returns False', () => {
+  it('should return False when onBeforeCellRangeSelected is called, getEditorLock returns False and the current cell is the active cell (within editor)', () => {
     const mouseEvent = addVanillaEventPropagation(new Event('mouseenter'));
     jest.spyOn(gridStub.getEditorLock(), 'isActive').mockReturnValue(true);
+    jest.spyOn(gridStub, 'getActiveCell').mockReturnValue({ cell: 2, row: 3 });
+    jest.spyOn(gridStub, 'getCellFromEvent').mockReturnValue({ cell: 2, row: 3 });
     const stopPropSpy = jest.spyOn(mouseEvent, 'stopPropagation');
 
     plugin.init(gridStub);

packages/common/src/extensions/slickCellRangeSelector.ts

@@ -346,8 +346,14 @@ export class SlickCellRangeSelector {
     }
 
     // prevent the grid from cancelling drag'n'drop by default
-    // unless an editor is open if so keep bubbling the event to avoid breaking editor inputs focusing/selecting
-    if (!this._grid.getEditorLock().isActive()) {
+    // unless an editor is open on the current cell.
+    // if so keep bubbling the event to avoid breaking editor inputs focusing/selecting
+    const cell = this._grid.getCellFromEvent(e);
+    const activeCell = this._grid.getActiveCell();
+
+    if (!this._grid.getEditorLock().isActive()
+      || !(activeCell && cell && activeCell.row === cell.row && activeCell.cell === cell.cell)
+    ) {
       e.stopImmediatePropagation();
       e.preventDefault();
     }

packages/common/src/extensions/slickCellSelectionModel.ts

@@ -163,7 +163,12 @@ export class SlickCellSelectionModel implements SelectionModel {
   }
 
   protected handleBeforeCellRangeSelected(e: SlickEventData): boolean | void {
-    if (this._grid.getEditorLock().isActive()) {
+    const cell = this._grid.getCellFromEvent(e);
+    const activeCell = this._grid.getActiveCell();
+
+    if (this._grid.getEditorLock().isActive()
+      && (activeCell && cell && activeCell.row === cell.row && activeCell.cell === cell.cell)
+    ) {
       e.stopPropagation();
       return false;
     }

packages/composite-editor-component/src/slick-composite-editor.component.spec.ts

@@ -1190,53 +1190,6 @@ describe('CompositeEditorService', () => {
           done();
         });
       });
-
-      it('should handle saving and grid changes when save button is clicked and user provides a custom "onSave" async function', (done) => {
-        const newGridOptions = { ...gridOptionsMock, enableAddRow: true };
-        const mockNewProduct = { address: { zip: 345678 }, product: { name: 'Product DEF', price: 22.33 } };
-        const getDataItemSpy = jest.spyOn(gridStub, 'getDataItem').mockReturnValueOnce(mockNewProduct);
-        jest.spyOn(gridStub, 'getOptions').mockReturnValue(newGridOptions);
-        (getEditControllerMock as any).commitCurrentEdit = () => {
-          gridStub.onAddNewRow.notify({ grid: gridStub, item: mockNewProduct, column: columnsMock[0] });
-          return true;
-        };
-        const mockEditOnSave = jest.fn();
-        mockEditOnSave.mockResolvedValue(Promise.resolve(true));
-
-        const mockModalOptions = { headerTitle: 'Details', modalType: 'edit', insertNewId: 3, onSave: mockEditOnSave } as CompositeEditorOpenDetailOption;
-        component = new SlickCompositeEditorComponent();
-        component.init(gridStub, container);
-        component.openDetails(mockModalOptions);
-        component.editors = { productName: { setValue: jest.fn(), isValueChanged: () => true } as unknown as Editor }; // return True for value changed
-
-        const compositeContainerElm = document.querySelector('div.slick-editor-modal.slickgrid_123456') as HTMLSelectElement;
-        const compositeFooterElm = compositeContainerElm.querySelector('.slick-editor-modal-footer') as HTMLSelectElement;
-        const compositeFooterSaveBtnElm = compositeFooterElm.querySelector('.btn-save') as HTMLSelectElement;
-        const compositeHeaderElm = document.querySelector('.slick-editor-modal-header') as HTMLSelectElement;
-        const compositeTitleElm = compositeHeaderElm.querySelector('.slick-editor-modal-title') as HTMLSelectElement;
-        const compositeBodyElm = document.querySelector('.slick-editor-modal-body') as HTMLSelectElement;
-        const productNameDetailContainerElm = compositeBodyElm.querySelector('.item-details-container.editor-productName.slick-col-medium-12') as HTMLSelectElement;
-        const productNameDetailCellElm = productNameDetailContainerElm.querySelector('.slick-cell') as HTMLSelectElement;
-        const productNameLabelElm = productNameDetailContainerElm.querySelector('.item-details-label.editor-productName') as HTMLSelectElement;
-
-        gridStub.onCompositeEditorChange.notify({ row: 0, cell: 0, column: columnsMock[0], item: mockNewProduct, formValues: { productName: 'test' }, editors: {}, grid: gridStub });
-
-        compositeFooterSaveBtnElm.click();
-
-        setTimeout(() => {
-          expect(component).toBeTruthy();
-          expect(component.constructor).toBeDefined();
-          expect(compositeContainerElm).toBeTruthy();
-          expect(compositeHeaderElm).toBeTruthy();
-          expect(compositeTitleElm).toBeTruthy();
-          expect(compositeTitleElm.textContent).toBe('Details');
-          expect(productNameLabelElm.textContent).toBe('Product');
-          expect(productNameDetailCellElm.classList.contains('modified')).toBe(true);
-          expect(gridStub.getDataItem).toHaveBeenCalled();
-          expect(mockEditOnSave).toHaveBeenCalledWith({ productName: 'test' }, { gridRowIndexes: [], dataContextIds: [] }, { ...mockNewProduct, id: 3 });
-          done();
-        });
-      });
     });
 
     describe('Form Logics', () => {

packages/empty-warning-component/src/slick-empty-warning.spec.ts

@@ -13,9 +13,9 @@ const mockGridOptions = {
 const gridStub = {
   applyHtmlCode: (elm, val) => {
     if (val instanceof HTMLElement || val instanceof DocumentFragment) {
-      elm.appendChild(val)
+      elm.appendChild(val);
     } else {
-      elm.innerHTML = val || ''
+      elm.innerHTML = val || '';
     }
   },
   getGridPosition: () => mockGridOptions,

packages/event-pub-sub/src/eventPubSub.service.spec.ts

@@ -101,7 +101,7 @@ describe('EventPubSub Service', () => {
       expect(mockCallback).toHaveBeenCalledWith({ name: 'John' });
     });
 
-    it('should call subscribe method and expect "addEventListener" and "getEventNameByNamingConvention" to be called', () => {
+    it('should call subscribe method and expect "addEventListener" and "getEventNameByNamingConvention" to be called with kebabCase', () => {
       const addEventSpy = jest.spyOn(divContainer, 'addEventListener');
       const getEventNameSpy = jest.spyOn(service, 'getEventNameByNamingConvention');
       const mockCallback = jest.fn();
@@ -141,7 +141,7 @@ describe('EventPubSub Service', () => {
       // expect(mockCallback).toHaveBeenCalledWith({ detail: { name: 'John' } });
     });
 
-    it('should call subscribe method and expect "addEventListener" and "getEventNameByNamingConvention" to be called', () => {
+    it('should call subscribe method and expect "addEventListener" and "getEventNameByNamingConvention" to be called with kebabCase', () => {
       const addEventSpy = jest.spyOn(divContainer, 'addEventListener');
       const getEventNameSpy = jest.spyOn(service, 'getEventNameByNamingConvention');
       const mockCallback = jest.fn();

packages/excel-export/src/excelExport.service.spec.ts

@@ -220,7 +220,7 @@ describe('ExcelExportService', () => {
         mockExportExcelOptions = {
           filename: 'export',
           format: FileType.xlsx
-        }
+        };
       });
 
       it(`should have the Order exported correctly with multiple formatters which have 1 of them returning an object with a text property (instead of simple string)`, async () => {

packages/excel-export/src/excelUtils.spec.ts

@@ -626,15 +626,6 @@ describe('excelUtils', () => {
         expect(output).toEqual({ groupType: 'sum', stylesheetFormatter: { id: 135 } });
       });
 
-      it('should get excel excel metadata style format for GroupTotalFormatters.sumTotalsCurrencyColored', () => {
-        const column = {
-          type: FieldType.number, formatter: Formatters.decimal, groupTotalsFormatter: GroupTotalFormatters.sumTotalsCurrencyColored,
-        } as Column;
-        const output = getExcelFormatFromGridFormatter(stylesheetStub, {}, column, gridStub, 'group');
-
-        expect(output).toEqual({ groupType: 'sum', stylesheetFormatter: { id: 135 } });
-      });
-
       it('should get excel excel metadata style format for GroupTotalFormatters.sumTotalsDollarColoredBold', () => {
         const column = {
           type: FieldType.number, formatter: Formatters.decimal, groupTotalsFormatter: GroupTotalFormatters.sumTotalsDollarColoredBold,

packages/row-detail-view-plugin/src/slickRowDetailView.spec.ts

@@ -594,7 +594,7 @@ describe('SlickRowDetailView plugin', () => {
       });
     });
 
-    it('should call "resizeDetailView" and then calculate out of range detail views when calling on scroll', () => {
+    it('should call "resizeDetailView" and then calculate out of range detail views when calling on scroll (2)', () => {
       const itemMock = {
         id: 123, firstName: 'John', lastName: 'Doe',
         _collapsed: true, _detailViewLoaded: true, _sizePadding: 1, _height: 150, _detailContent: '<span>loading...</span>',