fix: regression with onSelectedRowsChanged not receiving correct `c…

…aller` prop (#1341) - 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 } }`
ghiscoding · Jan 17, 2024 · 03cad4a · 03cad4a
1 parent d9c714c
commit 03cad4a
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 10 deletions.
diff --git a/packages/common/src/extensions/__tests__/slickCellSelectionModel.spec.ts b/packages/common/src/extensions/__tests__/slickCellSelectionModel.spec.ts
@@ -23,7 +23,7 @@ const addVanillaEventPropagation = function (event, commandKeys: string[] = [],
     Object.defineProperty(event, 'key', { writable: true, configurable: true, value: keyName });
   }
   return event;
-}
+};
 
 const mockGridOptions = {
   frozenColumn: 1,
@@ -341,7 +341,14 @@ 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' } }) })
+    );
+    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"', () => {

diff --git a/packages/common/src/extensions/__tests__/slickRowSelectionModel.spec.ts b/packages/common/src/extensions/__tests__/slickRowSelectionModel.spec.ts
@@ -19,7 +19,7 @@ const addVanillaEventPropagation = function (event, commandKey = '', keyName = '
     Object.defineProperty(event, 'key', { writable: true, configurable: true, value: keyName });
   }
   return event;
-}
+};
 
 const mockGridOptions = {
   frozenColumn: 1,
@@ -201,9 +201,18 @@ describe('SlickRowSelectionModel Plugin', () => {
 
     expect(onSelectedRangeSpy).toHaveBeenCalledWith(
       [new SlickRange(0, 0, 0, 2)],
-      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([new SlickRange(0, 0, 0, 2)], caller);
+
+    expect(onSelectedRangeSpy).toHaveBeenCalledWith(
+      [new SlickRange(0, 0, 0, 2)],
+      expect.objectContaining({ event: expect.objectContaining({ detail: { caller } }) }));
   });
 
   it('should call "setSelectedRanges" with Slick Ranges when triggered by "onActiveCellChanged" and "selectActiveRow" is True', () => {

diff --git a/packages/common/src/extensions/slickCellSelectionModel.ts b/packages/common/src/extensions/slickCellSelectionModel.ts
@@ -138,8 +138,9 @@ export class SlickCellSelectionModel implements SelectionModel {
 
     this._ranges = this.removeInvalidRanges(ranges);
     if (rangeHasChanged) {
-      const eventData = new SlickEventData();
-      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 SlickEventData(new CustomEvent('click', { detail: { caller } }), this._ranges);
       this.onSelectedRangesChanged.notify(this._ranges, eventData);
     }
   }

diff --git a/packages/common/src/extensions/slickRowSelectionModel.ts b/packages/common/src/extensions/slickRowSelectionModel.ts
@@ -117,8 +117,10 @@ export class SlickRowSelectionModel implements SelectionModel {
       return;
     }
     this._ranges = ranges;
-    const eventData = new SlickEventData();
-    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 SlickEventData(new CustomEvent('click', { detail: { caller } }), this._ranges);
     this.onSelectedRangesChanged.notify(this._ranges, eventData);
   }