fix: regression onSelectedRowsChanged not receiving correct caller (#1342)

* fix: regression `onSelectedRowsChanged` not receiving correct `caller`
- a regression was introduced when dropping jQuery, the SlickEvent structure changed in the `notify` function. Previously a SlickEvent would accept a CustomEvent directly and the previous code was expecting that event to exists and override its CustomEvent `detail`, however the newer approach is to always use a SlickEventData and no longer use the CustomEvent directly and this caused the regression since the SlickEventData doesn't have a `detail` property but rather something like this `SlickEventData { event: { detail } }`
- the fix is to simply create a CustomEvent with `{ detail: caller }` which we then pass to the `SlickEventData` constructor so that our `caller` isn't lost and rather reused later when triggered by `onSelectedRowsChanged`

Author: ghiscoding

.github/workflows/cypress.yml

@@ -4,6 +4,8 @@ on:
   push:
     branches:
       - master
+      - next
+      - version3
   pull_request:
     branches:
       - '**'

.github/workflows/main.yml

@@ -1,11 +1,11 @@
 # CI Build & Jest Unit Tests (ship smaller name for CI badge)
 name: Build & Tests
 on:
-  # Trigger the workflow on push or pull request,
-  # but only for the master branch on Push and any branches on PR
   push:
     branches:
       - master
+      - next
+      - version3
   pull_request:
     branches:
       - '**'

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

@@ -22,7 +22,7 @@ const addVanillaEventPropagation = function (event, commandKeys: string[] = [],
     Object.defineProperty(event, 'key', { writable: true, configurable: true, value: keyName });
   }
   return event;
-}
+};
 
 const mockGridOptions = {
   frozenColumn: 1,
@@ -44,7 +44,7 @@ const mockColumns = [
   { id: 'firstName', field: 'firstName' },
   { id: 'lastName', field: 'lastName' },
   { id: 'age', field: 'age' },
-]
+];
 
 const gridStub = {
   canCellBeSelected: jest.fn(),
@@ -330,7 +330,10 @@ describe('CellSelectionModel Plugin', () => {
     expect(setSelectRangeSpy).toHaveBeenCalledWith(expectedRangeCalled);
     expect(scrollCellSpy).toHaveBeenCalledWith(4, 2, false);
     expect(scrollRowSpy).toHaveBeenCalledWith(4);
-    expect(onSelectedRangeSpy).toHaveBeenCalledWith(expectedRangeCalled, expect.objectContaining({ detail: { caller: 'SlickCellSelectionModel.setSelectedRanges' } }));
+    expect(onSelectedRangeSpy).toHaveBeenCalledWith(
+      expectedRangeCalled,
+      expect.objectContaining({ event: expect.objectContaining({ detail: { caller: 'SlickCellSelectionModel.setSelectedRanges' } }) })
+    );
   });
 
   it('should call "setSelectedRanges" with Slick Range from current position to a calculated size of a page down when using Shift+PageDown key combo when triggered by "onKeyDown"', () => {

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

@@ -18,7 +18,7 @@ const addVanillaEventPropagation = function (event, commandKey = '', keyName = '
     Object.defineProperty(event, 'key', { writable: true, configurable: true, value: keyName });
   }
   return event;
-}
+};
 
 const mockGridOptions = {
   frozenColumn: 1,
@@ -192,9 +192,18 @@ describe('SlickRowSelectionModel Plugin', () => {
 
     expect(onSelectedRangeSpy).toHaveBeenCalledWith(
       [{ fromCell: 0, fromRow: 0, toCell: 2, toRow: 0, }],
-      expect.objectContaining({
-        detail: { caller: 'SlickRowSelectionModel.setSelectedRanges' }
-      }));
+      expect.objectContaining({ event: expect.objectContaining({ detail: { caller: 'SlickRowSelectionModel.setSelectedRanges' } }) }));
+  });
+
+  it('should call "setSelectedRanges" with valid ranges input with a "caller" defined and expect to "onSelectedRangesChanged" to be triggered', () => {
+    const caller = 'click.toggle';
+    const onSelectedRangeSpy = jest.spyOn(plugin.onSelectedRangesChanged, 'notify');
+
+    plugin.setSelectedRanges([{ fromCell: 0, fromRow: 0, toCell: 2, toRow: 0, }], caller);
+
+    expect(onSelectedRangeSpy).toHaveBeenCalledWith(
+      [{ fromCell: 0, fromRow: 0, toCell: 2, toRow: 0, }],
+      expect.objectContaining({ event: expect.objectContaining({ detail: { caller } }) }));
   });
 
   it('should call "setSelectedRanges" with Slick Ranges when triggered by "onActiveCellChanged" and "selectActiveRow" is True', () => {

packages/common/src/extensions/slickCellSelectionModel.ts

@@ -149,8 +149,9 @@ export class SlickCellSelectionModel {
 
     this._ranges = this.removeInvalidRanges(ranges);
     if (rangeHasChanged) {
-      const eventData = new Slick.EventData();
-      Object.defineProperty(eventData, 'detail', { writable: true, configurable: true, value: { caller } });
+      // provide extra "caller" argument through SlickEventData event to avoid breaking the previous pubsub event structure
+      // that only accepts an array of selected range `SlickRange[]`, the SlickEventData args will be merged and used later by `onSelectedRowsChanged`
+      const eventData = new Slick.EventData(new CustomEvent('click', { detail: { caller } }), this._ranges);
       this.onSelectedRangesChanged.notify(this._ranges, eventData);
     }
   }

packages/common/src/extensions/slickRowSelectionModel.ts

@@ -120,8 +120,9 @@ export class SlickRowSelectionModel {
       return;
     }
     this._ranges = ranges;
-    const eventData = new Slick.EventData();
-    Object.defineProperty(eventData, 'detail', { writable: true, configurable: true, value: { caller } });
+    // provide extra "caller" argument through SlickEventData event to avoid breaking the previous pubsub event structure
+    // that only accepts an array of selected range `SlickRange[]`, the SlickEventData args will be merged and used later by `onSelectedRowsChanged`
+    const eventData = new Slick.EventData(new CustomEvent('click', { detail: { caller } }), this._ranges);
     this.onSelectedRangesChanged.notify(this._ranges, eventData);
   }
 
@@ -221,7 +222,7 @@ export class SlickRowSelectionModel {
         selectedRows = [activeRow.row];
       }
 
-      let active:number;
+      let active: number;
       let top = selectedRows[0];
       let bottom = selectedRows[selectedRows.length - 1];
 

packages/common/src/interfaces/slickNamespace.interface.ts

@@ -100,7 +100,7 @@ export interface SlickNamespace {
    * An event object for passing data to event handlers and letting them control propagation.
    * This is pretty much identical to how W3C implement events.
    */
-  EventData: new () => SlickEventData;
+  EventData: new (e?: Event, args?: any) => SlickEventData;
 
   /** EventHandler is a Pub/Sub SlickGrid Event Handler */
   EventHandler: new () => SlickEventHandler;
@@ -195,13 +195,13 @@ export interface SlickNamespace {
     innerSize: (elm: HTMLElement, type: 'height' | 'width') => number;
     height: (elm: HTMLElement, val?: number | string) => number | void;
     width: (elm: HTMLElement, val?: number | string) => number | void;
-    offset: (elm: HTMLElement) => undefined | { top: number, left: number };
+    offset: (elm: HTMLElement) => undefined | { top: number, left: number; };
     isEmptyObject: (obj: any) => boolean;
     parents: (elm: HTMLElement, selector: string) => HTMLElement[];
     setStyleSize: (elm: HTMLElement, style: string, val: string | (() => string)) => void;
     hide: (elm: HTMLElement, type?: string) => void;
     show: (elm: HTMLElement, type?: string) => void;
     toFloat: (val: number) => number;
     windowScrollPosition: () => { top: number; left: number; };
-  }
+  };
 }