fix(plugins): clicking a grid cell should close any open menu (#1515)

- clicking a grid cell should close any opened menu, except Cell Menu.
ghiscoding · May 8, 2024 · 383792d · 383792d
1 parent 9c1afd4
commit 383792d
Show file tree

Hide file tree

Showing 8 changed files with 52 additions and 24 deletions.
diff --git a/packages/common/src/extensions/__tests__/slickColumnPicker.spec.ts b/packages/common/src/extensions/__tests__/slickColumnPicker.spec.ts
@@ -21,6 +21,7 @@ const gridStub = {
   setColumns: jest.fn(),
   setOptions: jest.fn(),
   setSelectedRows: jest.fn(),
+  onClick: new SlickEvent(),
   onColumnsReordered: new SlickEvent(),
   onHeaderContextMenu: new SlickEvent(),
 } as unknown as SlickGrid;
@@ -147,10 +148,15 @@ describe('ColumnPickerControl', () => {
       gridStub.onHeaderContextMenu.notify({ column: columnsMock[1], grid: gridStub }, eventData as any, gridStub);
       control.menuElement!.querySelector('input[type="checkbox"]')!.dispatchEvent(new Event('click', { bubbles: true }));
 
-      expect(handlerSpy).toHaveBeenCalledTimes(2);
+      expect(handlerSpy).toHaveBeenCalledTimes(3);
       expect(readjustSpy).toHaveBeenCalledWith(0, columnsMock, columnsMock);
       expect(control.getAllColumns()).toEqual(columnsMock);
       expect(control.getVisibleColumns()).toEqual(columnsMock);
+
+      // cell click should close it
+      gridStub.onClick.notify({ row: 1, cell: 2, grid: gridStub }, eventData as any, gridStub);
+
+      expect(control.menuElement).toBeFalsy();
     });
 
     it('should query an input checkbox change event and expect "headerColumnValueExtractor" method to be called when defined', () => {
@@ -166,11 +172,11 @@ describe('ColumnPickerControl', () => {
       control.menuElement!.querySelector<HTMLInputElement>('input[type="checkbox"]')!.dispatchEvent(new Event('click', { bubbles: true }));
       const liElmList = control.menuElement!.querySelectorAll<HTMLLIElement>('li');
 
-      expect(handlerSpy).toHaveBeenCalledTimes(2);
+      expect(handlerSpy).toHaveBeenCalledTimes(3);
       expect(readjustSpy).toHaveBeenCalledWith(0, columnsMock, columnsMock);
       expect(control.getAllColumns()).toEqual(columnsMock);
       expect(control.getVisibleColumns()).toEqual(columnsMock);
-      expect(liElmList[2].textContent).toBe('Billing - Field 3')
+      expect(liElmList[2].textContent).toBe('Billing - Field 3');
     });
 
     it('should open the column picker via "onHeaderContextMenu" and expect "Forcefit" to be checked when "hideForceFitButton" is false', () => {
@@ -187,11 +193,11 @@ describe('ColumnPickerControl', () => {
       const inputForcefitElm = control.menuElement!.querySelector('#slickgrid_124343-colpicker-forcefit') as HTMLInputElement;
       const labelSyncElm = control.menuElement!.querySelector('label[for=slickgrid_124343-colpicker-forcefit]') as HTMLDivElement;
 
-      expect(handlerSpy).toHaveBeenCalledTimes(2);
+      expect(handlerSpy).toHaveBeenCalledTimes(3);
       expect(control.getAllColumns()).toEqual(columnsMock);
       expect(control.getVisibleColumns()).toEqual(columnsMock);
       expect(inputForcefitElm.checked).toBeTruthy();
-      expect(inputForcefitElm.dataset.option).toBe('autoresize')
+      expect(inputForcefitElm.dataset.option).toBe('autoresize');
       expect(labelSyncElm.textContent).toBe('Force fit columns');
     });
 
@@ -209,7 +215,7 @@ describe('ColumnPickerControl', () => {
       const inputSyncElm = control.menuElement!.querySelector('#slickgrid_124343-colpicker-syncresize') as HTMLInputElement;
       const labelSyncElm = control.menuElement!.querySelector('label[for=slickgrid_124343-colpicker-syncresize]') as HTMLDivElement;
 
-      expect(handlerSpy).toHaveBeenCalledTimes(2);
+      expect(handlerSpy).toHaveBeenCalledTimes(3);
       expect(control.getAllColumns()).toEqual(columnsMock);
       expect(control.getVisibleColumns()).toEqual(columnsMock);
       expect(inputSyncElm.checked).toBeTruthy();
@@ -238,7 +244,7 @@ describe('ColumnPickerControl', () => {
         visibleColumns: columnsMock,
         grid: gridStub,
       };
-      expect(handlerSpy).toHaveBeenCalledTimes(2);
+      expect(handlerSpy).toHaveBeenCalledTimes(3);
       expect(control.getAllColumns()).toEqual(columnsMock);
       expect(control.getVisibleColumns()).toEqual(columnsMock);
       expect(onColChangedMock).toBeCalledWith(expect.anything(), expectedCallbackArgs);
@@ -262,7 +268,7 @@ describe('ColumnPickerControl', () => {
       const labelSyncElm = control.menuElement!.querySelector('label[for=slickgrid_124343-colpicker-forcefit]') as HTMLDivElement;
       inputForcefitElm.dispatchEvent(new Event('click', { bubbles: true }));
 
-      expect(handlerSpy).toHaveBeenCalledTimes(2);
+      expect(handlerSpy).toHaveBeenCalledTimes(3);
       expect(control.getAllColumns()).toEqual(columnsMock);
       expect(inputForcefitElm.checked).toBeTruthy();
       expect(inputForcefitElm.dataset.option).toBe('autoresize');
@@ -288,7 +294,7 @@ describe('ColumnPickerControl', () => {
       const labelSyncElm = control.menuElement!.querySelector('label[for=slickgrid_124343-colpicker-syncresize]') as HTMLDivElement;
       inputSyncElm.dispatchEvent(new Event('click', { bubbles: true }));
 
-      expect(handlerSpy).toHaveBeenCalledTimes(2);
+      expect(handlerSpy).toHaveBeenCalledTimes(3);
       expect(control.getAllColumns()).toEqual(columnsMock);
       expect(inputSyncElm.checked).toBeTruthy();
       expect(inputSyncElm.dataset.option).toBe('syncresize');
@@ -334,7 +340,7 @@ describe('ColumnPickerControl', () => {
         control.menuElement!.querySelector<HTMLInputElement>('input[type="checkbox"]')!.dispatchEvent(new Event('click', { bubbles: true }));
         const col4 = control.menuElement!.querySelector<HTMLInputElement>('li.hidden input[data-columnid=field4]');
 
-        expect(handlerSpy).toHaveBeenCalledTimes(2);
+        expect(handlerSpy).toHaveBeenCalledTimes(3);
         expect(control.getAllColumns()).toEqual(columnsMock);
         expect(control.getVisibleColumns()).toEqual(columnsMock);
         expect(control.columns).toEqual(columnsMock);
@@ -362,7 +368,7 @@ describe('ColumnPickerControl', () => {
       const labelForcefitElm = control.menuElement!.querySelector('label[for=slickgrid_124343-colpicker-forcefit]') as HTMLDivElement;
       const labelSyncElm = control.menuElement!.querySelector('label[for=slickgrid_124343-colpicker-syncresize]') as HTMLDivElement;
 
-      expect(handlerSpy).toHaveBeenCalledTimes(2);
+      expect(handlerSpy).toHaveBeenCalledTimes(3);
       expect(labelForcefitElm.textContent).toBe('Ajustement forcé des colonnes');
       expect(labelSyncElm.textContent).toBe('Redimension synchrone');
       expect(utilitySpy).toHaveBeenCalled();

diff --git a/packages/common/src/extensions/__tests__/slickContextMenu.spec.ts b/packages/common/src/extensions/__tests__/slickContextMenu.spec.ts
@@ -1,12 +1,11 @@
 import { BasePubSubService } from '@slickgrid-universal/event-pub-sub';
 import { deepCopy } from '@slickgrid-universal/utils';
-import { type SlickDataView, SlickEvent, SlickEventData, SlickGrid } from '../../core/index';
 
+import { type SlickDataView, SlickEvent, SlickEventData, SlickGrid } from '../../core/index';
 import { DelimiterType, FileType } from '../../enums/index';
 import type { ContextMenu, Column, ElementPosition, GridOption, MenuCommandItem, MenuOptionItem, Formatter } from '../../interfaces/index';
 import { BackendUtilityService, ExcelExportService, SharedService, TextExportService, TreeDataService, } from '../../services/index';
 import { ExtensionUtility } from '../../extensions/extensionUtility';
-import { Formatters } from '../../formatters';
 import { TranslateServiceStub } from '../../../../../test/translateServiceStub';
 import { SlickContextMenu } from '../slickContextMenu';
 
@@ -281,6 +280,12 @@ describe('ContextMenu Plugin', () => {
 
       expect(contextMenuElm).toBeTruthy();
       expect(contextMenuElm.classList.contains('slick-dark-mode')).toBeTruthy();
+
+      // cell click should close it
+      gridStub.onClick.notify({ row: 1, cell: 2, grid: gridStub }, eventData as any, gridStub);
+      contextMenuElm = document.body.querySelector('.slick-context-menu.slickgrid12345') as HTMLDivElement;
+
+      expect(contextMenuElm).toBeNull();
     });
 
     it('should "autoAlignSide" and expect menu to aligned left with a calculate offset when showing menu', () => {

diff --git a/packages/common/src/extensions/__tests__/slickGridMenu.spec.ts b/packages/common/src/extensions/__tests__/slickGridMenu.spec.ts
@@ -62,6 +62,7 @@ const gridStub = {
   setOptions: jest.fn(),
   scrollColumnIntoView: jest.fn(),
   onBeforeDestroy: new SlickEvent(),
+  onClick: new SlickEvent(),
   onColumnsReordered: new SlickEvent(),
   onSetOptions: new SlickEvent(),
 } as unknown as SlickGrid;
@@ -263,10 +264,15 @@ describe('GridMenuControl', () => {
         buttonElm.dispatchEvent(new Event('click', { bubbles: true, cancelable: true, composed: false }));
         control.menuElement!.querySelector('input[type="checkbox"]')!.dispatchEvent(new Event('click', { bubbles: true }));
 
-        expect(handlerSpy).toHaveBeenCalledTimes(3);
+        expect(handlerSpy).toHaveBeenCalledTimes(4);
         expect(readjustSpy).toHaveBeenCalledWith(0, columnsMock, columnsMock);
         expect(control.getAllColumns()).toEqual(columnsMock);
         expect(control.getVisibleColumns()).toEqual(columnsMock);
+
+        // cell click should close it
+        gridStub.onClick.notify({ row: 1, cell: 2, grid: gridStub }, eventData as any, gridStub);
+
+        expect(control.menuElement).toBeFalsy();
       });
 
       it('should query an input checkbox change event and expect "readjustFrozenColumnIndexWhenNeeded" method to be called when the grid is detected to be a frozen grid', () => {
@@ -282,7 +288,7 @@ describe('GridMenuControl', () => {
         buttonElm.dispatchEvent(new Event('click', { bubbles: true, cancelable: true, composed: false }));
         control.menuElement!.querySelector('input[type="checkbox"]')!.dispatchEvent(new Event('click', { bubbles: true }));
 
-        expect(handlerSpy).toHaveBeenCalledTimes(3);
+        expect(handlerSpy).toHaveBeenCalledTimes(4);
         expect(readjustSpy).toHaveBeenCalledWith(0, columnsMock, columnsMock);
         expect(control.getAllColumns()).toEqual(columnsMock);
         expect(control.getVisibleColumns()).toEqual(columnsMock);
@@ -315,7 +321,7 @@ describe('GridMenuControl', () => {
         control.menuElement!.querySelector('input[type="checkbox"]')!.dispatchEvent(new Event('click', { bubbles: true }));
         const liElmList = control.menuElement!.querySelectorAll<HTMLLIElement>('li');
 
-        expect(handlerSpy).toHaveBeenCalledTimes(3);
+        expect(handlerSpy).toHaveBeenCalledTimes(4);
         expect(readjustSpy).toHaveBeenCalledWith(0, columnsMock, columnsMock);
         expect(control.getAllColumns()).toEqual(columnsMock);
         expect(control.getVisibleColumns()).toEqual(columnsMock);
@@ -337,7 +343,7 @@ describe('GridMenuControl', () => {
         control.menuElement!.querySelector('input[type="checkbox"]')!.dispatchEvent(new Event('click', { bubbles: true }));
         const liElmList = control.menuElement!.querySelectorAll<HTMLLIElement>('li');
 
-        expect(handlerSpy).toHaveBeenCalledTimes(3);
+        expect(handlerSpy).toHaveBeenCalledTimes(4);
         expect(readjustSpy).toHaveBeenCalledWith(0, columnsMock, columnsMock);
         expect(control.getAllColumns()).toEqual(columnsMock);
         expect(control.getVisibleColumns()).toEqual(columnsMock);
@@ -427,7 +433,7 @@ describe('GridMenuControl', () => {
         const inputForcefitElm = control.menuElement!.querySelector('#slickgrid_124343-gridmenu-colpicker-forcefit') as HTMLInputElement;
         const labelSyncElm = control.menuElement!.querySelector('label[for=slickgrid_124343-gridmenu-colpicker-forcefit]') as HTMLLabelElement;
 
-        expect(handlerSpy).toHaveBeenCalledTimes(3);
+        expect(handlerSpy).toHaveBeenCalledTimes(4);
         expect(control.getAllColumns()).toEqual(columnsMock);
         expect(control.getVisibleColumns()).toEqual(columnsMock);
         expect(inputForcefitElm.checked).toBeTruthy();
@@ -450,7 +456,7 @@ describe('GridMenuControl', () => {
         const inputSyncElm = control.menuElement!.querySelector('#slickgrid_124343-gridmenu-colpicker-syncresize') as HTMLInputElement;
         const labelSyncElm = control.menuElement!.querySelector('label[for=slickgrid_124343-gridmenu-colpicker-syncresize]') as HTMLLabelElement;
 
-        expect(handlerSpy).toHaveBeenCalledTimes(3);
+        expect(handlerSpy).toHaveBeenCalledTimes(4);
         expect(control.getAllColumns()).toEqual(columnsMock);
         expect(control.getVisibleColumns()).toEqual(columnsMock);
         expect(inputSyncElm.checked).toBeTruthy();
@@ -480,7 +486,7 @@ describe('GridMenuControl', () => {
           visibleColumns: columnsMock,
           grid: gridStub,
         };
-        expect(handlerSpy).toHaveBeenCalledTimes(3);
+        expect(handlerSpy).toHaveBeenCalledTimes(4);
         expect(control.getAllColumns()).toEqual(columnsMock);
         expect(control.getVisibleColumns()).toEqual(columnsMock);
         expect(onColChangedMock).toBeCalledWith(expect.anything(), expectedCallbackArgs);
@@ -505,7 +511,7 @@ describe('GridMenuControl', () => {
         const labelSyncElm = control.menuElement!.querySelector('label[for=slickgrid_124343-gridmenu-colpicker-forcefit]') as HTMLLabelElement;
         inputForcefitElm.dispatchEvent(new Event('click', { bubbles: true }));
 
-        expect(handlerSpy).toHaveBeenCalledTimes(3);
+        expect(handlerSpy).toHaveBeenCalledTimes(4);
         expect(control.getAllColumns()).toEqual(columnsMock);
         expect(inputForcefitElm.checked).toBeTruthy();
         expect(inputForcefitElm.dataset.option).toBe('autoresize');
@@ -532,7 +538,7 @@ describe('GridMenuControl', () => {
         const labelSyncElm = control.menuElement!.querySelector('label[for=slickgrid_124343-gridmenu-colpicker-syncresize]') as HTMLLabelElement;
         inputSyncElm.dispatchEvent(new Event('click', { bubbles: true }));
 
-        expect(handlerSpy).toHaveBeenCalledTimes(3);
+        expect(handlerSpy).toHaveBeenCalledTimes(4);
         expect(control.getAllColumns()).toEqual(columnsMock);
         expect(inputSyncElm.checked).toBeTruthy();
         expect(inputSyncElm.dataset.option).toBe('syncresize');
@@ -1572,7 +1578,7 @@ describe('GridMenuControl', () => {
         gridStub.onColumnsReordered.notify({ impactedColumns: columnsUnorderedMock, grid: gridStub }, eventData as any, gridStub);
         control.menuElement!.querySelector('input[type="checkbox"]')!.dispatchEvent(new Event('click', { bubbles: true }));
 
-        expect(handlerSpy).toHaveBeenCalledTimes(3);
+        expect(handlerSpy).toHaveBeenCalledTimes(4);
         expect(control.getAllColumns()).toEqual(columnsMock);
         expect(control.getVisibleColumns()).toEqual(columnsMock);
         expect(control.columns).toEqual(columnsMock);
@@ -1614,7 +1620,7 @@ describe('GridMenuControl', () => {
       const labelForcefitElm = control.menuElement!.querySelector('label[for=slickgrid_124343-gridmenu-colpicker-forcefit]') as HTMLLabelElement;
       const labelSyncElm = control.menuElement!.querySelector('label[for=slickgrid_124343-gridmenu-colpicker-syncresize]') as HTMLLabelElement;
 
-      expect(handlerSpy).toHaveBeenCalledTimes(3);
+      expect(handlerSpy).toHaveBeenCalledTimes(4);
       expect(labelForcefitElm.textContent).toBe('Ajustement forcé des colonnes');
       expect(labelSyncElm.textContent).toBe('Redimension synchrone');
       expect(utilitySpy).toHaveBeenCalled();

diff --git a/packages/common/src/extensions/__tests__/slickHeaderMenu.spec.ts b/packages/common/src/extensions/__tests__/slickHeaderMenu.spec.ts
@@ -59,6 +59,7 @@ const gridStub = {
   updateColumnHeader: jest.fn(),
   onBeforeSetColumns: new SlickEvent(),
   onBeforeHeaderCellDestroy: new SlickEvent(),
+  onClick: new SlickEvent(),
   onHeaderCellRendered: new SlickEvent(),
   onHeaderMouseEnter: new SlickEvent(),
   onMouseEnter: new SlickEvent(),
@@ -735,6 +736,7 @@ describe('HeaderMenu Plugin', () => {
       });
 
       it('should create a Grid Menu item with commands sub-menu commandItems and expect sub-menu to be positioned on top (dropup)', () => {
+        const hideMenuSpy = jest.spyOn(plugin, 'hideMenu');
         const onCommandMock = jest.fn();
         Object.defineProperty(document.documentElement, 'clientWidth', { writable: true, configurable: true, value: 50 });
         jest.spyOn(gridStub, 'getColumns').mockReturnValueOnce(columnsMock);
@@ -775,6 +777,11 @@ describe('HeaderMenu Plugin', () => {
 
         expect(headerMenu2Elm2.classList.contains('dropup')).toBeTruthy();
         expect(headerMenu2Elm2.classList.contains('dropdown')).toBeFalsy();
+
+        // cell click should close it
+        gridStub.onClick.notify({ row: 1, cell: 2, grid: gridStub }, eventData as any, gridStub);
+
+        expect(hideMenuSpy).toHaveBeenCalled();
       });
     });
 

diff --git a/packages/common/src/extensions/slickColumnPicker.ts b/packages/common/src/extensions/slickColumnPicker.ts
@@ -101,6 +101,7 @@ export class SlickColumnPicker {
 
     this._eventHandler.subscribe(this.grid.onHeaderContextMenu, this.handleHeaderContextMenu.bind(this));
     this._eventHandler.subscribe(this.grid.onColumnsReordered, updateColumnPickerOrder.bind(this));
+    this._eventHandler.subscribe(this.grid.onClick, this.disposeMenu.bind(this));
 
     // Hide the menu on outside click.
     this._bindEventService.bind(document.body, 'mousedown', this.handleBodyMouseDown.bind(this) as EventListener, undefined, 'body');

diff --git a/packages/common/src/extensions/slickContextMenu.ts b/packages/common/src/extensions/slickContextMenu.ts
@@ -75,6 +75,7 @@ export class SlickContextMenu extends MenuFromCellBaseClass<ContextMenu> {
     this.sortMenuItems();
 
     this._eventHandler.subscribe(this.grid.onContextMenu, this.handleOnContextMenu.bind(this));
+    this._eventHandler.subscribe(this.grid.onClick, this.hideMenu.bind(this));
 
     if (this._addonOptions.hideMenuOnScroll) {
       this._eventHandler.subscribe(this.grid.onScroll, this.closeMenu.bind(this));

diff --git a/packages/common/src/extensions/slickGridMenu.ts b/packages/common/src/extensions/slickGridMenu.ts
@@ -114,6 +114,7 @@ export class SlickGridMenu extends MenuBaseClass<GridMenu> {
   initEventHandlers() {
     // when grid columns are reordered then we also need to update/resync our picker column in the same order
     this._eventHandler.subscribe(this.grid.onColumnsReordered, updateColumnPickerOrder.bind(this));
+    this._eventHandler.subscribe(this.grid.onClick, (e) => this.hideMenu(e as any));
 
     // subscribe to the grid, when it's destroyed, we should also destroy the Grid Menu
     this._eventHandler.subscribe(this.grid.onBeforeDestroy, this.dispose.bind(this));

diff --git a/packages/common/src/extensions/slickHeaderMenu.ts b/packages/common/src/extensions/slickHeaderMenu.ts
@@ -79,6 +79,7 @@ export class SlickHeaderMenu extends MenuBaseClass<HeaderMenu> {
     });
     this._eventHandler.subscribe(this.grid.onHeaderCellRendered, this.handleHeaderCellRendered.bind(this));
     this._eventHandler.subscribe(this.grid.onBeforeHeaderCellDestroy, this.handleBeforeHeaderCellDestroy.bind(this));
+    this._eventHandler.subscribe(this.grid.onClick, this.hideMenu.bind(this));
 
     // force the grid to re-render the header after the events are hooked up.
     this.grid.setColumns(this.grid.getColumns());