From 1f71d302d3e1562b75c444798822fe6627e3407b Mon Sep 17 00:00:00 2001 From: Radoslav Karaivanov Date: Fri, 25 Oct 2019 10:53:56 +0300 Subject: [PATCH] perf(igx-grid): Reduce detection cycles in grid sizing logic Cleaned up most of the DOM handlers to use `fromEvent` thus avoiding manual removing of event handlers. Use the resize observer as an actual observable, removing the extra grid subject for proxying the calls. Finally removed the `ngOnChanges` hook in the cell as it was obsolete since #3767 and #3916. Closes #6031 --- .../igniteui-angular/src/lib/core/utils.ts | 20 +++ .../src/lib/grids/cell.component.ts | 20 +-- .../src/lib/grids/grid-base.directive.ts | 147 ++++++------------ .../src/lib/grids/grid/grid.component.ts | 3 +- .../hierarchical-grid.component.ts | 12 +- .../grids/tree-grid/tree-grid.component.ts | 4 +- 6 files changed, 77 insertions(+), 129 deletions(-) diff --git a/projects/igniteui-angular/src/lib/core/utils.ts b/projects/igniteui-angular/src/lib/core/utils.ts index 1d4a72f24ea..05ee70ce248 100644 --- a/projects/igniteui-angular/src/lib/core/utils.ts +++ b/projects/igniteui-angular/src/lib/core/utils.ts @@ -1,5 +1,7 @@ import { Injectable, PLATFORM_ID, Inject } from '@angular/core'; import { isPlatformBrowser } from '@angular/common'; +import { Observable } from 'rxjs'; +import ResizeObserver from 'resize-observer-polyfill'; /** *@hidden @@ -321,3 +323,21 @@ export const NAVIGATION_KEYS = new Set([ export const ROW_EXPAND_KEYS = new Set('right down arrowright arrowdown'.split(' ')); export const ROW_COLLAPSE_KEYS = new Set('left up arrowleft arrowup'.split(' ')); export const SUPPORTED_KEYS = new Set([...Array.from(NAVIGATION_KEYS), 'tab', 'enter', 'f2', 'escape', 'esc']); + + +/** + * @hidden + * @internal + * + * Creates a new ResizeObserver on `target` and returns it as an Observable. + */ +export function resizeObservable(target: HTMLElement): Observable { + return new Observable((observer) => { + const instance = new ResizeObserver((entries: ResizeObserverEntry[]) => { + observer.next(entries); + }); + instance.observe(target); + const unsubscribe = () => instance.disconnect(); + return unsubscribe; + }); +} diff --git a/projects/igniteui-angular/src/lib/grids/cell.component.ts b/projects/igniteui-angular/src/lib/grids/cell.component.ts index 334d57c617f..fe52e7529e9 100644 --- a/projects/igniteui-angular/src/lib/grids/cell.component.ts +++ b/projects/igniteui-angular/src/lib/grids/cell.component.ts @@ -10,9 +10,7 @@ ViewChild, NgZone, OnInit, - OnDestroy, - OnChanges, - SimpleChanges + OnDestroy } from '@angular/core'; import { IgxTextHighlightDirective } from '../directives/text-highlight/text-highlight.directive'; import { GridBaseAPIService } from './api.service'; @@ -48,7 +46,7 @@ import { GridType } from './common/grid.interface'; templateUrl: './cell.component.html', providers: [HammerGesturesManager] }) -export class IgxGridCellComponent implements OnInit, OnChanges, OnDestroy { +export class IgxGridCellComponent implements OnInit, OnDestroy { private _vIndex = -1; /** @@ -654,20 +652,6 @@ export class IgxGridCellComponent implements OnInit, OnChanges, OnDestroy { return this.selectionService.selected(this.selectionNode); } - /** - * @hidden - * @internal - */ - public ngOnChanges(changes: SimpleChanges): void { - if (changes.value && !changes.value.firstChange) { - if (this.highlight) { - this.highlight.lastSearchInfo.searchedText = this.grid.lastSearchInfo.searchText; - this.highlight.lastSearchInfo.caseSensitive = this.grid.lastSearchInfo.caseSensitive; - this.highlight.lastSearchInfo.exactMatch = this.grid.lastSearchInfo.exactMatch; - } - } - } - /** * Starts/ends edit mode for the cell. * diff --git a/projects/igniteui-angular/src/lib/grids/grid-base.directive.ts b/projects/igniteui-angular/src/lib/grids/grid-base.directive.ts index 6b475212685..65c9b97ba4d 100644 --- a/projects/igniteui-angular/src/lib/grids/grid-base.directive.ts +++ b/projects/igniteui-angular/src/lib/grids/grid-base.directive.ts @@ -27,10 +27,9 @@ import { DoCheck, Directive } from '@angular/core'; -import ResizeObserver from 'resize-observer-polyfill'; -import { Subject, combineLatest, pipe } from 'rxjs'; +import { Subject, combineLatest, pipe, fromEvent } from 'rxjs'; import { takeUntil, first, filter, throttleTime, map } from 'rxjs/operators'; -import { cloneArray, isEdge, isNavigationKey, flatten, mergeObjects, isIE } from '../core/utils'; +import { cloneArray, isEdge, isNavigationKey, flatten, mergeObjects, isIE, resizeObservable } from '../core/utils'; import { DataType } from '../data-operations/data-util'; import { FilteringLogic, IFilteringExpression } from '../data-operations/filtering-expression.interface'; import { IGroupByRecord } from '../data-operations/groupby-record.interface'; @@ -169,7 +168,6 @@ export class IgxGridBaseDirective extends DisplayDensityBase implements private overlayIDs = []; private _filteringStrategy: IFilteringStrategy; - private _hostWidth; private _advancedFilteringOverlayId: string; private _advancedFilteringPositionSettings: PositionSettings = { verticalDirection: VerticalAlignment.Middle, @@ -674,7 +672,7 @@ export class IgxGridBaseDirective extends DisplayDensityBase implements */ @HostBinding('style.width') get hostWidth() { - return this._width || this._hostWidth; + return this._width; } /** * Returns the width of the `IgxGridComponent`. @@ -2632,6 +2630,8 @@ export class IgxGridBaseDirective extends DisplayDensityBase implements * @hidden */ protected destroy$ = new Subject(); + protected destructor = takeUntil(this.destroy$); + protected initialized = filter(() => !this._init); /** * @hidden @@ -2707,10 +2707,6 @@ export class IgxGridBaseDirective extends DisplayDensityBase implements protected _allowAdvancedFiltering = false; protected _filterMode = FilterMode.quickFilter; - protected observer: ResizeObserver = new ResizeObserver(() => {}); - - protected resizeNotify = new Subject(); - private columnListDiffer; private _hiddenColumnsText = ''; @@ -2759,7 +2755,7 @@ export class IgxGridBaseDirective extends DisplayDensityBase implements private verticalScrollHandler = (event) => { this.verticalScrollContainer.onScroll(event); - if (isEdge()) { this.wheelHandler(false); } + if (isEdge()) { this.wheelHandler(null, false); } this.disableTransitions = true; this.zone.run(() => { @@ -2778,7 +2774,7 @@ export class IgxGridBaseDirective extends DisplayDensityBase implements private horizontalScrollHandler = (event) => { const scrollLeft = event.target.scrollLeft; - if (isEdge()) { this.wheelHandler(true); } + if (isEdge()) { this.wheelHandler(null, true); } this.headerContainer.onHScroll(scrollLeft); this._horizontalForOfs.forEach(vfor => vfor.onHScroll(scrollLeft)); this.cdr.markForCheck(); @@ -2799,11 +2795,8 @@ export class IgxGridBaseDirective extends DisplayDensityBase implements public hideOverlays() { this.overlayIDs.forEach(overlayID => { this.overlayService.hide(overlayID); - this.overlayService.onClosed.pipe( - filter(o => o.id === overlayID), - takeUntil(this.destroy$)).subscribe(() => { - this.nativeElement.focus(); - }); + this.overlayService.onClosed.pipe(this.destructor, filter(o => o.id === overlayID)) + .subscribe(() => this.nativeElement.focus()); }); } @@ -2853,15 +2846,14 @@ export class IgxGridBaseDirective extends DisplayDensityBase implements } _setupListeners() { - const destructor = takeUntil(this.destroy$); - this.onRowAdded.pipe(destructor).subscribe(args => this.refreshGridState(args)); - this.onRowDeleted.pipe(destructor).subscribe(args => { + this.onRowAdded.pipe(this.destructor).subscribe(args => this.refreshGridState(args)); + this.onRowDeleted.pipe(this.destructor).subscribe(args => { this.summaryService.deleteOperation = true; this.summaryService.clearSummaryCache(args); }); - this.transactions.onStateUpdate.pipe(destructor).subscribe(() => { + this.transactions.onStateUpdate.pipe(this.destructor).subscribe(() => { this.selectionService.clearHeaderCBState(); this.summaryService.clearSummaryCache(); this._pipeTrigger++; @@ -2874,22 +2866,15 @@ export class IgxGridBaseDirective extends DisplayDensityBase implements } }); - this.resizeNotify.pipe(destructor, filter(() => !this._init), throttleTime(100)) - .subscribe(() => { - this.zone.run(() => { - this.notifyChanges(true); - }); - }); - - this.onPagingDone.pipe(destructor).subscribe(() => { + this.onPagingDone.pipe(this.destructor).subscribe(() => { this.endEdit(true); this.selectionService.clear(true); }); - this.onColumnMoving.pipe(destructor).subscribe(() => this.endEdit(true)); - this.onColumnResized.pipe(destructor).subscribe(() => this.endEdit(true)); + this.onColumnMoving.pipe(this.destructor).subscribe(() => this.endEdit(true)); + this.onColumnResized.pipe(this.destructor).subscribe(() => this.endEdit(true)); - this.overlayService.onOpening.pipe(destructor).subscribe((event) => { + this.overlayService.onOpening.pipe(this.destructor).subscribe((event) => { if (this._advancedFilteringOverlayId === event.id) { const instance = event.componentRef.instance as IgxAdvancedFilteringDialogComponent; if (instance) { @@ -2898,7 +2883,7 @@ export class IgxGridBaseDirective extends DisplayDensityBase implements } }); - this.overlayService.onOpened.pipe(destructor).subscribe((event) => { + this.overlayService.onOpened.pipe(this.destructor).subscribe((event) => { // do not hide the advanced filtering overlay on scroll if (this._advancedFilteringOverlayId === event.id) { return; @@ -2910,7 +2895,7 @@ export class IgxGridBaseDirective extends DisplayDensityBase implements } }); - this.overlayService.onClosed.pipe(destructor, filter(() => !this._init)).subscribe((event) => { + this.overlayService.onClosed.pipe(this.destructor, this.initialized).subscribe((event) => { if (this._advancedFilteringOverlayId === event.id) { this._advancedFilteringOverlayId = null; return; @@ -2922,18 +2907,18 @@ export class IgxGridBaseDirective extends DisplayDensityBase implements } }); - this.verticalScrollContainer.onDataChanging.pipe(destructor, filter(() => !this._init)).subscribe(($event) => { + this.verticalScrollContainer.onDataChanging.pipe(this.destructor, this.initialized).subscribe(($event) => { this.calculateGridHeight(); $event.containerSize = this.calcHeight; this.evaluateLoadingState(); this.notifyChanges(true); }); - this.verticalScrollContainer.onContentSizeChange.pipe(destructor, filter(() => !this._init)).subscribe(($event) => { + this.verticalScrollContainer.onContentSizeChange.pipe(this.destructor, this.initialized).subscribe(() => { this.calculateGridSizes(); }); - this.onDensityChanged.pipe(destructor).subscribe(() => { + this.onDensityChanged.pipe(this.destructor).subscribe(() => { this.summaryService.summaryHeight = 0; this.endEdit(true); this.cdr.markForCheck(); @@ -2962,7 +2947,7 @@ export class IgxGridBaseDirective extends DisplayDensityBase implements this.columnListDiffer.diff(this.columnList); this.columnList.changes - .pipe(takeUntil(this.destroy$)) + .pipe(this.destructor) .subscribe((change: QueryList) => { this.onColumnsChanged(change); }); } @@ -3034,7 +3019,7 @@ export class IgxGridBaseDirective extends DisplayDensityBase implements const rowListObserver = extractForOfs(this._dataRowList.changes); const summaryRowObserver = extractForOfs(this._summaryRowList.changes); - combineLatest([rowListObserver, summaryRowObserver]).pipe(takeUntil(this.destroy$)) + combineLatest([rowListObserver, summaryRowObserver]).pipe(this.destructor) .subscribe(([row, summary]) => this._horizontalForOfs = [...row, ...summary]); this._horizontalForOfs = [ @@ -3045,12 +3030,18 @@ export class IgxGridBaseDirective extends DisplayDensityBase implements public _zoneBegoneListeners() { this.zone.runOutsideAngular(() => { - this.nativeElement.addEventListener('keydown', this.keydownHandler); - this.verticalScrollContainer.getScroll().addEventListener('scroll', this.verticalScrollHandler); - this.headerContainer.getScroll().addEventListener('scroll', this.horizontalScrollHandler); + fromEvent(this.nativeElement, 'keydown').pipe(this.destructor).subscribe(this.keydownHandler); + + fromEvent(this.verticalScrollContainer.getScroll(), 'scroll').pipe(this.destructor) + .subscribe(this.verticalScrollHandler); + + fromEvent(this.headerContainer.getScroll(), 'scroll').pipe(this.destructor) + .subscribe(this.horizontalScrollHandler); - this.observer = new ResizeObserver(() => this.resizeNotify.next()); - this.observer.observe(this.nativeElement); + resizeObservable(this.nativeElement).pipe(this.destructor, throttleTime(40)).subscribe(() => { + const callback = () => this.notifyChanges(true); + NgZone.isInAngularZone() ? callback() : this.zone.run(callback); + }); }); } @@ -3066,8 +3057,8 @@ export class IgxGridBaseDirective extends DisplayDensityBase implements this._zoneBegoneListeners(); const vertScrDC = this.verticalScrollContainer.displayContainer; - vertScrDC.addEventListener('scroll', this.scrollHandler); - vertScrDC.addEventListener('wheel', () => this.wheelHandler()); + fromEvent(vertScrDC, 'scroll').pipe(this.destructor).subscribe(this.scrollHandler); + fromEvent(vertScrDC, 'wheel').pipe(this.destructor).subscribe(this.wheelHandler); } @@ -3116,16 +3107,6 @@ export class IgxGridBaseDirective extends DisplayDensityBase implements if (this._advancedFilteringOverlayId) { this.overlayService.hide(this._advancedFilteringOverlayId); } - - this.zone.runOutsideAngular(() => { - this.observer.disconnect(); - this.nativeElement.removeEventListener('keydown', this.keydownHandler); - this.verticalScrollContainer.getScroll().removeEventListener('scroll', this.verticalScrollHandler); - this.headerContainer.getScroll().removeEventListener('scroll', this.horizontalScrollHandler); - const vertScrDC = this.verticalScrollContainer.displayContainer; - vertScrDC.removeEventListener('scroll', this.scrollHandler); - vertScrDC.removeEventListener('wheel', () => this.wheelHandler()); - }); } /** @@ -4623,23 +4604,20 @@ export class IgxGridBaseDirective extends DisplayDensityBase implements /** * @hidden + * @internal */ protected calculateGridSizes() { - /* - TODO: (R.K.) This layered lasagne should be refactored - ASAP. The reason I have to reset the caches so many times is because - after teach `detectChanges` call they are filled with invalid - state. Of course all of this happens midway through the grid - sizing process which of course, uses values from the caches, thus resulting - in a broken layout. - */ this.resetCaches(); - this.cdr.detectChanges(); - const hasScroll = this.hasVerticalSroll(); + /** + * R.K. + * Explore the option to extract the column sizing logic from + * the `calculateGridWidth` call and run it after the grid is sized + * horizontally and a detection cycle has passed. + */ this.calculateGridWidth(); - this.resetCaches(); this.cdr.detectChanges(); this.calculateGridHeight(); + this.cdr.detectChanges(); if (this.rowEditable) { this.repositionRowEditingOverlay(this.rowInEditMode); @@ -4648,40 +4626,6 @@ export class IgxGridBaseDirective extends DisplayDensityBase implements if (this.filteringService.isFilterRowVisible) { this.filteringRow.resetChipsArea(); } - - this.cdr.detectChanges(); - // in case scrollbar has appeared recalc to size correctly. - if (hasScroll !== this.hasVerticalSroll()) { - this.calculateGridWidth(); - this.cdr.detectChanges(); - } - if (this.zone.isStable) { - this.zone.run(() => { - this._applyWidthHostBinding(); - this.cdr.detectChanges(); - }); - } else { - this.zone.onStable.pipe(first()).subscribe(() => { - this.zone.run(() => { - this._applyWidthHostBinding(); - }); - }); - } - this.resetCaches(); - } - - private _applyWidthHostBinding() { - let width = this._width; - if (width === null) { - let currentWidth = this.calcWidth; - if (this.hasVerticalSroll()) { - currentWidth += this.scrollWidth; - } - width = currentWidth + 'px'; - this.resetCaches(); - } - this._hostWidth = width; - this.cdr.markForCheck(); } @@ -5364,8 +5308,9 @@ export class IgxGridBaseDirective extends DisplayDensityBase implements /** * @hidden + * @internal */ - public wheelHandler = (isScroll = false) => { + public wheelHandler = (_?: any, isScroll = false) => { if (this.document.activeElement && // tslint:disable-next-line:no-bitwise (this.document.activeElement.compareDocumentPosition(this.tbody.nativeElement) & Node.DOCUMENT_POSITION_CONTAINS || diff --git a/projects/igniteui-angular/src/lib/grids/grid/grid.component.ts b/projects/igniteui-angular/src/lib/grids/grid/grid.component.ts index cfefa0215fb..110beb039f7 100644 --- a/projects/igniteui-angular/src/lib/grids/grid/grid.component.ts +++ b/projects/igniteui-angular/src/lib/grids/grid/grid.component.ts @@ -15,7 +15,6 @@ import { IGroupByExpandState } from '../../data-operations/groupby-expand-state. import { IBaseChipEventArgs, IChipClickEventArgs, IChipKeyDownEventArgs } from '../../chips/chip.component'; import { IChipsAreaReorderEventArgs } from '../../chips/chips-area.component'; import { IgxColumnComponent } from '../columns/column.component'; -import { takeUntil } from 'rxjs/operators'; import { IgxFilteringService } from '../filtering/grid-filtering.service'; import { IGroupingExpression } from '../../data-operations/grouping-expression.interface'; import { IgxColumnResizingService } from '../resizing/resizing.service'; @@ -945,7 +944,7 @@ export class IgxGridComponent extends IgxGridBaseDirective implements GridType, public ngOnInit() { super.ngOnInit(); - this.onGroupingDone.pipe(takeUntil(this.destroy$)).subscribe((args) => { + this.onGroupingDone.pipe(this.destructor).subscribe((args) => { this.endEdit(true); this.summaryService.updateSummaryCache(args); }); diff --git a/projects/igniteui-angular/src/lib/grids/hierarchical-grid/hierarchical-grid.component.ts b/projects/igniteui-angular/src/lib/grids/hierarchical-grid/hierarchical-grid.component.ts index 574634f4c3c..16a2efeb822 100644 --- a/projects/igniteui-angular/src/lib/grids/hierarchical-grid/hierarchical-grid.component.ts +++ b/projects/igniteui-angular/src/lib/grids/hierarchical-grid/hierarchical-grid.component.ts @@ -380,7 +380,7 @@ export class IgxHierarchicalGridComponent extends IgxHierarchicalGridBaseDirecti this.cdr.detectChanges(); } - this.verticalScrollContainer.onBeforeViewDestroyed.pipe(takeUntil(this.destroy$)).subscribe((view) => { + this.verticalScrollContainer.onBeforeViewDestroyed.pipe(this.destructor).subscribe((view) => { const rowData = view.context.$implicit; if (this.isChildGridRecord(rowData)) { const cachedData = this.childGridTemplates.get(rowData.rowID); @@ -393,7 +393,7 @@ export class IgxHierarchicalGridComponent extends IgxHierarchicalGridBaseDirecti if (this.parent) { this._displayDensity = this.rootGrid._displayDensity; - this.rootGrid.onDensityChanged.pipe(takeUntil(this.destroy$)).subscribe(() => { + this.rootGrid.onDensityChanged.pipe(this.destructor).subscribe(() => { this._displayDensity = this.rootGrid._displayDensity; this.notifyChanges(true); this.cdr.markForCheck(); @@ -457,11 +457,11 @@ export class IgxHierarchicalGridComponent extends IgxHierarchicalGridBaseDirecti ngAfterContentInit() { this.updateColumnList(false); this.childLayoutKeys = this.parent ? - this.parentIsland.children.map((item) => item.key) : - this.childLayoutKeys = this.childLayoutList.map((item) => item.key); + this.parentIsland.children.map((item) => item.key) : + this.childLayoutKeys = this.childLayoutList.map((item) => item.key); this.childLayoutList.notifyOnChanges(); - this.childLayoutList.changes.pipe(takeUntil(this.destroy$)) - .subscribe(() => this.onRowIslandChange()); + this.childLayoutList.changes.pipe(this.destructor) + .subscribe(() => this.onRowIslandChange()); super.ngAfterContentInit(); } diff --git a/projects/igniteui-angular/src/lib/grids/tree-grid/tree-grid.component.ts b/projects/igniteui-angular/src/lib/grids/tree-grid/tree-grid.component.ts index e002ccb4974..4d52b0d461a 100644 --- a/projects/igniteui-angular/src/lib/grids/tree-grid/tree-grid.component.ts +++ b/projects/igniteui-angular/src/lib/grids/tree-grid/tree-grid.component.ts @@ -26,7 +26,7 @@ import { IgxTreeGridNavigationService } from './tree-grid-navigation.service'; import { IgxGridSummaryService } from '../summaries/grid-summary.service'; import { IgxGridSelectionService, IgxGridCRUDService } from '../selection/selection.service'; import { mergeObjects } from '../../core/utils'; -import { first, takeUntil } from 'rxjs/operators'; +import { first } from 'rxjs/operators'; import { IgxRowLoadingIndicatorTemplateDirective } from './tree-grid.directives'; import { IgxForOfSyncService, IgxForOfScrollSyncService } from '../../directives/for-of/for_of.sync.service'; import { IgxDragIndicatorIconDirective } from '../row-drag.directive'; @@ -419,7 +419,7 @@ export class IgxTreeGridComponent extends IgxGridBaseDirective implements GridTy public ngOnInit() { super.ngOnInit(); - this.onRowToggle.pipe(takeUntil(this.destroy$)).subscribe((args) => { + this.onRowToggle.pipe(this.destructor).subscribe((args) => { this.loadChildrenOnRowExpansion(args); }); }