Permalink
Browse files

fix(core): ensure ngFor only inserts/moves/removes elements when nece…

…ssary (#10287)

Closes #9960
Closes #7239
Closes #9672
Closes #9454
Closes #10287
  • Loading branch information...
1 parent 4df7b1c commit e18626b7a21f9256987d63274a480c48a407999f @matsko matsko committed on GitHub Aug 1, 2016
@@ -117,24 +117,23 @@ export class NgFor implements DoCheck, OnChanges {
}
private _applyChanges(changes: DefaultIterableDiffer) {
- // TODO(rado): check if change detection can produce a change record that is
- // easier to consume than current.
- const recordViewTuples: RecordViewTuple[] = [];
- changes.forEachRemovedItem(
- (removedRecord: CollectionChangeRecord) =>
- recordViewTuples.push(new RecordViewTuple(removedRecord, null)));
-
- changes.forEachMovedItem(
- (movedRecord: CollectionChangeRecord) =>
- recordViewTuples.push(new RecordViewTuple(movedRecord, null)));
-
- const insertTuples = this._bulkRemove(recordViewTuples);
-
- changes.forEachAddedItem(
- (addedRecord: CollectionChangeRecord) =>
- insertTuples.push(new RecordViewTuple(addedRecord, null)));
-
- this._bulkInsert(insertTuples);
+ const insertTuples: RecordViewTuple[] = [];
+ changes.forEachOperation(
+ (item: CollectionChangeRecord, adjustedPreviousIndex: number, currentIndex: number) => {
+ if (item.previousIndex == null) {
+ let view = this._viewContainer.createEmbeddedView(
+ this._templateRef, new NgForRow(null, null, null), currentIndex);
+ let tuple = new RecordViewTuple(item, view);
+ insertTuples.push(tuple);
+ } else if (currentIndex == null) {
+ this._viewContainer.remove(adjustedPreviousIndex);
+ } else {
+ let view = this._viewContainer.get(adjustedPreviousIndex);
+ this._viewContainer.move(view, currentIndex);
+ let tuple = new RecordViewTuple(item, <EmbeddedViewRef<NgForRow>>view);
+ insertTuples.push(tuple);
+ }
+ });
for (let i = 0; i < insertTuples.length; i++) {
this._perViewChange(insertTuples[i].view, insertTuples[i].record);
@@ -155,39 +154,6 @@ export class NgFor implements DoCheck, OnChanges {
private _perViewChange(view: EmbeddedViewRef<NgForRow>, record: CollectionChangeRecord) {
view.context.$implicit = record.item;
}
-
- private _bulkRemove(tuples: RecordViewTuple[]): RecordViewTuple[] {
- tuples.sort(
- (a: RecordViewTuple, b: RecordViewTuple) =>
- a.record.previousIndex - b.record.previousIndex);
- const movedTuples: RecordViewTuple[] = [];
- for (let i = tuples.length - 1; i >= 0; i--) {
- const tuple = tuples[i];
- // separate moved views from removed views.
- if (isPresent(tuple.record.currentIndex)) {
- tuple.view =
- <EmbeddedViewRef<NgForRow>>this._viewContainer.detach(tuple.record.previousIndex);
- movedTuples.push(tuple);
- } else {
- this._viewContainer.remove(tuple.record.previousIndex);
- }
- }
- return movedTuples;
- }
-
- private _bulkInsert(tuples: RecordViewTuple[]): RecordViewTuple[] {
- tuples.sort((a, b) => a.record.currentIndex - b.record.currentIndex);
- for (let i = 0; i < tuples.length; i++) {
- var tuple = tuples[i];
- if (isPresent(tuple.view)) {
- this._viewContainer.insert(tuple.view, tuple.record.currentIndex);
- } else {
- tuple.view = this._viewContainer.createEmbeddedView(
- this._templateRef, new NgForRow(null, null, null), tuple.record.currentIndex);
- }
- }
- return tuples;
- }
}
class RecordViewTuple {
@@ -63,6 +63,56 @@ export class DefaultIterableDiffer implements IterableDiffer {
}
}
+ forEachOperation(
+ fn: (item: CollectionChangeRecord, previousIndex: number, currentIndex: number) => void) {
+ var nextIt = this._itHead;
+ var nextRemove = this._removalsHead;
+ var addRemoveOffset = 0;
+ var moveOffsets: number[] = null;
+ while (nextIt || nextRemove) {
+ // Figure out which is the next record to process
+ // Order: remove, add, move
+ let record = !nextRemove ||
+ nextIt &&
+ nextIt.currentIndex < getPreviousIndex(nextRemove, addRemoveOffset, moveOffsets) ?
+ nextIt :
+ nextRemove;
+ var adjPreviousIndex = getPreviousIndex(record, addRemoveOffset, moveOffsets);
+ var currentIndex = record.currentIndex;
+
+ // consume the item, and adjust the addRemoveOffset and update moveDistance if necessary
+ if (record === nextRemove) {
+ addRemoveOffset--;
+ nextRemove = nextRemove._nextRemoved;
+ } else {
+ nextIt = nextIt._next;
+ if (record.previousIndex == null) {
+ addRemoveOffset++;
+ } else {
+ // INVARIANT: currentIndex < previousIndex
+ if (!moveOffsets) moveOffsets = [];
+ let localMovePreviousIndex = adjPreviousIndex - addRemoveOffset;
+ let localCurrentIndex = currentIndex - addRemoveOffset;
+ if (localMovePreviousIndex != localCurrentIndex) {
+ for (var i = 0; i < localMovePreviousIndex; i++) {
+ var offset = i < moveOffsets.length ? moveOffsets[i] : (moveOffsets[i] = 0);
+ var index = offset + i;
+ if (localCurrentIndex <= index && index < localMovePreviousIndex) {
+ moveOffsets[i] = offset + 1;
+ }
+ }
+ var previousIndex = record.previousIndex;
+ moveOffsets[previousIndex] = localCurrentIndex - localMovePreviousIndex;
+ }
+ }
+ }
+
+ if (adjPreviousIndex !== currentIndex) {
+ fn(record, adjPreviousIndex, currentIndex);
+ }
+ }
+ }
+
forEachPreviousItem(fn: Function) {
var record: CollectionChangeRecord;
for (record = this._previousItHead; record !== null; record = record._nextPrevious) {
@@ -700,3 +750,13 @@ class _DuplicateMap {
toString(): string { return '_DuplicateMap(' + stringify(this.map) + ')'; }
}
+
+function getPreviousIndex(item: any, addRemoveOffset: number, moveOffsets: number[]): number {
+ var previousIndex = item.previousIndex;
+ if (previousIndex === null) return previousIndex;
+ var moveOffset = 0;
+ if (moveOffsets && previousIndex < moveOffsets.length) {
+ moveOffset = moveOffsets[previousIndex];
+ }
+ return previousIndex + addRemoveOffset + moveOffset;
+}
@@ -60,6 +60,30 @@ export class AppElement {
return result;
}
+ moveView(view: AppView<any>, currentIndex: number) {
+ var previousIndex = this.nestedViews.indexOf(view);
+ if (view.type === ViewType.COMPONENT) {
+ throw new BaseException(`Component views can't be moved!`);
+ }
+ var nestedViews = this.nestedViews;
+ if (nestedViews == null) {
+ nestedViews = [];
+ this.nestedViews = nestedViews;
+ }
+ ListWrapper.removeAt(nestedViews, previousIndex);
+ ListWrapper.insert(nestedViews, currentIndex, view);
+ var refRenderNode: any /** TODO #9100 */;
+ if (currentIndex > 0) {
+ var prevView = nestedViews[currentIndex - 1];
+ refRenderNode = prevView.lastRootNode;
+ } else {
+ refRenderNode = this.nativeElement;
+ }
+ if (isPresent(refRenderNode)) {
+ view.renderer.attachViewAfter(refRenderNode, view.flatRootNodes);
+ }
+ view.markContentChildAsMoved(this);
+ }
attachView(view: AppView<any>, viewIndex: number) {
if (view.type === ViewType.COMPONENT) {
@@ -288,6 +288,8 @@ export abstract class AppView<T> {
}
}
+ markContentChildAsMoved(renderAppElement: AppElement): void { this.dirtyParentQueriesInternal(); }
+
addToContentChildren(renderAppElement: AppElement): void {
renderAppElement.parentView.contentChildren.push(this);
this.viewContainerElement = renderAppElement;
@@ -100,6 +100,13 @@ export abstract class ViewContainerRef {
abstract insert(viewRef: ViewRef, index?: number): ViewRef;
/**
+ * Moves a View identified by a {@link ViewRef} into the container at the specified `index`.
+ *
+ * Returns the inserted {@link ViewRef}.
+ */
+ abstract move(viewRef: ViewRef, currentIndex: number): ViewRef;
+
+ /**
* Returns the index of the View, specified via {@link ViewRef}, within the current container or
* `-1` if this container doesn't contain the View.
*/
@@ -170,6 +177,14 @@ export class ViewContainerRef_ implements ViewContainerRef {
return wtfLeave(s, viewRef_);
}
+ move(viewRef: ViewRef, currentIndex: number): ViewRef {
+ var s = this._insertScope();
+ if (currentIndex == -1) return;
+ var viewRef_ = <ViewRef_<any>>viewRef;
+ this._element.moveView(viewRef_.internalView, currentIndex);
+ return wtfLeave(s, viewRef_);
+ }
+
indexOf(viewRef: ViewRef): number {
return ListWrapper.indexOf(this._element.nestedViews, (<ViewRef_<any>>viewRef).internalView);
}
Oops, something went wrong.

0 comments on commit e18626b

Please sign in to comment.