From 8612af9c50c9c714fa3bb940c8d817542be6efaa Mon Sep 17 00:00:00 2001 From: Rado Kirov Date: Thu, 15 Jan 2015 13:03:50 -0800 Subject: [PATCH] fix(views): adds dehydration calls to ng-repeat removed views. Closes #416 --- modules/core/src/compiler/viewport.js | 23 +++-- modules/core/test/compiler/viewport_spec.js | 105 +++++++++++++++++++- modules/directives/src/ng_repeat.js | 6 +- 3 files changed, 121 insertions(+), 13 deletions(-) diff --git a/modules/core/src/compiler/viewport.js b/modules/core/src/compiler/viewport.js index bef0a64254..52f598b76a 100644 --- a/modules/core/src/compiler/viewport.js +++ b/modules/core/src/compiler/viewport.js @@ -89,18 +89,29 @@ export class ViewPort { return view; } - remove(atIndex=-1): View { + remove(atIndex=-1) { if (atIndex == -1) atIndex = this._views.length - 1; - var removedView = this.get(atIndex); + var view = this.detach(atIndex); + view.dehydrate(); + // view is intentionally not returned to the client. + } + + /** + * The method can be used together with insert to implement a view move, i.e. + * moving the dom nodes while the directives in the view stay intact. + */ + detach(atIndex=-1): View { + if (atIndex == -1) atIndex = this._views.length - 1; + var detachedView = this.get(atIndex); ListWrapper.removeAt(this._views, atIndex); if (isBlank(this._lightDom)) { - ViewPort.removeViewNodesFromParent(this.templateElement.parentNode, removedView); + ViewPort.removeViewNodesFromParent(this.templateElement.parentNode, detachedView); } else { this._lightDom.redistribute(); } - removedView.changeDetector.remove(); - this._unlinkElementInjectors(removedView); - return removedView; + detachedView.changeDetector.remove(); + this._unlinkElementInjectors(detachedView); + return detachedView; } contentTagContainers() { diff --git a/modules/core/test/compiler/viewport_spec.js b/modules/core/test/compiler/viewport_spec.js index 61c9ddbc65..be9d8099ad 100644 --- a/modules/core/test/compiler/viewport_spec.js +++ b/modules/core/test/compiler/viewport_spec.js @@ -1,11 +1,12 @@ import {describe, xit, it, expect, beforeEach, ddescribe, iit, el} from 'test_lib/test_lib'; import {View, ProtoView} from 'core/compiler/view'; import {ViewPort} from 'core/compiler/viewport'; +import {proxy, IMPLEMENTS} from 'facade/lang'; import {DOM} from 'facade/dom'; import {ListWrapper, MapWrapper} from 'facade/collection'; import {Injector} from 'di/di'; import {ProtoElementInjector, ElementInjector} from 'core/compiler/element_injector'; -import {ProtoChangeDetector, Lexer, Parser} from 'change_detection/change_detection'; +import {ProtoChangeDetector, ChangeDetector, Lexer, Parser} from 'change_detection/change_detection'; function createView(nodes) { var view = new View(null, nodes, new ProtoChangeDetector(), MapWrapper.create()); @@ -13,6 +14,51 @@ function createView(nodes) { return view; } +@proxy +@IMPLEMENTS(ChangeDetector) +class AttachableChangeDetector { + parent; + constructor() { + } + remove() { + this.parent = null; + } + noSuchMethod(i) { + super.noSuchMethod(i); + } +} + +@proxy +@IMPLEMENTS(View) +class HydrateAwareFakeView { + isHydrated: boolean; + nodes: List; + changeDetector: ChangeDetector; + rootElementInjectors; + constructor(isHydrated) { + this.isHydrated = isHydrated; + this.nodes = [DOM.createElement('div')]; + this.rootElementInjectors = []; + this.changeDetector = new AttachableChangeDetector(); + } + + hydrated() { + return this.isHydrated; + } + + hydrate(_, __, ___) { + this.isHydrated = true; + } + + dehydrate() { + this.isHydrated = false; + } + + noSuchMethod(i) { + super.noSuchMethod(i); + } +} + export function main() { describe('viewport', () => { var viewPort, parentView, protoView, dom, customViewWithOneNode, @@ -81,10 +127,9 @@ export function main() { it('should remove the last view by default', () => { viewPort.insert(customViewWithOneNode); - var removedView = viewPort.remove(); + viewPort.remove(); expect(textInViewPort()).toEqual('filler'); - expect(removedView).toBe(customViewWithOneNode); expect(viewPort.length).toBe(1); }); @@ -92,13 +137,63 @@ export function main() { viewPort.insert(customViewWithOneNode); viewPort.insert(customViewWithTwoNodes); - var removedView = viewPort.remove(1); - expect(removedView).toBe(customViewWithOneNode); + viewPort.remove(1); + expect(textInViewPort()).toEqual('filler one two'); expect(viewPort.get(1)).toBe(customViewWithTwoNodes); expect(viewPort.length).toBe(2); }); + it('should detach the last view by default', () => { + viewPort.insert(customViewWithOneNode); + expect(viewPort.length).toBe(2); + + var detachedView = viewPort.detach(); + + expect(detachedView).toBe(customViewWithOneNode); + expect(textInViewPort()).toEqual('filler'); + expect(viewPort.length).toBe(1); + }); + + it('should detach the view at a given index', () => { + viewPort.insert(customViewWithOneNode); + viewPort.insert(customViewWithTwoNodes); + expect(viewPort.length).toBe(3); + + var detachedView = viewPort.detach(1); + expect(detachedView).toBe(customViewWithOneNode); + expect(textInViewPort()).toEqual('filler one two'); + expect(viewPort.length).toBe(2); + }); + + it('should keep views hydration state during insert', () => { + var hydratedView = new HydrateAwareFakeView(true); + var dehydratedView = new HydrateAwareFakeView(false); + viewPort.insert(hydratedView); + viewPort.insert(dehydratedView); + + expect(hydratedView.hydrated()).toBe(true); + expect(dehydratedView.hydrated()).toBe(false); + }); + + it('should dehydrate on remove', () => { + var hydratedView = new HydrateAwareFakeView(true); + viewPort.insert(hydratedView); + viewPort.remove(); + + expect(hydratedView.hydrated()).toBe(false); + }); + + it('should keep views hydration state during detach', () => { + var hydratedView = new HydrateAwareFakeView(true); + var dehydratedView = new HydrateAwareFakeView(false); + viewPort.insert(hydratedView); + viewPort.insert(dehydratedView); + + expect(viewPort.detach().hydrated()).toBe(false); + expect(viewPort.detach().hydrated()).toBe(true); + }); + it('should support adding/removing views with more than one node', () => { viewPort.insert(customViewWithTwoNodes); viewPort.insert(customViewWithOneNode); diff --git a/modules/directives/src/ng_repeat.js b/modules/directives/src/ng_repeat.js index c32931e916..3710e8d32a 100644 --- a/modules/directives/src/ng_repeat.js +++ b/modules/directives/src/ng_repeat.js @@ -59,10 +59,12 @@ export class NgRepeat extends OnChange { var movedTuples = []; for (var i = tuples.length - 1; i >= 0; i--) { var tuple = tuples[i]; - var view = viewPort.remove(tuple.record.previousIndex); + // separate moved views from removed views. if (isPresent(tuple.record.currentIndex)) { - tuple.view = view; + tuple.view = viewPort.detach(tuple.record.previousIndex); ListWrapper.push(movedTuples, tuple); + } else { + viewPort.remove(tuple.record.previousIndex); } } return movedTuples;