From c4424bf3f156b4cf366acdf60998825e23193bc3 Mon Sep 17 00:00:00 2001 From: Eudes Petonnet-Vincent Date: Tue, 27 Jun 2017 11:46:04 -0400 Subject: [PATCH] [NG] Datagrid: Fix performance issue for action overflow We now properly add the click listener on the whole document only when the action overflow menu is open. Fixes #1111. Signed-off-by: Eudes Petonnet-Vincent --- src/app/datagrid/selection/selection.ts | 2 +- .../data/datagrid/datagrid-action-overflow.ts | 48 ++++++------- .../data/datagrid/datagrid.module.ts | 4 +- .../utils/outside-click/index.ts | 13 ++++ .../outside-click/outside-click.module.ts | 15 ++++ .../utils/outside-click/outside-click.spec.ts | 72 +++++++++++++++++++ .../utils/outside-click/outside-click.ts | 26 +++++++ 7 files changed, 151 insertions(+), 29 deletions(-) create mode 100644 src/clarity-angular/utils/outside-click/index.ts create mode 100644 src/clarity-angular/utils/outside-click/outside-click.module.ts create mode 100644 src/clarity-angular/utils/outside-click/outside-click.spec.ts create mode 100644 src/clarity-angular/utils/outside-click/outside-click.ts diff --git a/src/app/datagrid/selection/selection.ts b/src/app/datagrid/selection/selection.ts index 71270a0a5d..cbb5b41332 100644 --- a/src/app/datagrid/selection/selection.ts +++ b/src/app/datagrid/selection/selection.ts @@ -78,4 +78,4 @@ export class DatagridSelectionDemo { this.cleanUp(); this.toAdd = this.selected.slice(); } -} \ No newline at end of file +} diff --git a/src/clarity-angular/data/datagrid/datagrid-action-overflow.ts b/src/clarity-angular/data/datagrid/datagrid-action-overflow.ts index ece50d649e..b0c30e7e0a 100644 --- a/src/clarity-angular/data/datagrid/datagrid-action-overflow.ts +++ b/src/clarity-angular/data/datagrid/datagrid-action-overflow.ts @@ -4,17 +4,17 @@ * The full license information can be found in LICENSE in the root directory of this project. */ import { - Component, EventEmitter, HostListener, Input, Output, ElementRef + Component, EventEmitter, Input, Output, ElementRef } from "@angular/core"; import {Point} from "../../popover/common/popover"; @Component({ selector: "clr-dg-action-overflow", template: ` - + -
+
@@ -48,36 +48,30 @@ export class DatagridActionOverflow { @Output("clrDgActionOverflowOpenChange") public openChanged = new EventEmitter(false); + /* + * We need to remember the click that opens the menu, to make sure it doesn't close the menu instantly + * when the event bubbles up the DOM all the way to the document, which we also listen to. + */ + private openingEvent: any; + /** * Shows/hides the action overflow menu */ - public toggle() { + public toggle(event: any) { + this.openingEvent = event; this.open = !this.open; } - //called on mouse clicks anywhere in the DOM. - //Checks to see if the mouseclick happened on the host or outside - @HostListener("document:click", [ "$event.target" ]) - onMouseClick(target: any): void { - if (this._open) { - let current: any = target; //Get the element in the DOM on which the mouse was clicked - let actionMenuHost: any = this.elementRef.nativeElement; //Get the current actionMenu native HTML element - - if (target.className === "datagrid-action-overflow") { - return; // if clicking on the action overflow container but not the content, return without closing - } - - //Start checking if current and actionMenuHost are equal. If not traverse to the parentNode and check again. - while (current) { - if (current.className === "datagrid-action-overflow") { - break; // if user clicked on the overflow menu, hide it - } - if (current === actionMenuHost) { - return; - } - current = current.parentNode; - } - this._open = false; // Hide the overflow menu + public close(event: MouseEvent) { + /* + * Because this listener is added synchonously, before the event finishes bubbling up the DOM, + * we end up firing on the very click that just opened the menu, p + * otentially closing it immediately every time. So we just ignore it. + */ + if (event === this.openingEvent) { + delete this.openingEvent; + return; } + this.open = false; } } diff --git a/src/clarity-angular/data/datagrid/datagrid.module.ts b/src/clarity-angular/data/datagrid/datagrid.module.ts index e3aa641f41..a63a2e826b 100644 --- a/src/clarity-angular/data/datagrid/datagrid.module.ts +++ b/src/clarity-angular/data/datagrid/datagrid.module.ts @@ -12,6 +12,7 @@ import { FormsModule } from "@angular/forms"; import { ClrCommonPopoverModule } from "../../popover/common/popover.module"; import { ClrLoadingModule } from "../../utils/loading/loading.module"; import { DATAGRID_DIRECTIVES } from "./index"; +import { ClrOutsideClickModule } from "../../utils/outside-click/outside-click.module"; @NgModule({ imports: [ @@ -20,7 +21,8 @@ import { DATAGRID_DIRECTIVES } from "./index"; ClrFormsModule, FormsModule, ClrCommonPopoverModule, - ClrLoadingModule + ClrLoadingModule, + ClrOutsideClickModule ], declarations: [ DATAGRID_DIRECTIVES, diff --git a/src/clarity-angular/utils/outside-click/index.ts b/src/clarity-angular/utils/outside-click/index.ts new file mode 100644 index 0000000000..5b06efa93e --- /dev/null +++ b/src/clarity-angular/utils/outside-click/index.ts @@ -0,0 +1,13 @@ +/* + * Copyright (c) 2016 VMware, Inc. All Rights Reserved. + * This software is released under MIT license. + * The full license information can be found in LICENSE in the root directory of this project. + */ +import { Type } from "@angular/core"; +import {OutsideClick} from "./outside-click"; + +export * from "./outside-click"; + +export const OUSTIDE_CLICK_DIRECTIVES: Type[] = [ + OutsideClick +]; diff --git a/src/clarity-angular/utils/outside-click/outside-click.module.ts b/src/clarity-angular/utils/outside-click/outside-click.module.ts new file mode 100644 index 0000000000..f161c19296 --- /dev/null +++ b/src/clarity-angular/utils/outside-click/outside-click.module.ts @@ -0,0 +1,15 @@ +/** + * Copyright (c) 2016-2017 VMware, Inc. All Rights Reserved. + * This software is released under MIT license. + * The full license information can be found in LICENSE in the root directory of this project. + */ +import { NgModule } from "@angular/core"; +import { CommonModule } from "@angular/common"; +import {OUSTIDE_CLICK_DIRECTIVES} from "./index"; + +@NgModule({ + imports: [CommonModule], + declarations: [OUSTIDE_CLICK_DIRECTIVES], + exports: [OUSTIDE_CLICK_DIRECTIVES] +}) +export class ClrOutsideClickModule {} diff --git a/src/clarity-angular/utils/outside-click/outside-click.spec.ts b/src/clarity-angular/utils/outside-click/outside-click.spec.ts new file mode 100644 index 0000000000..86c1a60bed --- /dev/null +++ b/src/clarity-angular/utils/outside-click/outside-click.spec.ts @@ -0,0 +1,72 @@ +/* + * Copyright (c) 2016 VMware, Inc. All Rights Reserved. + * This software is released under MIT license. + * The full license information can be found in LICENSE in the root directory of this project. + */ +import {Component} from "@angular/core"; +import {TestBed} from "@angular/core/testing"; +import {By} from "@angular/platform-browser"; +import {OutsideClick} from "./outside-click"; + +describe("Loading directive", function() { + beforeEach(function () { + TestBed.configureTestingModule({ + declarations: [OutsideClick, FullTest] + }); + this.fixture = TestBed.createComponent(FullTest); + this.fixture.detectChanges(); + this.testComponent = this.fixture.componentInstance; + this.host = this.fixture.debugElement.query(By.css(".host")).nativeElement; + this.button = this.fixture.debugElement.query(By.css("button")).nativeElement; + this.outside = this.fixture.debugElement.query(By.css(".outside")).nativeElement; + }); + + afterEach(function() { + this.fixture.destroy(); + }); + + it("emits clicks outside of the host", function () { + expect(this.testComponent.nbClicks).toBe(0); + this.outside.click(); + expect(this.testComponent.nbClicks).toBe(1); + this.outside.click(); + expect(this.testComponent.nbClicks).toBe(2); + }); + + it("ignores clicks inside of the host", function () { + expect(this.testComponent.nbClicks).toBe(0); + this.host.click(); + expect(this.testComponent.nbClicks).toBe(0); + this.button.click(); + expect(this.testComponent.nbClicks).toBe(0); + }); + + it("offers a strict input to only ignore clicks that happen exactly on the host", function () { + this.testComponent.strict = true; + this.fixture.detectChanges(); + expect(this.testComponent.nbClicks).toBe(0); + this.host.click(); + expect(this.testComponent.nbClicks).toBe(0); + this.button.click(); + expect(this.testComponent.nbClicks).toBe(1); + }); +}); + + +@Component({ + template: ` +

Hello World

+

+ +

+ + ` +}) +class FullTest { + public strict = false; + public nbClicks = 0; + + inc() { + this.nbClicks++; + } +} diff --git a/src/clarity-angular/utils/outside-click/outside-click.ts b/src/clarity-angular/utils/outside-click/outside-click.ts new file mode 100644 index 0000000000..3881879062 --- /dev/null +++ b/src/clarity-angular/utils/outside-click/outside-click.ts @@ -0,0 +1,26 @@ +import {Directive, Output, EventEmitter, HostListener, ElementRef, Input} from "@angular/core"; + +@Directive({ + selector: "[clrOutsideClick]" +}) +export class OutsideClick { + constructor(private el: ElementRef) {} + + @Input("clrStrict") strict = false; + + @Output("clrOutsideClick") outsideClick = new EventEmitter(false); + + @HostListener("document:click", ["$event"]) + documentClick(event: MouseEvent) { + let target = event.target; //Get the element in the DOM on which the mouse was clicked + let host = this.el.nativeElement; //Get the current actionMenu native HTML element + + if (target === host) { + return; + } + if (!this.strict && host.contains(target)) { + return; + } + this.outsideClick.emit(event); + } +}