From afc5830c31a6a180037ee887d6ba5d3341135237 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michaela=20Kr=C3=B6ber?= <85624192+mkrbr@users.noreply.github.com> Date: Thu, 21 Nov 2024 15:32:25 +0100 Subject: [PATCH] AAE-10239 showRowContextMenu-datatable-action-called-multiple-times (#10386) * [AAE-10239] use an action when the datatable component is loaded * [AAE-10239] add test for disabled context menu * [AAE-10239] fixed context menu for tree component * [AAE-10239] added new test for disabled context menu * [AAE-10239] fixed context menu for recent changes for tree component * [AAE-10239] backward compatible approach * [AAE-10239] no need to format the file * [AAE-10239] reverted rename of method * [AAE-10239] renamed file so you can find it better --- .../context-menu.directive.spec.ts | 231 ++++++++++++++++++ .../context-menu/context-menu.directive.ts | 15 +- .../src/lib/context-menu/context-menu.spec.ts | 200 --------------- .../datatable/datatable.component.spec.ts | 14 ++ .../datatable/datatable.component.ts | 10 +- 5 files changed, 260 insertions(+), 210 deletions(-) create mode 100644 lib/core/src/lib/context-menu/context-menu.directive.spec.ts delete mode 100644 lib/core/src/lib/context-menu/context-menu.spec.ts diff --git a/lib/core/src/lib/context-menu/context-menu.directive.spec.ts b/lib/core/src/lib/context-menu/context-menu.directive.spec.ts new file mode 100644 index 0000000000..e8237c43fa --- /dev/null +++ b/lib/core/src/lib/context-menu/context-menu.directive.spec.ts @@ -0,0 +1,231 @@ +/*! + * @license + * Copyright © 2005-2024 Hyland Software, Inc. and its affiliates. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Component } from '@angular/core'; +import { TestBed, ComponentFixture } from '@angular/core/testing'; +import { CONTEXT_MENU_DIRECTIVES } from './context-menu.module'; +import { CoreTestingModule } from '../testing/core.testing.module'; +import { HarnessLoader } from '@angular/cdk/testing'; +import { MatIconHarness } from '@angular/material/icon/testing'; +import { TestbedHarnessEnvironment } from '@angular/cdk/testing/testbed'; + +@Component({ + selector: 'adf-test-component', + template: `
` +}) +class TestComponent { + actions: any[] | (() => any[]); + isEnabled: boolean; +} + +const actions = [ + { + model: { + visible: false, + title: 'Action 1' + }, + subject: { + next: jasmine.createSpy('next') + } + }, + { + model: { + visible: true, + disabled: true, + title: 'Action 2', + icon: null + }, + subject: { + next: jasmine.createSpy('next') + } + }, + { + model: { + visible: true, + disabled: false, + title: 'Action 3', + icon: 'action-icon-3' + }, + subject: { + next: jasmine.createSpy('next') + } + }, + { + model: { + visible: true, + disabled: false, + title: 'Action 4', + icon: 'action-icon-4' + }, + subject: { + next: jasmine.createSpy('next') + } + }, + { + model: { + visible: true, + disabled: false, + title: 'action-5', + icon: 'action-icon-5', + tooltip: 'Action 5 tooltip' + }, + subject: { + next: jasmine.createSpy('next') + } + }, + { + model: { + visible: true, + disabled: false, + title: 'action-6', + icon: 'action-icon-6' + }, + subject: { + next: jasmine.createSpy('next') + } + } +]; + +const testCases = [ + { + description: 'with actions as an array', + actions + }, + { + description: 'with actions as a function', + actions: () => actions + } +]; + +testCases.forEach((testCase) => { + describe(`ContextMenuDirective ${testCase.description}`, () => { + let fixture: ComponentFixture; + + beforeEach(() => { + TestBed.configureTestingModule({ + imports: [CoreTestingModule, CONTEXT_MENU_DIRECTIVES], + declarations: [TestComponent] + }); + fixture = TestBed.createComponent(TestComponent); + fixture.componentInstance.isEnabled = false; + fixture.componentInstance.actions = testCase.actions; + fixture.detectChanges(); + }); + + it('should not show menu on mouse contextmenu event when context menu is disabled', () => { + const targetElement = fixture.debugElement.nativeElement.querySelector('#target'); + targetElement.dispatchEvent(new CustomEvent('contextmenu')); + fixture.detectChanges(); + + const contextMenu = document.querySelector('.adf-context-menu'); + expect(contextMenu).toBe(null); + }); + + describe('Events', () => { + let targetElement: HTMLElement; + let contextMenu: HTMLElement | null; + + beforeEach(() => { + fixture.componentInstance.isEnabled = true; + fixture.detectChanges(); + + targetElement = fixture.debugElement.nativeElement.querySelector('#target'); + targetElement.dispatchEvent(new CustomEvent('contextmenu')); + fixture.detectChanges(); + contextMenu = document.querySelector('.adf-context-menu'); + }); + + it('should show menu on mouse contextmenu event', () => { + expect(contextMenu).not.toBe(null); + }); + + it('should set DOM element reference on menu open event', () => { + expect(contextMenu?.className).toContain('adf-context-menu'); + }); + + it('should reset DOM element reference on Escape event', () => { + const event = new KeyboardEvent('keydown', { + bubbles: true, + cancelable: true, + key: 'Escape' + }); + + document.querySelector('.cdk-overlay-backdrop')?.dispatchEvent(event); + fixture.detectChanges(); + expect(document.querySelector('.adf-context-menu')).toBe(null); + }); + }); + + describe('Contextmenu list', () => { + let targetElement: HTMLElement; + let contextMenu: HTMLElement | null; + let loader: HarnessLoader; + + beforeEach(() => { + fixture.componentInstance.isEnabled = true; + fixture.detectChanges(); + + targetElement = fixture.debugElement.nativeElement.querySelector('#target'); + targetElement.dispatchEvent(new CustomEvent('contextmenu')); + fixture.detectChanges(); + contextMenu = document.querySelector('.adf-context-menu'); + loader = TestbedHarnessEnvironment.documentRootLoader(fixture); + }); + + it('should not render item with visibility property set to false', () => { + expect(contextMenu?.querySelectorAll('button').length).toBe(5); + }); + + it('should render item as disabled when `disabled` property is set to true', async () => { + expect(contextMenu?.querySelectorAll('button')[0].disabled).toBe(true); + }); + + it('should set first not disabled item as active', async () => { + const icon = await loader.getHarness(MatIconHarness.with({ ancestor: 'adf-context-menu' })); + + expect(await icon.getName()).toEqual('action-icon-3'); + }); + + it('should not allow action event when item is disabled', () => { + contextMenu?.querySelectorAll('button')[0].click(); + fixture.detectChanges(); + + expect(actions[1].subject.next).not.toHaveBeenCalled(); + }); + + it('should perform action when item is not disabled', () => { + contextMenu?.querySelectorAll('button')[1].click(); + fixture.detectChanges(); + + expect(actions[2].subject.next).toHaveBeenCalled(); + }); + + it('should not render item icon if not set', async () => { + expect( + ( + await loader.getAllHarnesses( + MatIconHarness.with({ + ancestor: 'adf-context-menu', + name: 'Action 1' + }) + ) + ).length + ).toBe(0); + }); + }); + }); +}); diff --git a/lib/core/src/lib/context-menu/context-menu.directive.ts b/lib/core/src/lib/context-menu/context-menu.directive.ts index d5a7e1cb9c..41ba924928 100644 --- a/lib/core/src/lib/context-menu/context-menu.directive.ts +++ b/lib/core/src/lib/context-menu/context-menu.directive.ts @@ -27,7 +27,7 @@ import { ContextMenuOverlayService } from './context-menu-overlay.service'; export class ContextMenuDirective { /** Items for the menu. */ @Input('adf-context-menu') - links: any[]; + links: any[] | (() => any[]); /** Is the menu enabled? */ @Input('adf-context-menu-enabled') @@ -42,11 +42,14 @@ export class ContextMenuDirective { event.preventDefault(); } - if (this.links && this.links.length > 0) { - this.contextMenuService.open({ - source: event, - data: this.links - }); + if (this.links) { + const actions = typeof this.links === 'function' ? this.links() : this.links; + if (actions.length > 0) { + this.contextMenuService.open({ + source: event, + data: actions + }); + } } } } diff --git a/lib/core/src/lib/context-menu/context-menu.spec.ts b/lib/core/src/lib/context-menu/context-menu.spec.ts deleted file mode 100644 index 1bee7feaa8..0000000000 --- a/lib/core/src/lib/context-menu/context-menu.spec.ts +++ /dev/null @@ -1,200 +0,0 @@ -/*! - * @license - * Copyright © 2005-2024 Hyland Software, Inc. and its affiliates. All rights reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import { Component } from '@angular/core'; -import { TestBed, ComponentFixture } from '@angular/core/testing'; -import { CONTEXT_MENU_DIRECTIVES } from './context-menu.module'; -import { CoreTestingModule } from '../testing/core.testing.module'; -import { HarnessLoader } from '@angular/cdk/testing'; -import { MatIconHarness } from '@angular/material/icon/testing'; -import { TestbedHarnessEnvironment } from '@angular/cdk/testing/testbed'; - -@Component({ - selector: 'adf-test-component', - template: `
` -}) -class TestComponent { - actions; -} - -describe('ContextMenuDirective', () => { - let fixture: ComponentFixture; - const actions = [ - { - model: { - visible: false, - title: 'Action 1' - }, - subject: { - next: jasmine.createSpy('next') - } - }, - { - model: { - visible: true, - disabled: true, - title: 'Action 2', - icon: null - }, - subject: { - next: jasmine.createSpy('next') - } - }, - { - model: { - visible: true, - disabled: false, - title: 'Action 3', - icon: 'action-icon-3' - }, - subject: { - next: jasmine.createSpy('next') - } - }, - { - model: { - visible: true, - disabled: false, - title: 'Action 4', - icon: 'action-icon-4' - }, - subject: { - next: jasmine.createSpy('next') - } - }, - { - model: { - visible: true, - disabled: false, - title: 'action-5', - icon: 'action-icon-5', - tooltip: 'Action 5 tooltip' - }, - subject: { - next: jasmine.createSpy('next') - } - }, - { - model: { - visible: true, - disabled: false, - title: 'action-6', - icon: 'action-icon-6' - }, - subject: { - next: jasmine.createSpy('next') - } - } - ]; - - beforeEach(() => { - TestBed.configureTestingModule({ - imports: [CoreTestingModule, CONTEXT_MENU_DIRECTIVES], - declarations: [TestComponent] - }); - fixture = TestBed.createComponent(TestComponent); - fixture.componentInstance.actions = actions; - fixture.detectChanges(); - }); - - describe('Events', () => { - let targetElement: HTMLElement; - let contextMenu: HTMLElement | null; - - beforeEach(() => { - targetElement = fixture.debugElement.nativeElement.querySelector('#target'); - targetElement.dispatchEvent(new CustomEvent('contextmenu')); - fixture.detectChanges(); - contextMenu = document.querySelector('.adf-context-menu'); - }); - - it('should show menu on mouse contextmenu event', () => { - expect(contextMenu).not.toBe(null); - }); - - it('should set DOM element reference on menu open event', () => { - expect(contextMenu?.className).toContain('adf-context-menu'); - }); - - it('should reset DOM element reference on Escape event', () => { - const event = new KeyboardEvent('keydown', { - bubbles: true, - cancelable: true, - key: 'Escape' - }); - - document.querySelector('.cdk-overlay-backdrop')?.dispatchEvent(event); - fixture.detectChanges(); - expect(document.querySelector('.adf-context-menu')).toBe(null); - }); - }); - - describe('Contextmenu list', () => { - let targetElement: HTMLElement; - let contextMenu: HTMLElement | null; - let loader: HarnessLoader; - - beforeEach(() => { - targetElement = fixture.debugElement.nativeElement.querySelector('#target'); - targetElement.dispatchEvent(new CustomEvent('contextmenu')); - fixture.detectChanges(); - contextMenu = document.querySelector('.adf-context-menu'); - loader = TestbedHarnessEnvironment.documentRootLoader(fixture); - }); - - it('should not render item with visibility property set to false', () => { - expect(contextMenu?.querySelectorAll('button').length).toBe(5); - }); - - it('should render item as disabled when `disabled` property is set to true', async () => { - expect(contextMenu?.querySelectorAll('button')[0].disabled).toBe(true); - }); - - it('should set first not disabled item as active', async () => { - const icon = await loader.getHarness(MatIconHarness.with({ ancestor: 'adf-context-menu' })); - - expect(await icon.getName()).toEqual('action-icon-3'); - }); - - it('should not allow action event when item is disabled', () => { - contextMenu?.querySelectorAll('button')[0].click(); - fixture.detectChanges(); - - expect(fixture.componentInstance.actions[1].subject.next).not.toHaveBeenCalled(); - }); - - it('should perform action when item is not disabled', () => { - contextMenu?.querySelectorAll('button')[1].click(); - fixture.detectChanges(); - - expect(fixture.componentInstance.actions[2].subject.next).toHaveBeenCalled(); - }); - - it('should not render item icon if not set', async () => { - expect( - ( - await loader.getAllHarnesses( - MatIconHarness.with({ - ancestor: 'adf-context-menu', - name: 'Action 1' - }) - ) - ).length - ).toBe(0); - }); - }); -}); diff --git a/lib/core/src/lib/datatable/components/datatable/datatable.component.spec.ts b/lib/core/src/lib/datatable/components/datatable/datatable.component.spec.ts index 3c78929b60..1f60f6c6dc 100644 --- a/lib/core/src/lib/datatable/components/datatable/datatable.component.spec.ts +++ b/lib/core/src/lib/datatable/components/datatable/datatable.component.spec.ts @@ -156,6 +156,20 @@ describe('DataTable', () => { fixture.destroy(); }); + it('should not emit showRowContextMenu when the component is loaded with rows', () => { + spyOn(dataTable.showRowContextMenu, 'emit'); + const newData = new ObjectDataTableAdapter([{ name: 'TEST' }, { name: 'FAKE' }], [new ObjectDataColumn({ key: 'name' })]); + dataTable.data = new ObjectDataTableAdapter([{ name: '1' }, { name: '2' }], [new ObjectDataColumn({ key: 'name' })]); + + dataTable.ngOnChanges({ + data: new SimpleChange(null, newData, false) + }); + + fixture.detectChanges(); + + expect(dataTable.showRowContextMenu.emit).not.toHaveBeenCalled(); + }); + it('should return only visible columns', () => { const columns = [ { key: 'col1', isHidden: false }, diff --git a/lib/core/src/lib/datatable/components/datatable/datatable.component.ts b/lib/core/src/lib/datatable/components/datatable/datatable.component.ts index 7967780f8c..fd99a890ad 100644 --- a/lib/core/src/lib/datatable/components/datatable/datatable.component.ts +++ b/lib/core/src/lib/datatable/components/datatable/datatable.component.ts @@ -803,10 +803,12 @@ export class DataTableComponent implements OnInit, AfterContentInit, OnChanges, return false; } - getContextMenuActions(row: DataRow, col: DataColumn): any[] { - const event = new DataCellEvent(row, col, []); - this.showRowContextMenu.emit(event); - return event.value.actions; + getContextMenuActions(row: DataRow, col: DataColumn): () => any[] { + return () => { + const event = new DataCellEvent(row, col, []); + this.showRowContextMenu.emit(event); + return event.value.actions; + }; } getRowActions(row: DataRow, col?: DataColumn): any[] {