Skip to content

Commit

Permalink
fix: Manually added all document listeners to avoid performance issues (
Browse files Browse the repository at this point in the history
  • Loading branch information
edcarroll authored and mcosta74 committed Aug 20, 2017
1 parent 8085f4f commit 5739d86
Show file tree
Hide file tree
Showing 13 changed files with 68 additions and 46 deletions.
17 changes: 12 additions & 5 deletions src/modules/datepicker/views/calendar-view.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Input, QueryList, ViewChildren, AfterViewInit, HostListener } from "@angular/core";
import { Input, QueryList, ViewChildren, AfterViewInit, HostListener, Renderer2, OnDestroy } from "@angular/core";
import { KeyCode } from "../../../misc/util/index";
import { CalendarItem, SuiCalendarItem } from "../directives/calendar-item";
import { CalendarService } from "../services/calendar.service";
Expand All @@ -13,7 +13,7 @@ export enum CalendarViewType {
}
export type CalendarViewResult = [Date, CalendarViewType];

export abstract class CalendarView implements AfterViewInit {
export abstract class CalendarView implements AfterViewInit, OnDestroy {
private _type:CalendarViewType;
private _service:CalendarService;

Expand Down Expand Up @@ -48,9 +48,13 @@ export abstract class CalendarView implements AfterViewInit {
return this.service.selectedDate;
}

constructor(viewType:CalendarViewType, ranges:CalendarRangeService) {
private _documentKeyDownListener:() => void;

constructor(renderer:Renderer2, viewType:CalendarViewType, ranges:CalendarRangeService) {
this._type = viewType;
this.ranges = ranges;

this._documentKeyDownListener = renderer.listen("document", "keydown", (e:KeyboardEvent) => this.onDocumentKeyDown(e));
}

// Template Methods
Expand Down Expand Up @@ -107,8 +111,7 @@ export abstract class CalendarView implements AfterViewInit {
}
}

@HostListener("document:keydown", ["$event"])
private onDocumentKeydown(e:KeyboardEvent):void {
private onDocumentKeyDown(e:KeyboardEvent):void {
if (this._highlightedItem && e.keyCode === KeyCode.Enter) {
this.setDate(this._highlightedItem);
return;
Expand Down Expand Up @@ -181,4 +184,8 @@ export abstract class CalendarView implements AfterViewInit {
this.ranges.move(isMovingForward);
this._highlightedItem = this.ranges.current.find(nextItem);
}

public ngOnDestroy():void {
this._documentKeyDownListener();
}
}
6 changes: 3 additions & 3 deletions src/modules/datepicker/views/date-view.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Component } from "@angular/core";
import { Component, Renderer2 } from "@angular/core";
import { DateUtil, DatePrecision } from "../../../misc/util/index";
import { CalendarItem } from "../directives/calendar-item";
import { CalendarView, CalendarViewType } from "./calendar-view";
Expand Down Expand Up @@ -57,7 +57,7 @@ export class SuiCalendarDateView extends CalendarView {
return new DateParser(this.service.localeValues.formats.month, this.service.localeValues).format(this.currentDate);
}

constructor() {
super(CalendarViewType.Date, new CalendarRangeDateService(DatePrecision.Month, 6, 7));
constructor(renderer:Renderer2) {
super(renderer, CalendarViewType.Date, new CalendarRangeDateService(DatePrecision.Month, 6, 7));
}
}
6 changes: 3 additions & 3 deletions src/modules/datepicker/views/hour-view.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Component } from "@angular/core";
import { Component, Renderer2 } from "@angular/core";
import { DatePrecision } from "../../../misc/util/index";
import { CalendarView, CalendarViewType } from "./calendar-view";
import { CalendarItem } from "../directives/calendar-item";
Expand Down Expand Up @@ -45,7 +45,7 @@ export class SuiCalendarHourView extends CalendarView {
return new DateParser(this.service.localeValues.formats.date, this.service.localeValues).format(this.currentDate);
}

constructor() {
super(CalendarViewType.Hour, new CalendarRangeHourService(DatePrecision.Date, 6, 4));
constructor(renderer:Renderer2) {
super(renderer, CalendarViewType.Hour, new CalendarRangeHourService(DatePrecision.Date, 6, 4));
}
}
6 changes: 3 additions & 3 deletions src/modules/datepicker/views/minute-view.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Component } from "@angular/core";
import { Component, Renderer2 } from "@angular/core";
import { Util, DateUtil, DatePrecision } from "../../../misc/util/index";
import { CalendarView, CalendarViewType } from "./calendar-view";
import { CalendarItem } from "../directives/calendar-item";
Expand Down Expand Up @@ -61,7 +61,7 @@ export class SuiCalendarMinuteView extends CalendarView {
}
}

