Skip to content

Commit

Permalink
perf(item): reorder is only added to the DOM if needed
Browse files Browse the repository at this point in the history
I have measured the performance impact of this change, since we use the push change detector strategy, the *ngIf is only evaluated once.

Items wrapped around an element with the ListReorder directive will receive a hidden `<ion-reorder>` in their DOM, but items that are not wrapped (i.e. they CAN NOT be reordered) will not even have the `<ion-reorder>` element in their DOM.

fixes #9065
  • Loading branch information
manucorporat committed Nov 8, 2016
1 parent ac157c0 commit dec5a0b
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 45 deletions.
14 changes: 7 additions & 7 deletions src/components/item/item-reorder-gesture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export class ItemReorderGesture {
console.error('ion-reorder does not contain $ionComponent');
return false;
}
this.reorderList.reorderPrepare();
this.reorderList._reorderPrepare();

let item = reorderMark.getReorderNode();
if (!item) {
Expand All @@ -63,13 +63,13 @@ export class ItemReorderGesture {
this.lastToIndex = indexForItem(item);

this.windowHeight = window.innerHeight - AUTO_SCROLL_MARGIN;
this.lastScrollPosition = this.reorderList.scrollContent(0);
this.lastScrollPosition = this.reorderList._scrollContent(0);

this.offset = pointerCoord(ev);
this.offset.y += this.lastScrollPosition;

item.classList.add(ITEM_REORDER_ACTIVE);
this.reorderList.reorderStart();
this.reorderList._reorderStart();
return true;
}

Expand Down Expand Up @@ -97,7 +97,7 @@ export class ItemReorderGesture {
this.lastToIndex = toIndex;
this.lastYcoord = posY;
this.emptyZone = false;
this.reorderList.reorderMove(fromIndex, toIndex, this.selectedItemHeight);
this.reorderList._reorderMove(fromIndex, toIndex, this.selectedItemHeight);
}
} else {
this.emptyZone = true;
Expand Down Expand Up @@ -133,7 +133,7 @@ export class ItemReorderGesture {
} else {
reorderInactive();
}
this.reorderList.reorderEmit(fromIndex, toIndex);
this.reorderList._reorderEmit(fromIndex, toIndex);
}

