From c1e70ce8e0c8c510f0dee4de043e78f61c3e9c3d Mon Sep 17 00:00:00 2001 From: Paul Rohde Date: Thu, 6 Mar 2014 16:47:52 -0800 Subject: [PATCH] feat(blockhole): Change blockhole to have the insert / remove / move methods. Closes #689 --- lib/core_dom/block.dart | 119 +++++++++------------------- lib/core_dom/block_factory.dart | 13 +-- lib/directive/ng_if.dart | 8 +- lib/directive/ng_include.dart | 22 ++--- lib/directive/ng_repeat.dart | 19 ++--- lib/directive/ng_switch.dart | 12 +-- lib/routing/ng_view.dart | 22 ++--- test/core_dom/block_spec.dart | 84 +++++--------------- test/core_dom/compiler_spec.dart | 4 +- test/core_dom/ng_mustache_spec.dart | 8 +- test/directive/ng_if_spec.dart | 2 +- 11 files changed, 116 insertions(+), 197 deletions(-) diff --git a/lib/core_dom/block.dart b/lib/core_dom/block.dart index 14d77139a..45a15a038 100644 --- a/lib/core_dom/block.dart +++ b/lib/core_dom/block.dart @@ -1,104 +1,61 @@ part of angular.core.dom; -/** -* ElementWrapper is an interface for [Block]s and [BlockHole]s. Its purpose is -* to allow treating [Block] and [BlockHole] under same interface so that -* [Block]s can be added after [BlockHole]. -*/ -abstract class ElementWrapper { - List elements; - ElementWrapper next; - ElementWrapper previous; -} - /** * A Block is a fundamental building block of DOM. It is a chunk of DOM which - * can not be structural changed. It can only have its attributes changed. - * A Block can have [BlockHole]s embedded in its DOM. A [BlockHole] can - * contain other [Block]s and it is the only way in which DOM can be changed - * structurally. + * can not be structurally changed. A Block can have [BlockHole] placeholders + * embedded in its DOM. A [BlockHole] can contain other [Block]s and it is the + * only way in which DOM structure can be modified. * * A [Block] is a collection of DOM nodes - * + * A [Block] can be created from [BlockFactory]. * */ -class Block implements ElementWrapper { - List elements; - ElementWrapper next; - ElementWrapper previous; +class Block { + final List nodes; + Block(this.nodes); +} +/** + * A BlockHole maintains an ordered list of [Block]'s. It contains a + * [placeholder] node that is used as the insertion point for block nodes. + */ +class BlockHole { + final dom.Node placeholder; final NgAnimate _animate; + final List _blocks = []; - Block(this.elements, this._animate); - - Block insertAfter(ElementWrapper previousBlock) { - // Update Link List. - next = previousBlock.next; - if (next != null) { - next.previous = this; - } - previous = previousBlock; - previousBlock.next = this; + BlockHole(this.placeholder, this._animate); - // Update DOM - List previousElements = previousBlock.elements; - dom.Node previousElement = previousElements[previousElements.length - 1]; - dom.Node insertBeforeElement = previousElement.nextNode; - dom.Node parentElement = previousElement.parentNode; + void insert(Block block, { Block insertAfter }) { + dom.Node previousNode = _lastNode(insertAfter); + _blocksInsertAfter(block, insertAfter); - _animate.insert(elements, parentElement, insertBefore: insertBeforeElement); - - return this; + _animate.insert(block.nodes, placeholder.parentNode, + insertBefore: previousNode.nextNode); } - Block remove() { - bool preventDefault = false; - - _animate.remove(elements); - - // Remove block from list - if (previous != null && (previous.next = next) != null) { - next.previous = previous; - } - next = previous = null; - return this; + void remove(Block block) { + _blocks.remove(block); + _animate.remove(block.nodes); } - Block moveAfter(ElementWrapper previousBlock) { - var previousElements = previousBlock.elements, - previousElement = previousElements[previousElements.length - 1], - insertBeforeElement = previousElement.nextNode, - parentElement = previousElement.parentNode; - - elements.forEach((el) => parentElement.insertBefore(el, insertBeforeElement)); + void move(Block block, { Block moveAfter }) { + dom.Node previousNode = _lastNode(moveAfter); + _blocks.remove(block); + _blocksInsertAfter(block, moveAfter); - // Remove block from list - previous.next = next; - if (next != null) { - next.previous = previous; - } - // Add block to list - next = previousBlock.next; - if (next != null) { - next.previous = this; - } - previous = previousBlock; - previousBlock.next = this; - return this; + _animate.move(block.nodes, placeholder.parentNode, + insertBefore: previousNode.nextNode); } -} -/** - * A BlockHole is an instance of a hole. BlockHoles designate where child - * [Block]s can be added in parent [Block]. BlockHoles wrap a DOM element, - * and act as references which allows more blocks to be added. - */ -class BlockHole extends ElementWrapper { - List elements; - ElementWrapper previous; - ElementWrapper next; + void _blocksInsertAfter(Block block, Block insertAfter) { + int index = (insertAfter != null) ? _blocks.indexOf(insertAfter) : -1; + _blocks.insert(index + 1, block); + } - BlockHole(this.elements); + dom.Node _lastNode(Block insertAfter) => + insertAfter == null + ? placeholder + : insertAfter.nodes[insertAfter.nodes.length - 1]; } - diff --git a/lib/core_dom/block_factory.dart b/lib/core_dom/block_factory.dart index bfc3ae6ce..48289214d 100644 --- a/lib/core_dom/block_factory.dart +++ b/lib/core_dom/block_factory.dart @@ -35,13 +35,13 @@ class BlockFactory implements Function { BoundBlockFactory bind(Injector injector) => new BoundBlockFactory(this, injector); - Block call(Injector injector, [List elements]) { - if (elements == null) elements = cloneElements(templateElements); + Block call(Injector injector, [List nodes]) { + if (nodes == null) nodes = cloneElements(templateElements); var timerId; try { assert((timerId = _perf.startTimer('ng.block')) != false); - var block = new Block(elements, injector.get(NgAnimate)); - _link(block, elements, directivePositions, injector); + var block = new Block(nodes); + _link(block, nodes, directivePositions, injector); return block; } finally { assert(_perf.stopTimer(timerId) != false); @@ -176,7 +176,8 @@ class BlockFactory implements Function { if (annotation.children == NgAnnotation.TRANSCLUDE_CHILDREN) { // Currently, transclude is only supported for NgDirective. assert(annotation is NgDirective); - blockHoleFactory = (_) => new BlockHole([node]); + blockHoleFactory = (_) => new BlockHole(node, + parentInjector.get(NgAnimate)); blockFactory = (_) => ref.blockFactory; boundBlockFactory = (Injector injector) => ref.blockFactory.bind(injector); @@ -384,7 +385,7 @@ class _ComponentFactory implements Function { attachBlockToShadowDom(BlockFactory blockFactory) { var block = blockFactory(shadowInjector); - shadowDom.nodes.addAll(block.elements); + shadowDom.nodes.addAll(block.nodes); return shadowDom; } diff --git a/lib/directive/ng_if.dart b/lib/directive/ng_if.dart index 548c536e4..29a8f2a56 100644 --- a/lib/directive/ng_if.dart +++ b/lib/directive/ng_if.dart @@ -28,18 +28,18 @@ abstract class _NgUnlessIfAttrDirectiveBase { if (_block == null) { _childScope = _scope.createChild(new PrototypeMap(_scope.context)); _block = _boundBlockFactory(_childScope); - var insertBlock = _block; + var block = _block; _scope.rootScope.domWrite(() { - insertBlock.insertAfter(_blockHole); + _blockHole.insert(block); }); } } void _ensureBlockDestroyed() { if (_block != null) { - var removeBlock = _block; + var block = _block; _scope.rootScope.domWrite(() { - removeBlock.remove(); + _blockHole.remove(block); }); _childScope.destroy(); _block = null; diff --git a/lib/directive/ng_include.dart b/lib/directive/ng_include.dart index f0e35e078..b607c803c 100644 --- a/lib/directive/ng_include.dart +++ b/lib/directive/ng_include.dart @@ -26,29 +26,29 @@ class NgIncludeDirective { final Injector injector; final DirectiveMap directives; - Block _previousBlock; - Scope _previousScope; + Block _block; + Scope _scope; NgIncludeDirective(this.element, this.scope, this.blockCache, this.injector, this.directives); _cleanUp() { - if (_previousBlock == null) return; + if (_block == null) return; - _previousBlock.remove(); - _previousScope.destroy(); + _block.nodes.forEach((node) => node.remove); + _scope.destroy(); element.innerHtml = ''; - _previousBlock = null; - _previousScope = null; + _block = null; + _scope = null; } _updateContent(createBlock) { // create a new scope - _previousScope = scope.createChild(new PrototypeMap(scope.context)); - _previousBlock = createBlock(injector.createChild([new Module() - ..value(Scope, _previousScope)])); + _scope = scope.createChild(new PrototypeMap(scope.context)); + _block = createBlock(injector.createChild([new Module() + ..value(Scope, _scope)])); - _previousBlock.elements.forEach((elm) => element.append(elm)); + _block.nodes.forEach((node) => element.append(node)); } diff --git a/lib/directive/ng_repeat.dart b/lib/directive/ng_repeat.dart index 70c92f74c..8e5d494dd 100644 --- a/lib/directive/ng_repeat.dart +++ b/lib/directive/ng_repeat.dart @@ -6,7 +6,7 @@ class _Row { Block block; dom.Element startNode; dom.Element endNode; - List elements; + List nodes; _Row(this.id); } @@ -177,7 +177,7 @@ class NgRepeatDirective { } // remove existing items _rows.forEach((key, row) { - row.block.remove(); + _blockHole.remove(row.block); row.scope.destroy(); }); _rows = newRows; @@ -185,12 +185,13 @@ class NgRepeatDirective { } _onCollectionChange(Iterable collection) { - dom.Node previousNode = _blockHole.elements[0]; // current position of the node + dom.Node previousNode = _blockHole.placeholder; // current position of the + // node dom.Node nextNode; Scope childScope; Map childContext; Scope trackById; - ElementWrapper cursor = _blockHole; + Block cursor; List<_Row> newRowOrder = _computeNewRows(collection, trackById); @@ -211,7 +212,7 @@ class NgRepeatDirective { if (row.startNode != nextNode) { // existing item which got moved - row.block.moveAfter(cursor); + _blockHole.move(row.block, moveAfter: cursor); } previousNode = row.endNode; } else { @@ -237,10 +238,10 @@ class NgRepeatDirective { _rows[row.id] = row ..block = block ..scope = childScope - ..elements = block.elements - ..startNode = row.elements[0] - ..endNode = row.elements[row.elements.length - 1]; - block.insertAfter(cursor); + ..nodes = block.nodes + ..startNode = row.nodes[0] + ..endNode = row.nodes[row.nodes.length - 1]; + _blockHole.insert(block, insertAfter: cursor); } cursor = row.block; } diff --git a/lib/directive/ng_switch.dart b/lib/directive/ng_switch.dart index fb9309bc7..42284cd36 100644 --- a/lib/directive/ng_switch.dart +++ b/lib/directive/ng_switch.dart @@ -74,7 +74,7 @@ class NgSwitchDirective { set value(val) { currentBlocks ..forEach((_BlockScopePair pair) { - pair.block.remove(); + pair.hole.remove(pair.block); pair.scope.destroy(); }) ..clear(); @@ -83,8 +83,10 @@ class NgSwitchDirective { (cases.containsKey(val) ? cases[val] : cases['?']) .forEach((_Case caze) { Scope childScope = scope.createChild(new PrototypeMap(scope.context)); - var block = caze.blockFactory(childScope)..insertAfter(caze.anchor); - currentBlocks.add(new _BlockScopePair(block, childScope)); + var block = caze.blockFactory(childScope); + caze.anchor.insert(block); + currentBlocks.add(new _BlockScopePair(block, caze.anchor, + childScope)); }); if (onChange != null) { onChange(); @@ -94,9 +96,10 @@ class NgSwitchDirective { class _BlockScopePair { final Block block; + final BlockHole hole; final Scope scope; - _BlockScopePair(this.block, this.scope); + _BlockScopePair(this.block, this.hole, this.scope); } class _Case { @@ -121,7 +124,6 @@ class NgSwitchWhenDirective { set value(String value) => ngSwitch.addCase('!$value', hole, blockFactory); } - @NgDirective( children: NgAnnotation.TRANSCLUDE_CHILDREN, selector: '[ng-switch-default]') diff --git a/lib/routing/ng_view.dart b/lib/routing/ng_view.dart index deb733e57..519830566 100644 --- a/lib/routing/ng_view.dart +++ b/lib/routing/ng_view.dart @@ -67,8 +67,8 @@ class NgViewDirective implements NgDetachAware, RouteProvider { final Scope scope; RouteHandle _route; - Block _previousBlock; - Scope _previousScope; + Block _block; + Scope _scope; Route _viewRoute; NgViewDirective(this.element, this.blockCache, @@ -116,23 +116,23 @@ class NgViewDirective implements NgDetachAware, RouteProvider { var newDirectives = viewInjector.get(DirectiveMap); blockCache.fromUrl(templateUrl, newDirectives).then((blockFactory) { _cleanUp(); - _previousScope = scope.createChild(new PrototypeMap(scope.context)); - _previousBlock = blockFactory( + _scope = scope.createChild(new PrototypeMap(scope.context)); + _block = blockFactory( viewInjector.createChild( - [new Module()..value(Scope, _previousScope)])); + [new Module()..value(Scope, _scope)])); - _previousBlock.elements.forEach((elm) => element.append(elm)); + _block.nodes.forEach((elm) => element.append(elm)); }); } _cleanUp() { - if (_previousBlock == null) return; + if (_block == null) return; - _previousBlock.remove(); - _previousScope.destroy(); + _block.nodes.forEach((node) => node.remove()); + _scope.destroy(); - _previousBlock = null; - _previousScope = null; + _block = null; + _scope = null; } Route get route => _viewRoute; diff --git a/test/core_dom/block_spec.dart b/test/core_dom/block_spec.dart index 3c7559fc4..fefa1f299 100644 --- a/test/core_dom/block_spec.dart +++ b/test/core_dom/block_spec.dart @@ -75,7 +75,8 @@ main() { beforeEach(inject((Injector injector, Profiler perf) { $rootElement.html(''); - anchor = new BlockHole($rootElement.contents().eq(0)); + anchor = new BlockHole($rootElement.contents().eq(0)[0], + injector.get(NgAnimate)); a = (new BlockFactory($('Aa'), [], perf, expando))(injector); b = (new BlockFactory($('Bb'), [], perf, expando))(injector); })); @@ -83,76 +84,50 @@ main() { describe('insertAfter', () { it('should insert block after anchor block', () { - a.insertAfter(anchor); + anchor.insert(a); expect($rootElement.html()).toEqual('Aa'); - expect(anchor.next).toBe(a); - expect(anchor.previous).toBe(null); - expect(a.next).toBe(null); - expect(a.previous).toBe(anchor); }); it('should insert multi element block after another multi element block', () { - b.insertAfter(a.insertAfter(anchor)); + anchor.insert(a); + anchor.insert(b, insertAfter: a); expect($rootElement.html()).toEqual('AaBb'); - expect(anchor.next).toBe(a); - expect(anchor.previous).toBe(null); - expect(a.next).toBe(b); - expect(a.previous).toBe(anchor); - expect(b.next).toBe(null); - expect(b.previous).toBe(a); }); it('should insert multi element block before another multi element block', () { - b.insertAfter(anchor); - a.insertAfter(anchor); + anchor.insert(b); + anchor.insert(a); expect($rootElement.html()).toEqual('AaBb'); - expect(anchor.next).toBe(a); - expect(anchor.previous).toBe(null); - expect(a.next).toBe(b); - expect(a.previous).toBe(anchor); - expect(b.next).toBe(null); - expect(b.previous).toBe(a); }); }); describe('remove', () { beforeEach(() { - b.insertAfter(a.insertAfter(anchor)); + anchor.insert(a); + anchor.insert(b, insertAfter: a); expect($rootElement.text()).toEqual('AaBb'); }); it('should remove the last block', () { - b.remove(); + anchor.remove(b); expect($rootElement.html()).toEqual('Aa'); - expect(anchor.next).toBe(a); - expect(anchor.previous).toBe(null); - expect(a.next).toBe(null); - expect(a.previous).toBe(anchor); - expect(b.next).toBe(null); - expect(b.previous).toBe(null); }); it('should remove child blocks from parent pseudo black', () { - a.remove(); + anchor.remove(a); expect($rootElement.html()).toEqual('Bb'); - expect(anchor.next).toBe(b); - expect(anchor.previous).toBe(null); - expect(a.next).toBe(null); - expect(a.previous).toBe(null); - expect(b.next).toBe(null); - expect(b.previous).toBe(anchor); }); it('should remove', inject((Logger logger, Injector injector, Profiler perf) { - a.remove(); - b.remove(); + anchor.remove(a); + anchor.remove(b); // TODO(dart): I really want to do this: // class Directive { @@ -172,21 +147,21 @@ main() { perf, new Expando()); - var outterBlock = outerBlockType(injector); + var outerBlock = outerBlockType(injector); // The LoggerBlockDirective caused a BlockHole for innerBlockType to // be created at logger[0]; - BlockHole outterAnchor = logger[0]; + BlockHole outerAnchor = logger[0]; BoundBlockFactory outterBoundBlockFactory = logger[1]; - outterBlock.insertAfter(anchor); + anchor.insert(outerBlock); // outterAnchor is a BlockHole, but it has "elements" set to the 0th element // of outerBlockType. So, calling insertAfter() will insert the new // block after the element. - outterBoundBlockFactory(null).insertAfter(outterAnchor); + outerAnchor.insert(outterBoundBlockFactory(null)); expect($rootElement.text()).toEqual('text'); - outterBlock.remove(); + anchor.remove(outerBlock); expect($rootElement.text()).toEqual(''); })); @@ -195,33 +170,16 @@ main() { describe('moveAfter', () { beforeEach(() { - b.insertAfter(a.insertAfter(anchor)); + anchor.insert(a); + anchor.insert(b, insertAfter: a); expect($rootElement.text()).toEqual('AaBb'); }); it('should move last to middle', () { - b.moveAfter(anchor); + anchor.move(a, moveAfter: b); expect($rootElement.html()).toEqual('BbAa'); - expect(anchor.next).toBe(b); - expect(anchor.previous).toBe(null); - expect(a.next).toBe(null); - expect(a.previous).toBe(b); - expect(b.next).toBe(a); - expect(b.previous).toBe(anchor); - }); - - - it('should move middle to last', () { - a.moveAfter(b); - expect($rootElement.html()).toEqual('BbAa'); - expect(anchor.next).toBe(b); - expect(anchor.previous).toBe(null); - expect(a.next).toBe(null); - expect(a.previous).toBe(b); - expect(b.next).toBe(a); - expect(b.previous).toBe(anchor); }); }); }); diff --git a/test/core_dom/compiler_spec.dart b/test/core_dom/compiler_spec.dart index f070ba4e4..20a623d32 100644 --- a/test/core_dom/compiler_spec.dart +++ b/test/core_dom/compiler_spec.dart @@ -102,7 +102,7 @@ void main() { rootScope.context['name'] = 'OK'; var block = template(injector, element); - element = $(block.elements); + element = $(block.nodes); rootScope.apply(); expect(element.text()).toEqual('OK!'); @@ -655,7 +655,7 @@ class SimpleTranscludeInAttachAttrDirective { SimpleTranscludeInAttachAttrDirective(BlockHole blockHole, BoundBlockFactory boundBlockFactory, Logger log, RootScope scope) { scope.runAsync(() { var block = boundBlockFactory(scope); - block.insertAfter(blockHole); + blockHole.insert(block); log('SimpleTransclude'); }); } diff --git a/test/core_dom/ng_mustache_spec.dart b/test/core_dom/ng_mustache_spec.dart index 6a2c1dffc..25c538512 100644 --- a/test/core_dom/ng_mustache_spec.dart +++ b/test/core_dom/ng_mustache_spec.dart @@ -17,7 +17,7 @@ main() { rootScope.context['name'] = 'OK'; var block = template(injector); - element = $(block.elements); + element = $(block.nodes); rootScope.apply(); expect(element.text()).toEqual('OK!'); @@ -32,7 +32,7 @@ main() { rootScope.context['age'] = 23; var block = template(injector); - element = $(block.elements); + element = $(block.nodes); rootScope.apply(); expect(element.attr('some-attr')).toEqual('OK'); @@ -48,7 +48,7 @@ main() { rootScope.context['line2'] = 'L2'; var block = template(injector); - element = $(block.elements); + element = $(block.nodes); rootScope.apply(); expect(element.attr('multiline-attr')).toEqual('line1: L1\nline2: L2'); @@ -61,7 +61,7 @@ main() { var block = template(injector); rootScope.apply(); - element = $(block.elements); + element = $(block.nodes); expect(element.html()).toEqual('Hello, World!'); })); diff --git a/test/directive/ng_if_spec.dart b/test/directive/ng_if_spec.dart index ec2a92f51..057bc1480 100644 --- a/test/directive/ng_if_spec.dart +++ b/test/directive/ng_if_spec.dart @@ -10,7 +10,7 @@ class ChildController { BlockHole blockHole, Scope scope) { scope.context['setBy'] = 'childController'; - boundBlockFactory(scope).insertAfter(blockHole); + blockHole.insert(boundBlockFactory(scope)); } }