constructor() {
super(CalendarViewType.Minute, new CalendarRangeMinuteService(DatePrecision.Hour, 4, 3));
constructor(renderer:Renderer2) {
super(renderer, CalendarViewType.Minute, new CalendarRangeMinuteService(DatePrecision.Hour, 4, 3));
}
}
6 changes: 3 additions & 3 deletions src/modules/datepicker/views/month-view.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Component } from "@angular/core";
import { Component, Renderer2 } from "@angular/core";
import { DatePrecision } from "../../../misc/util/index";
import { CalendarView, CalendarViewType } from "./calendar-view";
import { CalendarItem } from "../directives/calendar-item";
Expand Down Expand Up @@ -42,7 +42,7 @@ export class SuiCalendarMonthView extends CalendarView {
return new DateParser(this.service.localeValues.formats.year, this.service.localeValues).format(this.currentDate);
}

constructor() {
super(CalendarViewType.Month, new CalendarRangeMonthService(DatePrecision.Year, 4, 3));
constructor(renderer:Renderer2) {
super(renderer, CalendarViewType.Month, new CalendarRangeMonthService(DatePrecision.Year, 4, 3));
}
}
6 changes: 3 additions & 3 deletions src/modules/datepicker/views/year-view.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Component } from "@angular/core";
import { Component, Renderer2 } from "@angular/core";
import { Util, DateUtil, DatePrecision } from "../../../misc/util/index";
import { CalendarView, CalendarViewType } from "./calendar-view";
import { CalendarItem } from "../directives/calendar-item";
Expand Down Expand Up @@ -43,8 +43,8 @@ export class SuiCalendarYearView extends CalendarView {
.getFullYear();
}

