Skip to content

Commit

Permalink
fix(common): context menu should close when clicking another cell (#1163
Browse files Browse the repository at this point in the history
)

* 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

* chore: fix Cypress flaky test
  • Loading branch information
ghiscoding authored Oct 30, 2023
1 parent 609f88b commit bd132c5
Show file tree
Hide file tree
Showing 9 changed files with 8 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion packages/common/src/extensions/slickCellMenu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ export class SlickCellMenu extends MenuFromCellBaseClass<CellMenu> {
}

// 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 });
}
}

Expand Down
2 changes: 1 addition & 1 deletion packages/common/src/extensions/slickColumnPicker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion packages/common/src/extensions/slickContextMenu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ export class SlickContextMenu extends MenuFromCellBaseClass<ContextMenu> {
}

// 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 });
}
}

Expand Down
2 changes: 1 addition & 1 deletion packages/common/src/extensions/slickGridMenu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ export class SlickGridMenu extends MenuBaseClass<GridMenu> {
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);
Expand Down
2 changes: 1 addition & 1 deletion packages/common/src/extensions/slickHeaderMenu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ export class SlickHeaderMenu extends MenuBaseClass<HeaderMenu> {
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 */
Expand Down
3 changes: 3 additions & 0 deletions test/cypress/e2e/example12.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down

0 comments on commit bd132c5

Please sign in to comment.