private itemForCoord(coord: PointerCoordinates): HTMLElement {
Expand All @@ -142,9 +142,9 @@ export class ItemReorderGesture {

private scroll(posY: number): number {
if (posY < AUTO_SCROLL_MARGIN) {
this.lastScrollPosition = this.reorderList.scrollContent(-SCROLL_JUMP);
this.lastScrollPosition = this.reorderList._scrollContent(-SCROLL_JUMP);
} else if (posY > this.windowHeight) {
this.lastScrollPosition = this.reorderList.scrollContent(SCROLL_JUMP);
this.lastScrollPosition = this.reorderList._scrollContent(SCROLL_JUMP);
}
return this.lastScrollPosition;
}
Expand Down
41 changes: 7 additions & 34 deletions src/components/item/item-reorder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,19 +133,10 @@ export interface ReorderIndexes {
})
export class ItemReorder {

/** @private */
_enableReorder: boolean = false;

/** @private */
_visibleReorder: boolean = false;

/** @private */
_reorderGesture: ItemReorderGesture;

/** @private */
_lastToIndex: number = -1;

/** @private */
_element: HTMLElement;

/**
Expand Down Expand Up @@ -196,10 +187,7 @@ export class ItemReorder {
}
}

/**
* @private
*/
reorderPrepare() {
_reorderPrepare() {
let ele = this._element;
let children: any = ele.children;
for (let i = 0, ilen = children.length; i < ilen; i++) {
Expand All @@ -209,18 +197,12 @@ export class ItemReorder {
}
}

/**
* @private
*/
reorderStart() {
_reorderStart() {
this.setElementClass('reorder-list-active', true);
}

/**
* @private
*/
reorderEmit(fromIndex: number, toIndex: number) {
this.reorderReset();
_reorderEmit(fromIndex: number, toIndex: number) {
this._reorderReset();
if (fromIndex !== toIndex) {
this._zone.run(() => {
this.ionItemReorder.emit({
Expand All @@ -231,21 +213,15 @@ export class ItemReorder {
}
}

/**
* @private
*/
scrollContent(scroll: number) {
_scrollContent(scroll: number) {
let scrollTop = this._content.getScrollTop() + scroll;
if (scroll !== 0) {
this._content.scrollTo(0, scrollTop, 0);
}
return scrollTop;
}

/**
* @private
*/
reorderReset() {
_reorderReset() {
let children = this._element.children;
let len = children.length;

Expand All @@ -257,10 +233,7 @@ export class ItemReorder {
this._lastToIndex = -1;
}

/**
* @private
*/
reorderMove(fromIndex: number, toIndex: number, itemHeight: number) {
_reorderMove(fromIndex: number, toIndex: number, itemHeight: number) {
if (this._lastToIndex === -1) {
this._lastToIndex = fromIndex;
}
Expand Down
1 change: 1 addition & 0 deletions src/components/item/item-sliding-gesture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const DRAG_THRESHOLD = 10;
const MAX_ATTACK_ANGLE = 20;

export class ItemSlidingGesture extends PanGesture {

private preSelectedContainer: ItemSliding = null;
private selectedContainer: ItemSliding = null;
private openContainer: ItemSliding = null;
Expand Down
16 changes: 12 additions & 4 deletions src/components/item/item.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import { ChangeDetectionStrategy, Component, ContentChild, ContentChildren, Directive, ElementRef, Input, QueryList, Renderer, ViewChild, ViewEncapsulation } from '@angular/core';
import { ChangeDetectionStrategy, Component, ContentChild, ContentChildren, Directive, ElementRef, Input, QueryList, Renderer, Optional, ViewChild, ViewEncapsulation } from '@angular/core';

import { Button } from '../button/button';
import { Config } from '../../config/config';
import { Form } from '../../util/form';
import { Icon } from '../icon/icon';
import { Ion } from '../ion';
import { Label } from '../label/label';
import { ItemReorder } from './item-reorder';


/**
Expand Down Expand Up @@ -283,7 +284,7 @@ import { Label } from '../label/label';
'<ng-content select="ion-select,ion-input,ion-textarea,ion-datetime,ion-range,[item-content]"></ng-content>' +
'</div>' +
'<ng-content select="[item-right],ion-radio,ion-toggle"></ng-content>' +
'<ion-reorder></ion-reorder>' +
'<ion-reorder *ngIf="_shouldHaveReorder"></ion-reorder>' +
'</div>' +
'<div class="button-effect"></div>',
host: {
Expand All @@ -297,6 +298,7 @@ export class Item extends Ion {
_inputs: Array<string> = [];
_label: Label;
_viewLabel: boolean = true;
_shouldHaveReorder: boolean = false;

/**
* @private
Expand Down Expand Up @@ -324,9 +326,15 @@ export class Item extends Ion {
this._setMode('item', val);
}

constructor(form: Form, config: Config, elementRef: ElementRef, renderer: Renderer) {
constructor(
form: Form,
config: Config,
elementRef: ElementRef,
renderer: Renderer,
@Optional() reorder: ItemReorder
) {
super(config, elementRef, renderer);

this._shouldHaveReorder = !!reorder;
this.mode = config.get('mode');
this.id = form.nextId().toString();
}
Expand Down
1 change: 1 addition & 0 deletions src/navigation/nav-controller-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { TransitionController } from '../transitions/transition-controller';
* This class is for internal use only. It is not exported publicly.
*/
export class NavControllerBase extends Ion implements NavController {

_children: any[] = [];
_ids: number = -1;
_init = false;
Expand Down

0 comments on commit dec5a0b

Please sign in to comment.