constructor() {
super(CalendarViewType.Year, new CalendarRangeYearService(DatePrecision.Decade, 4, 3));
constructor(renderer:Renderer2) {
super(renderer, CalendarViewType.Year, new CalendarRangeYearService(DatePrecision.Decade, 4, 3));
}

public pad(year:number):string {
Expand Down
15 changes: 11 additions & 4 deletions src/modules/dropdown/directives/dropdown-menu.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {
Directive, ContentChild, forwardRef, Renderer2, ElementRef, AfterContentInit,
ContentChildren, QueryList, Input, HostListener, ChangeDetectorRef
ContentChildren, QueryList, Input, HostListener, ChangeDetectorRef, OnDestroy
} from "@angular/core";
import { Transition, SuiTransition, TransitionController, TransitionDirection } from "../../transition/index";
import { HandledEvent, IAugmentedElement, KeyCode } from "../../../misc/util/index";
Expand Down Expand Up @@ -59,7 +59,7 @@ export class SuiDropdownMenuItem {
@Directive({
selector: "[suiDropdownMenu]"
})
export class SuiDropdownMenu extends SuiTransition implements AfterContentInit {
export class SuiDropdownMenu extends SuiTransition implements AfterContentInit, OnDestroy {
private _service:DropdownService;
private _transitionController:TransitionController;

Expand Down Expand Up @@ -129,6 +129,8 @@ export class SuiDropdownMenu extends SuiTransition implements AfterContentInit {
@Input()
public menuSelectedItemClass:string;

private _documentKeyDownListener:() => void;

constructor(renderer:Renderer2, public element:ElementRef, changeDetector:ChangeDetectorRef) {
super(renderer, element, changeDetector);

Expand All @@ -141,6 +143,8 @@ export class SuiDropdownMenu extends SuiTransition implements AfterContentInit {

this.menuAutoSelectFirst = false;
this.menuSelectedItemClass = "selected";

this._documentKeyDownListener = renderer.listen("document", "keydown", (e:KeyboardEvent) => this.onDocumentKeyDown(e));
}

@HostListener("click", ["$event"])
Expand All @@ -158,8 +162,7 @@ export class SuiDropdownMenu extends SuiTransition implements AfterContentInit {
}
}

@HostListener("document:keydown", ["$event"])
public onDocumentKeydown(e:KeyboardEvent):void {
public onDocumentKeyDown(e:KeyboardEvent):void {
// Only the root dropdown (i.e. not nested dropdowns) is responsible for keeping track of the currently selected item.
if (this._service.isOpen && !this._service.isNested) {
// Stop document events like scrolling while open.
Expand Down Expand Up @@ -312,4 +315,8 @@ export class SuiDropdownMenu extends SuiTransition implements AfterContentInit {
// We use `_items` rather than `items` in case one or more have become disabled.
this.resetSelection();
}

public ngOnDestroy():void {
this._documentKeyDownListener();
}
}
3 changes: 2 additions & 1 deletion src/modules/modal/components/modal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -288,8 +288,9 @@ export class SuiModal<T, U> implements OnInit, AfterViewInit {
e.stopPropagation();
}

// Document listener is fine here because nobody will enough modals open.
@HostListener("document:keyup", ["$event"])
public onDocumentKeyup(e:KeyboardEvent):void {
public onDocumentKeyUp(e:KeyboardEvent):void {
if (e.keyCode === KeyCode.Escape) {
// Close automatically covers case of `!isClosable`, so check not needed.
this.close();
Expand Down
13 changes: 5 additions & 8 deletions src/modules/popup/classes/popup-controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export abstract class SuiPopupController implements IPopup, OnDestroy {
// Function to remove the document click handler.
private _documentListener:() => void;

constructor(private _renderer:Renderer2,
constructor(renderer:Renderer2,
protected _element:ElementRef,
protected _componentFactory:SuiComponentFactory,
config:PopupConfig) {
Expand All @@ -39,6 +39,8 @@ export abstract class SuiPopupController implements IPopup, OnDestroy {

// When the popup is closed (onClose fires on animation complete),
this.popup.onClose.subscribe(() => this.cleanup());

this._documentListener = renderer.listen("document", "click", (e:MouseEvent) => this.onDocumentClick(e));
}

public configure(config?:IPopupConfig):void {
Expand All @@ -65,8 +67,6 @@ export abstract class SuiPopupController implements IPopup, OnDestroy {
// Attach a reference to the anchor element. We do it here because IE11 loves to complain.
this.popup.anchor = this._element;

this._documentListener = this._renderer.listen("document", "click", (e:MouseEvent) => this.onDocumentClick(e));

// Start popup open transition.
this.popup.open();

Expand Down Expand Up @@ -177,14 +177,11 @@ export abstract class SuiPopupController implements IPopup, OnDestroy {
}

this._componentFactory.detachFromApplication(this._componentRef);

if (this._documentListener) {
// Remove the document click handler
this._documentListener();
}
}

public ngOnDestroy():void {
this.cleanup();

this._documentListener();
}
}
15 changes: 11 additions & 4 deletions src/modules/search/components/search.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {
Component, ViewChild, HostBinding, Input, AfterViewInit, HostListener,
EventEmitter, Output, Directive, ElementRef, TemplateRef
EventEmitter, Output, Directive, ElementRef, TemplateRef, Renderer2, OnDestroy
} from "@angular/core";
import { Util, ITemplateRefContext, IFocusEvent } from "../../../misc/util/index";
import { DropdownService, SuiDropdownMenu } from "../../dropdown/index";
Expand Down Expand Up @@ -51,7 +51,7 @@ export interface IResultContext<T> extends ITemplateRefContext<T> {
}
`]
})
export class SuiSearch<T> implements AfterViewInit {
export class SuiSearch<T> implements AfterViewInit, OnDestroy {
public dropdownService:DropdownService;
public searchService:SearchService<T, T>;

Expand Down Expand Up @@ -182,7 +182,9 @@ export class SuiSearch<T> implements AfterViewInit {
@Input()
public transitionDuration:number;

constructor(private _element:ElementRef, private _localizationService:SuiLocalizationService) {
private _documentClickListener:() => void;

constructor(private _element:ElementRef, renderer:Renderer2, private _localizationService:SuiLocalizationService) {
this.dropdownService = new DropdownService();
this.searchService = new SearchService<T, T>();

Expand All @@ -199,6 +201,8 @@ export class SuiSearch<T> implements AfterViewInit {

this.transition = "scale";
this.transitionDuration = 200;

this._documentClickListener = renderer.listen("document", "click", (e:MouseEvent) => this.onDocumentClick(e));
}

public ngAfterViewInit():void {
Expand Down Expand Up @@ -247,7 +251,6 @@ export class SuiSearch<T> implements AfterViewInit {
}
}

@HostListener("document:click", ["$event"])
public onDocumentClick(e:MouseEvent):void {
if (!this._element.nativeElement.contains(e.target)) {
this.dropdownService.setOpenState(false);
Expand All @@ -258,4 +261,8 @@ export class SuiSearch<T> implements AfterViewInit {
public readValue(object:T):string {
return Util.Object.readValue<T, string>(object, this.searchService.optionsField);
}

public ngOnDestroy():void {
this._documentClickListener();
}
}
9 changes: 6 additions & 3 deletions src/modules/select/classes/select-base.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {
ViewChild, HostBinding, ElementRef, HostListener, Input, ContentChildren, QueryList,
AfterContentInit, TemplateRef, ViewContainerRef, ContentChild, EventEmitter, Output, OnDestroy
AfterContentInit, TemplateRef, ViewContainerRef, ContentChild, EventEmitter, Output, OnDestroy, Renderer2
} from "@angular/core";
import { Subscription } from "rxjs/Subscription";
import { DropdownService, SuiDropdownMenu } from "../../dropdown/index";
Expand Down Expand Up @@ -225,7 +225,9 @@ export abstract class SuiSelectBase<T, U> implements AfterContentInit, OnDestroy
@Output("touched")
public onTouched:EventEmitter<void>;

constructor(private _element:ElementRef, protected _localizationService:SuiLocalizationService) {
private _documentKeyDownListener:() => void;

constructor(private _element:ElementRef, renderer:Renderer2, protected _localizationService:SuiLocalizationService) {
this.dropdownService = new DropdownService();
// We do want an empty query to return all results.
this.searchService = new SearchService<T, U>(true);
Expand All @@ -241,6 +243,7 @@ export abstract class SuiSelectBase<T, U> implements AfterContentInit, OnDestroy
this.transitionDuration = 200;

this.onTouched = new EventEmitter<void>();
this._documentKeyDownListener = renderer.listen("document", "keydown", (e:KeyboardEvent) => this.onDocumentKeyDown(e));

this._selectClasses = true;
}
Expand Down Expand Up @@ -383,7 +386,6 @@ export abstract class SuiSelectBase<T, U> implements AfterContentInit, OnDestroy
}
}

@HostListener("document:keydown", ["$event"])
public onDocumentKeyDown(e:KeyboardEvent):void {
if (this._element.nativeElement.contains(e.target) &&
!this.dropdownService.isOpen &&
Expand Down Expand Up @@ -421,5 +423,6 @@ export abstract class SuiSelectBase<T, U> implements AfterContentInit, OnDestroy

public ngOnDestroy():void {
this._renderedSubscriptions.forEach(s => s.unsubscribe());
this._documentKeyDownListener();
}
}
6 changes: 3 additions & 3 deletions src/modules/select/components/multi-select.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Component, HostBinding, ElementRef, EventEmitter, Output, Input, Directive } from "@angular/core";
import { Component, HostBinding, ElementRef, EventEmitter, Output, Input, Directive, Renderer2 } from "@angular/core";
import { ICustomValueAccessorHost, KeyCode, customValueAccessorFactory, CustomValueAccessor } from "../../../misc/util/index";
import { SuiLocalizationService } from "../../../behaviors/localization/index";
import { SuiSelectBase } from "../classes/select-base";
Expand Down Expand Up @@ -134,8 +134,8 @@ export class SuiMultiSelect<T, U> extends SuiSelectBase<T, U> implements ICustom
@HostBinding("class.multiple")
private _multiSelectClasses:boolean;

constructor(element:ElementRef, localizationService:SuiLocalizationService) {
super(element, localizationService);
constructor(element:ElementRef, renderer:Renderer2, localizationService:SuiLocalizationService) {
super(element, renderer, localizationService);

this.selectedOptions = [];
this.selectedOptionsChange = new EventEmitter<U[]>();
Expand Down
6 changes: 3 additions & 3 deletions src/modules/select/components/select.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Component, ViewContainerRef, ViewChild, Output, EventEmitter, ElementRef, Directive, Input } from "@angular/core";
import { Component, ViewContainerRef, ViewChild, Output, EventEmitter, ElementRef, Directive, Input, Renderer2 } from "@angular/core";
import { ICustomValueAccessorHost, customValueAccessorFactory, CustomValueAccessor } from "../../../misc/util/index";
import { SuiLocalizationService } from "../../../behaviors/localization/index";
import { SuiSelectBase } from "../classes/select-base";
Expand Down Expand Up @@ -57,8 +57,8 @@ export class SuiSelect<T, U> extends SuiSelectBase<T, U> implements ICustomValue
this._placeholder = placeholder;
}

constructor(element:ElementRef, localizationService:SuiLocalizationService) {
super(element, localizationService);
constructor(element:ElementRef, renderer:Renderer2, localizationService:SuiLocalizationService) {
super(element, renderer, localizationService);

this.selectedOptionChange = new EventEmitter<U>();
}
Expand Down

0 comments on commit 5739d86

Please sign in to comment.