From 34bae6cb37c01c63993c5acba6e58127400b88d7 Mon Sep 17 00:00:00 2001 From: ghiscoding Date: Mon, 30 Oct 2023 16:16:07 -0400 Subject: [PATCH 1/2] fix(common): context menu should close when clicking another cell - prior to this PR, when opening a context menu in Example 19 and clicking another cell would not close the opened context menu and it should. It seems to be caused by potentially other body mousedown event listeners registered outside of the lib, what we can do is use `capture: true` to make sure our listener are being dispatched first before other external ones and that fixes our problem. - apply this patch to all Menu extensions/plugins --- .../common/src/extensions/__tests__/slickContextMenu.spec.ts | 1 - packages/common/src/extensions/__tests__/slickGridMenu.spec.ts | 3 --- .../common/src/extensions/__tests__/slickHeaderButtons.spec.ts | 1 - packages/common/src/extensions/slickCellMenu.ts | 2 +- packages/common/src/extensions/slickColumnPicker.ts | 2 +- packages/common/src/extensions/slickContextMenu.ts | 2 +- packages/common/src/extensions/slickGridMenu.ts | 2 +- packages/common/src/extensions/slickHeaderMenu.ts | 2 +- 8 files changed, 5 insertions(+), 10 deletions(-) diff --git a/packages/common/src/extensions/__tests__/slickContextMenu.spec.ts b/packages/common/src/extensions/__tests__/slickContextMenu.spec.ts index 785a874fe..09b26dc8e 100644 --- a/packages/common/src/extensions/__tests__/slickContextMenu.spec.ts +++ b/packages/common/src/extensions/__tests__/slickContextMenu.spec.ts @@ -155,7 +155,6 @@ const treeDataServiceStub = { } as unknown as TreeDataService; describe('ContextMenu Plugin', () => { - const consoleWarnSpy = jest.spyOn(global.console, 'warn').mockReturnValue(); let backendUtilityService: BackendUtilityService; let extensionUtility: ExtensionUtility; let translateService: TranslateServiceStub; diff --git a/packages/common/src/extensions/__tests__/slickGridMenu.spec.ts b/packages/common/src/extensions/__tests__/slickGridMenu.spec.ts index 78a4febdd..57fd5f163 100644 --- a/packages/common/src/extensions/__tests__/slickGridMenu.spec.ts +++ b/packages/common/src/extensions/__tests__/slickGridMenu.spec.ts @@ -146,11 +146,8 @@ describe('GridMenuControl', () => { describe('with I18N Service', () => { const consoleErrorSpy = jest.spyOn(global.console, 'error').mockReturnValue(); - const consoleWarnSpy = jest.spyOn(global.console, 'warn').mockReturnValue(); - // let divElement; beforeEach(() => { - // divElement = document.createElement('div'); div = document.createElement('div'); div.innerHTML = template; document.body.appendChild(div); diff --git a/packages/common/src/extensions/__tests__/slickHeaderButtons.spec.ts b/packages/common/src/extensions/__tests__/slickHeaderButtons.spec.ts index 9e46ffc3a..6fc659294 100644 --- a/packages/common/src/extensions/__tests__/slickHeaderButtons.spec.ts +++ b/packages/common/src/extensions/__tests__/slickHeaderButtons.spec.ts @@ -54,7 +54,6 @@ const columnsMock: Column[] = [ ]; describe('HeaderButton Plugin', () => { - const consoleWarnSpy = jest.spyOn(global.console, 'warn').mockReturnValue(); let backendUtilityService: BackendUtilityService; let extensionUtility: ExtensionUtility; let translateService: TranslateServiceStub; diff --git a/packages/common/src/extensions/slickCellMenu.ts b/packages/common/src/extensions/slickCellMenu.ts index b976ec112..24daf30a7 100644 --- a/packages/common/src/extensions/slickCellMenu.ts +++ b/packages/common/src/extensions/slickCellMenu.ts @@ -137,7 +137,7 @@ export class SlickCellMenu extends MenuFromCellBaseClass { } // Hide the menu on outside click. - this._bindEventService.bind(document.body, 'mousedown', this.handleBodyMouseDown.bind(this) as EventListener); + this._bindEventService.bind(document.body, 'mousedown', this.handleBodyMouseDown.bind(this) as EventListener, { capture: true }); } } diff --git a/packages/common/src/extensions/slickColumnPicker.ts b/packages/common/src/extensions/slickColumnPicker.ts index 5507025f4..b81a75e1e 100644 --- a/packages/common/src/extensions/slickColumnPicker.ts +++ b/packages/common/src/extensions/slickColumnPicker.ts @@ -116,7 +116,7 @@ export class SlickColumnPicker { this._bindEventService.bind(this._menuElm, 'click', handleColumnPickerItemClick.bind(this) as EventListener, undefined, 'parent-menu'); // Hide the menu on outside click. - this._bindEventService.bind(document.body, 'mousedown', this.handleBodyMouseDown.bind(this) as EventListener); + this._bindEventService.bind(document.body, 'mousedown', this.handleBodyMouseDown.bind(this) as EventListener, { capture: true }); // destroy the picker if user leaves the page this._bindEventService.bind(document.body, 'beforeunload', this.dispose.bind(this) as EventListener); diff --git a/packages/common/src/extensions/slickContextMenu.ts b/packages/common/src/extensions/slickContextMenu.ts index 3ed4d5b8b..5e118c53b 100644 --- a/packages/common/src/extensions/slickContextMenu.ts +++ b/packages/common/src/extensions/slickContextMenu.ts @@ -142,7 +142,7 @@ export class SlickContextMenu extends MenuFromCellBaseClass { } // Hide the menu on outside click. - this._bindEventService.bind(document.body, 'mousedown', this.handleBodyMouseDown.bind(this) as EventListener); + this._bindEventService.bind(document.body, 'mousedown', this.handleBodyMouseDown.bind(this) as EventListener, { capture: true }); } } diff --git a/packages/common/src/extensions/slickGridMenu.ts b/packages/common/src/extensions/slickGridMenu.ts index 1c5695338..38876c373 100644 --- a/packages/common/src/extensions/slickGridMenu.ts +++ b/packages/common/src/extensions/slickGridMenu.ts @@ -221,7 +221,7 @@ export class SlickGridMenu extends MenuBaseClass { this.translateTitleLabels(this.sharedService.gridOptions.gridMenu); // hide the menu on outside click. - this._bindEventService.bind(document.body, 'mousedown', this.handleBodyMouseDown.bind(this) as EventListener); + this._bindEventService.bind(document.body, 'mousedown', this.handleBodyMouseDown.bind(this) as EventListener, { capture: true }); // destroy the picker if user leaves the page this._bindEventService.bind(document.body, 'beforeunload', this.dispose.bind(this) as EventListener); diff --git a/packages/common/src/extensions/slickHeaderMenu.ts b/packages/common/src/extensions/slickHeaderMenu.ts index 47e16e5fc..86e4cdae6 100644 --- a/packages/common/src/extensions/slickHeaderMenu.ts +++ b/packages/common/src/extensions/slickHeaderMenu.ts @@ -84,7 +84,7 @@ export class SlickHeaderMenu extends MenuBaseClass { this.grid.setColumns(this.grid.getColumns()); // hide the menu when clicking outside the grid - this._bindEventService.bind(document.body, 'mousedown', this.handleBodyMouseDown.bind(this) as EventListener); + this._bindEventService.bind(document.body, 'mousedown', this.handleBodyMouseDown.bind(this) as EventListener, { capture: true }); } /** Dispose (destroy) of the plugin */ From 13dca50f84c51eae4b084a4a16ce0e748faab3b7 Mon Sep 17 00:00:00 2001 From: ghiscoding Date: Mon, 30 Oct 2023 16:33:52 -0400 Subject: [PATCH 2/2] chore: fix Cypress flaky test --- test/cypress/e2e/example12.cy.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/cypress/e2e/example12.cy.ts b/test/cypress/e2e/example12.cy.ts index 24bf13f6c..0a2961ab4 100644 --- a/test/cypress/e2e/example12.cy.ts +++ b/test/cypress/e2e/example12.cy.ts @@ -111,12 +111,15 @@ describe('Example 12 - Composite Editor Modal', { retries: 1 }, () => { // change Completed cy.get(`[style="top:${GRID_ROW_HEIGHT * 0}px"] > .slick-cell:nth(7)`).click(); cy.get('.editor-completed').check(); + cy.get(`[style="top:${GRID_ROW_HEIGHT * 0}px"] > .slick-cell:nth(7)`).find('.mdi.mdi-check.checkmark-icon').should('have.length', 1); cy.get(`[style="top:${GRID_ROW_HEIGHT * 1}px"] > .slick-cell:nth(7)`).click(); cy.get('.editor-completed').check(); + cy.get(`[style="top:${GRID_ROW_HEIGHT * 1}px"] > .slick-cell:nth(7)`).find('.mdi.mdi-check.checkmark-icon').should('have.length', 1); cy.get(`[style="top:${GRID_ROW_HEIGHT * 2}px"] > .slick-cell:nth(7)`).click(); cy.get('.editor-completed').check(); + cy.get(`[style="top:${GRID_ROW_HEIGHT * 2}px"] > .slick-cell:nth(7)`).find('.mdi.mdi-check.checkmark-icon').should('have.length', 1); }); it('should be able to change "Finish" values of row indexes 0-2', () => {