feat: (re)add option to cancel Row Detail opening (#1286)

* feat: (re)add option to cancel Row Detail opening
- I tried to implement this feature in a previous PR #1125 but that introduced a regression which is got fixed in this PR and reintroduces this feature without regressing this time.

examples/vite-demo-vanilla-bundle/src/examples/example11.ts

@@ -47,7 +47,7 @@ const myCustomTitleValidator = (value) => {
 };
 
 const customEditableInputFormatter = (_row, _cell, value, columnDef, _dataContext, grid) => {
-  const gridOptions = grid?.getOptions() ?? {};
+  const gridOptions = grid.getOptions();
   const isEditableItem = gridOptions.editable && columnDef.editor;
   value = (value === null || value === undefined) ? '' : value;
   return isEditableItem ? { html: value, addClasses: 'editable-field', toolTip: 'Click to Edit' } : value;

examples/vite-demo-vanilla-bundle/src/examples/example12.ts

@@ -48,7 +48,7 @@ const myCustomTitleValidator = (value, args) => {
  * @returns {boolean} isEditable
  */
 function checkItemIsEditable(dataContext, columnDef, grid) {
-  const gridOptions = grid && grid.getOptions && grid.getOptions();
+  const gridOptions = grid.getOptions();
   const hasEditor = columnDef.editor;
   const isGridEditable = gridOptions.editable;
   let isEditable = (isGridEditable && hasEditor);

examples/vite-demo-vanilla-bundle/src/examples/example14.ts

@@ -49,7 +49,7 @@ const myCustomTitleValidator = (value) => {
  * @returns {boolean} isEditable
  */
 function checkItemIsEditable(dataContext, columnDef, grid) {
-  const gridOptions = grid && grid.getOptions && grid.getOptions();
+  const gridOptions = grid.getOptions();
   const hasEditor = columnDef.editor;
   const isGridEditable = gridOptions.editable;
   let isEditable = (isGridEditable && hasEditor);

examples/vite-demo-vanilla-bundle/src/examples/example21.ts

@@ -81,7 +81,6 @@ export default class Example21 {
       autoResize: {
         container: '.demo-container',
       },
-      enableCellNavigation: true,
       enableColumnReorder: true,
       enableFiltering: true,
       enableRowDetailView: true,
@@ -110,6 +109,7 @@ export default class Example21 {
   }
 
   changeEditableGrid() {
+    this.rowDetail.collapseAll();
     this.rowDetail.addonOptions.useRowClick = false;
     this.gridOptions.autoCommitEdit = !this.gridOptions.autoCommitEdit;
     this.sgb.slickGrid?.setOptions({
@@ -133,6 +133,15 @@ export default class Example21 {
   }
 
   addRowDetailEventHandlers() {
+    this.rowDetail.onBeforeRowDetailToggle.subscribe((_e, args) => {
+      // you coud cancel opening certain rows
+      // if (args.item.id === 1) {
+      //   e.preventDefault();
+      //   return false;
+      // }
+      console.log('before toggling row detail', args.item);
+    });
+
     this._eventHandler.subscribe(this.rowDetail.onAfterRowDetailToggle, (_e, args) => {
       console.log('after toggling row detail', args.item);
       if (args.item._collapsed) {

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

@@ -76,6 +76,16 @@ describe('SlickCore file', () => {
       ed.addReturnValue(false);
       expect(ed.getReturnValue()).toBe('last value');
     });
+
+    it('should be able to reset value returned', () => {
+      const ed = new SlickEventData();
+      ed.addReturnValue('last value');
+
+      expect(ed.getReturnValue()).toBe('last value');
+
+      ed.resetReturnValue();
+      expect(ed.getReturnValue()).toBeUndefined();
+    });
   });
 
   describe('SlickEvent class', () => {
@@ -92,6 +102,24 @@ describe('SlickCore file', () => {
       expect(onClick.subscriberCount).toBe(1);
     });
 
+    it('should be able to call notify on SlickEventData and ignore any previous value', () => {
+      const spy1 = jest.fn();
+      const spy2 = jest.fn();
+      const ed = new SlickEventData();
+      const onClick = new SlickEvent('onClick');
+      const scope = { onClick };
+      const resetValSpy = jest.spyOn(ed, 'resetReturnValue');
+      onClick.subscribe(spy1);
+      onClick.subscribe(spy2);
+
+      expect(onClick.subscriberCount).toBe(2);
+
+      onClick.notify({ hello: 'world' }, ed, scope, true);
+
+      expect(spy1).toHaveBeenCalledWith(ed, { hello: 'world' });
+      expect(resetValSpy).toHaveBeenCalled();
+    });
+
     it('should be able to subscribe to an event, call notify() and all subscribers to receive what was sent', () => {
       const spy1 = jest.fn();
       const spy2 = jest.fn();

packages/common/src/core/slickCore.ts

@@ -117,6 +117,10 @@ export class SlickEventData<ArgType = any> {
   getArguments() {
     return this._arguments;
   }
+
+  resetReturnValue() {
+    this.returnValue = undefined;
+  }
 }
 
 /**
@@ -174,8 +178,11 @@ export class SlickEvent<ArgType = any> {
    * @param {Object} [scope] - The scope ("this") within which the handler will be executed.
    *      If not specified, the scope will be set to the <code>Event</code> instance.
    */
-  notify(args: ArgType, evt?: SlickEventData | Event | MergeTypes<SlickEventData, Event> | null, scope?: any) {
+  notify(args: ArgType, evt?: SlickEventData | Event | MergeTypes<SlickEventData, Event> | null, scope?: any, ignorePrevEventDataValue = false) {
     const sed = evt instanceof SlickEventData ? evt : new SlickEventData(evt, args);
+    if (ignorePrevEventDataValue) {
+      sed.resetReturnValue();
+    }
     scope = scope || this;
 
     for (let i = 0; i < this._handlers.length && !(sed.isPropagationStopped() || sed.isImmediatePropagationStopped()); i++) {

packages/common/src/services/__tests__/extension.service.spec.ts

@@ -384,6 +384,7 @@ describe('ExtensionService', () => {
         const gridOptionsMock = { enableCheckboxSelector: true } as GridOption;
         const extCreateSpy = jest.spyOn(mockCheckboxSelectColumn, 'create').mockReturnValue(mockCheckboxSelectColumn);
         const gridSpy = jest.spyOn(SharedService.prototype, 'gridOptions', 'get').mockReturnValue(gridOptionsMock);
+        const rowSelectionSpy = jest.spyOn(SlickRowSelectionModel.prototype, 'constructor' as any);
 
         service.createExtensionsBeforeGridCreation(columnsMock, gridOptionsMock);
         service.bindDifferentExtensions();
@@ -393,6 +394,7 @@ describe('ExtensionService', () => {
         expect(gridSpy).toHaveBeenCalled();
         expect(extCreateSpy).toHaveBeenCalledWith(columnsMock, gridOptionsMock);
         expect(rowSelectionInstance).not.toBeNull();
+        expect(rowSelectionSpy).toHaveBeenCalledWith({});
         expect(output).toEqual({ name: ExtensionName.checkboxSelector, instance: mockCheckboxSelectColumn as unknown } as ExtensionModel<any>);
       });
 
@@ -401,6 +403,7 @@ describe('ExtensionService', () => {
         const gridOptionsMock = { enableRowMoveManager: true } as GridOption;
         const extCreateSpy = jest.spyOn(mockRowMoveManager, 'create').mockReturnValue(mockRowMoveManager);
         const gridSpy = jest.spyOn(SharedService.prototype, 'gridOptions', 'get').mockReturnValue(gridOptionsMock);
+        const rowSelectionSpy = jest.spyOn(SlickRowSelectionModel.prototype, 'constructor' as any);
 
         service.createExtensionsBeforeGridCreation(columnsMock, gridOptionsMock);
         service.bindDifferentExtensions();
@@ -410,6 +413,7 @@ describe('ExtensionService', () => {
         expect(gridSpy).toHaveBeenCalled();
         expect(extCreateSpy).toHaveBeenCalledWith(columnsMock, gridOptionsMock);
         expect(rowSelectionInstance).not.toBeNull();
+        expect(rowSelectionSpy).toHaveBeenCalledWith({ dragToSelect: true });
         expect(output).toEqual({ name: ExtensionName.rowMoveManager, instance: mockRowMoveManager as unknown } as ExtensionModel<any>);
       });
 

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

@@ -315,10 +315,40 @@ describe('SlickRowDetailView plugin', () => {
     expect(stopPropagationSpy).not.toHaveBeenCalled();
   });
 
-  it('should trigger onClick and ', () => {
+  it('should trigger onClick and NOT expect Row Detail to be toggled when onBeforeRowDetailToggle returns false', () => {
+    const mockProcess = jest.fn();
+    const expandDetailViewSpy = jest.spyOn(plugin, 'expandDetailView');
+    const onBeforeSlickEventData = new SlickEventData();
+    jest.spyOn(onBeforeSlickEventData, 'getReturnValue').mockReturnValue(false);
+    const beforeRowDetailToggleSpy = jest.spyOn(plugin.onBeforeRowDetailToggle, 'notify').mockReturnValueOnce(onBeforeSlickEventData);
+    const afterRowDetailToggleSpy = jest.spyOn(plugin.onAfterRowDetailToggle, 'notify');
+    const itemMock = { id: 123, firstName: 'John', lastName: 'Doe', _collapsed: true };
+    const detailView = `<span>loading...</span>`;
+    jest.spyOn(gridStub.getEditorLock(), 'isActive').mockReturnValue(false);
+    jest.spyOn(gridStub.getEditorLock(), 'commitCurrentEdit').mockReturnValue(true);
+    jest.spyOn(gridStub, 'getDataItem').mockReturnValue(itemMock);
+    jest.spyOn(gridStub, 'getColumns').mockReturnValue(mockColumns);
+    jest.spyOn(gridStub, 'getOptions').mockReturnValue({ ...gridOptionsMock, rowDetailView: { process: mockProcess, columnIndexPosition: 0, useRowClick: true, maxRows: 2, panelRows: 2 } as any });
+
+    plugin.init(gridStub);
+    plugin.onAsyncResponse.notify({ item: itemMock, itemDetail: itemMock, detailView, }, new SlickEventData());
+
+    const clickEvent = new Event('click');
+    Object.defineProperty(clickEvent, 'target', { writable: true, configurable: true, value: document.createElement('div') });
+    Object.defineProperty(clickEvent, 'isPropagationStopped', { writable: true, configurable: true, value: jest.fn() });
+    Object.defineProperty(clickEvent, 'isImmediatePropagationStopped', { writable: true, configurable: true, value: jest.fn() });
+    gridStub.onClick.notify({ row: 0, cell: 1, grid: gridStub }, clickEvent);
+
+    expect(beforeRowDetailToggleSpy).toHaveBeenCalled();
+    expect(afterRowDetailToggleSpy).not.toHaveBeenCalled();
+    expect(expandDetailViewSpy).not.toHaveBeenCalled();
+  });
+
+  it('should trigger onClick and expect Row Detail to be toggled', () => {
     const mockProcess = jest.fn();
     const expandDetailViewSpy = jest.spyOn(plugin, 'expandDetailView');
     const beforeRowDetailToggleSpy = jest.spyOn(plugin.onBeforeRowDetailToggle, 'notify');
+    const afterRowDetailToggleSpy = jest.spyOn(plugin.onAfterRowDetailToggle, 'notify');
     const itemMock = { id: 123, firstName: 'John', lastName: 'Doe', _collapsed: true };
     const detailView = `<span>loading...</span>`;
     jest.spyOn(gridStub.getEditorLock(), 'isActive').mockReturnValue(false);
@@ -337,6 +367,7 @@ describe('SlickRowDetailView plugin', () => {
     gridStub.onClick.notify({ row: 0, cell: 1, grid: gridStub }, clickEvent);
 
     expect(beforeRowDetailToggleSpy).toHaveBeenCalled();
+    expect(afterRowDetailToggleSpy).toHaveBeenCalled();
     expect(expandDetailViewSpy).toHaveBeenCalledWith({
       _collapsed: false, _detailContent: undefined, _detailViewLoaded: true,
       _height: 75, _sizePadding: 3, firstName: 'John', id: 123, lastName: 'Doe'
@@ -419,7 +450,7 @@ describe('SlickRowDetailView plugin', () => {
         _collapsed: true, _detailViewLoaded: true, _sizePadding: 0, _height: 150, _detailContent: '<span>loading...</span>',
         id: 123, firstName: 'John', lastName: 'Doe',
       }
-    });
+    }, expect.anything(), expect.anything(), true);
     expect(afterRowDetailToggleSpy).toHaveBeenCalledWith({
       grid: gridStub,
       item: {

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

@@ -1,3 +1,4 @@
+import { createDomElement, SlickEvent, SlickEventHandler, } from '@slickgrid-universal/common';
 import type {
   Column,
   DOMMouseOrTouchEvent,
@@ -19,7 +20,6 @@ import type {
   SlickEventData,
   UsabilityOverrideFn,
 } from '@slickgrid-universal/common';
-import { createDomElement, SlickEvent, SlickEventHandler, } from '@slickgrid-universal/common';
 import { extend } from '@slickgrid-universal/utils';
 
 /**
@@ -683,6 +683,7 @@ export class SlickRowDetailView implements ExternalResource, UniversalRowDetailV
   /** Handle mouse click event */
   protected handleClick(e: DOMMouseOrTouchEvent<HTMLDivElement>, args: { row: number; cell: number; }) {
     const dataContext = this._grid.getDataItem(args.row);
+
     if (this.checkExpandableOverride(args.row, dataContext, this._grid)) {
       // clicking on a row select checkbox
       const columnDef = this._grid.getColumns()[args.cell];
@@ -695,10 +696,11 @@ export class SlickRowDetailView implements ExternalResource, UniversalRowDetailV
         }
 
         // trigger an event before toggling
-        this.onBeforeRowDetailToggle.notify({
-          grid: this._grid,
-          item: dataContext
-        });
+        // user could cancel the Row Detail opening when event is returning false
+        const ignorePrevEventDataValue = true; // click event might return false from Row Selection canCellBeActive() validation, we need to ignore that
+        if (this.onBeforeRowDetailToggle.notify({ grid: this._grid, item: dataContext }, e, this, ignorePrevEventDataValue).getReturnValue() === false) {
+          return;
+        }
 
         this.toggleRowSelection(args.row, dataContext);