From 14ff309ba0c52729f9741afd8e04fa4bf9270417 Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Wed, 3 Jun 2015 07:10:30 -0400 Subject: [PATCH 1/2] Make `childViews` recompute after rendering. --- .../ember-htmlbars/lib/morphs/attr-morph.js | 2 +- packages/ember-htmlbars/lib/morphs/morph.js | 1 + .../lib/node-managers/view-node-manager.js | 2 - .../ember-htmlbars/tests/helpers/each_test.js | 4 +- .../tests/helpers/input_test.js | 2 +- .../will-destroy-element-hook-test.js | 1 - packages/ember-metal-views/lib/renderer.js | 5 ++ .../lib/mixins/legacy_view_support.js | 4 +- .../lib/mixins/view_child_views_support.js | 50 ++++++++++++++----- .../ember-views/lib/views/container_view.js | 5 +- .../ember-views/lib/views/states/in_dom.js | 15 ------ packages/ember-views/lib/views/view.js | 2 +- 12 files changed, 54 insertions(+), 39 deletions(-) diff --git a/packages/ember-htmlbars/lib/morphs/attr-morph.js b/packages/ember-htmlbars/lib/morphs/attr-morph.js index cf0e5989b3b..23103b85dc6 100644 --- a/packages/ember-htmlbars/lib/morphs/attr-morph.js +++ b/packages/ember-htmlbars/lib/morphs/attr-morph.js @@ -12,8 +12,8 @@ export var styleWarning = '' + function EmberAttrMorph(element, attrName, domHelper, namespace) { HTMLBarsAttrMorph.call(this, element, attrName, domHelper, namespace); - this.streamUnsubscribers = null; + this.isAttrMorph = true; } var proto = EmberAttrMorph.prototype = o_create(HTMLBarsAttrMorph.prototype); diff --git a/packages/ember-htmlbars/lib/morphs/morph.js b/packages/ember-htmlbars/lib/morphs/morph.js index 459773f2889..ebca1f7da13 100644 --- a/packages/ember-htmlbars/lib/morphs/morph.js +++ b/packages/ember-htmlbars/lib/morphs/morph.js @@ -7,6 +7,7 @@ let guid = 1; function EmberMorph(DOMHelper, contextualElement) { this.HTMLBarsMorph$constructor(DOMHelper, contextualElement); + this.isElementMorph = true; this.emberView = null; this.emberToDestroy = null; this.streamUnsubscribers = null; diff --git a/packages/ember-htmlbars/lib/node-managers/view-node-manager.js b/packages/ember-htmlbars/lib/node-managers/view-node-manager.js index 55bc10a3499..f6dcdf4d301 100644 --- a/packages/ember-htmlbars/lib/node-managers/view-node-manager.js +++ b/packages/ember-htmlbars/lib/node-managers/view-node-manager.js @@ -70,8 +70,6 @@ ViewNodeManager.create = function(renderNode, env, attrs, found, parentView, pat } else { componentInfo.layout = getTemplate(component) || componentInfo.layout; } - - renderNode.emberView = component; } Ember.assert("BUG: ViewNodeManager.create can take a scope or a self, but not both", !(contentScope && found.self)); diff --git a/packages/ember-htmlbars/tests/helpers/each_test.js b/packages/ember-htmlbars/tests/helpers/each_test.js index a12713e1fc7..9d901820159 100644 --- a/packages/ember-htmlbars/tests/helpers/each_test.js +++ b/packages/ember-htmlbars/tests/helpers/each_test.js @@ -378,7 +378,7 @@ QUnit.test("it supports itemController", function() { assertText(view, "controller:Trek Glowackicontroller:Geoffrey Grosenbach"); - strictEqual(view.childViews[0].get('_arrayController.target'), parentController, "the target property of the child controllers are set correctly"); + strictEqual(get(view, 'childViews')[0].get('_arrayController.target'), parentController, "the target property of the child controllers are set correctly"); }); QUnit.test("itemController should not affect the DOM structure", function() { @@ -1044,7 +1044,7 @@ function testEachWithItem(moduleName, useBlockParams) { assertText(view, "controller:parentController - controller:Trek Glowacki - controller:parentController - controller:Geoffrey Grosenbach - "); - strictEqual(view.childViews[0].get('_arrayController.target'), parentController, "the target property of the child controllers are set correctly"); + strictEqual(get(view, 'childViews')[0].get('_arrayController.target'), parentController, "the target property of the child controllers are set correctly"); }); QUnit.test("itemController specified in ArrayController with name binding does not change context", function() { diff --git a/packages/ember-htmlbars/tests/helpers/input_test.js b/packages/ember-htmlbars/tests/helpers/input_test.js index 50f735a34e5..61d214ead24 100644 --- a/packages/ember-htmlbars/tests/helpers/input_test.js +++ b/packages/ember-htmlbars/tests/helpers/input_test.js @@ -116,7 +116,7 @@ QUnit.test("cursor position is not lost when updating content", function() { // set the cursor position to 3 (no selection) run(function() { input.value = 'derp'; - view.childViews[0]._elementValueDidChange(); + view.get('childViews')[0]._elementValueDidChange(); input.selectionStart = 3; input.selectionEnd = 3; }); diff --git a/packages/ember-htmlbars/tests/integration/will-destroy-element-hook-test.js b/packages/ember-htmlbars/tests/integration/will-destroy-element-hook-test.js index 987f17fd0f3..a7c68af4e0d 100644 --- a/packages/ember-htmlbars/tests/integration/will-destroy-element-hook-test.js +++ b/packages/ember-htmlbars/tests/integration/will-destroy-element-hook-test.js @@ -44,7 +44,6 @@ QUnit.test('willDestroyElement is only called once when a component leaves scope runAppend(component); assert.equal(component.$().text(), 'Truthy', 'precond - truthy template is displayed'); - assert.equal(component.get('childViews.length'), 1); run(function() { set(component, 'switch', false); diff --git a/packages/ember-metal-views/lib/renderer.js b/packages/ember-metal-views/lib/renderer.js index c29a7976e07..5b46833570f 100755 --- a/packages/ember-metal-views/lib/renderer.js +++ b/packages/ember-metal-views/lib/renderer.js @@ -75,6 +75,7 @@ Renderer.prototype.dispatchLifecycleHooks = } this.didRender(hook.view); + this.internalDidRender(hook.view); } ownerView._dispatching = null; @@ -159,6 +160,10 @@ Renderer.prototype.didRender = function (view) { if (view.trigger) { view.trigger('didRender'); } }; +Renderer.prototype.internalDidRender = function (view) { + if (view.trigger) { view.trigger('_internalDidRender'); } +}; + Renderer.prototype.updateAttrs = function (view, attrs) { this.setAttrs(view, attrs); }; // setting new attrs diff --git a/packages/ember-views/lib/mixins/legacy_view_support.js b/packages/ember-views/lib/mixins/legacy_view_support.js index 06254c37a7a..7305d269fe2 100644 --- a/packages/ember-views/lib/mixins/legacy_view_support.js +++ b/packages/ember-views/lib/mixins/legacy_view_support.js @@ -17,12 +17,12 @@ var LegacyViewSupport = Mixin.create({ afterRender(buffer) {}, walkChildViews(callback) { - var childViews = this.childViews.slice(); + var childViews = get(this, 'childViews').slice(); while (childViews.length) { var view = childViews.pop(); callback(view); - childViews.push(...view.childViews); + childViews.push(...get(view, 'childViews')); } }, diff --git a/packages/ember-views/lib/mixins/view_child_views_support.js b/packages/ember-views/lib/mixins/view_child_views_support.js index efca238b67c..c31705d6a37 100644 --- a/packages/ember-views/lib/mixins/view_child_views_support.js +++ b/packages/ember-views/lib/mixins/view_child_views_support.js @@ -8,8 +8,7 @@ import { removeObject } from "ember-metal/enumerable_utils"; import { get } from "ember-metal/property_get"; import { set } from "ember-metal/property_set"; import setProperties from "ember-metal/set_properties"; - -var EMPTY_ARRAY = []; +import { computed } from "ember-metal/computed"; var ViewChildViewsSupport = Mixin.create({ /** @@ -21,20 +20,44 @@ var ViewChildViewsSupport = Mixin.create({ @default [] @private */ - childViews: EMPTY_ARRAY, + childViews: computed(function() { + + if (!this._renderNode || !this._renderNode.childNodes) { + return Ember.A(); + } + + let currentElementMorph, index, length; + let childNodes = this._renderNode.childNodes; + + for (index = 0, length = this._renderNode.childNodes.length; index < length; index++) { + let node = childNodes[index]; + if (node.isElementMorph) { + currentElementMorph = node; + break; + } + } + + var childViews = []; + if (currentElementMorph.childNodes) { + for (index = 0, length = currentElementMorph.childNodes.length; index < length; index++) { + let node = currentElementMorph.childNodes[index]; + + if (node.isElementMorph && node.emberView) { + childViews.push(node.emberView); + } + } + } + + return Ember.A(childViews); + }), init() { this._super(...arguments); - - // setup child views. be sure to clone the child views array first - // 2.0TODO: Remove Ember.A() here - this.childViews = Ember.A(this.childViews.slice()); this.ownerView = this; }, appendChild(view) { this.linkChild(view); - this.childViews.push(view); }, destroyChild(view) { @@ -58,11 +81,6 @@ var ViewChildViewsSupport = Mixin.create({ // update parent node this.unlinkChild(view); - // remove view from childViews array. - var childViews = get(this, 'childViews'); - - removeObject(childViews, view); - return this; }, @@ -131,6 +149,12 @@ var ViewChildViewsSupport = Mixin.create({ unlinkChild(instance) { set(instance, 'parentView', null); instance.trigger('parentViewDidChange'); + }, + + _internalDidRender() { + this._super(...arguments); + + this.notifyPropertyChange('childViews'); } }); diff --git a/packages/ember-views/lib/views/container_view.js b/packages/ember-views/lib/views/container_view.js index a2d376091ae..7b8c7780c95 100644 --- a/packages/ember-views/lib/views/container_view.js +++ b/packages/ember-views/lib/views/container_view.js @@ -300,7 +300,10 @@ var ContainerView = View.extend(MutableArray, { this.renderer.didDestroyElement(childViews[i]); } } - }) + }), + + // override default hook which notifies `childViews` property + _internalDidRender() { } }); export default ContainerView; diff --git a/packages/ember-views/lib/views/states/in_dom.js b/packages/ember-views/lib/views/states/in_dom.js index 47a6bcdd9d0..dfdc4947eca 100644 --- a/packages/ember-views/lib/views/states/in_dom.js +++ b/packages/ember-views/lib/views/states/in_dom.js @@ -28,22 +28,7 @@ merge(inDOM, { exit(view) { view._unregister(); - }, - - appendAttr(view, attrNode) { - var childViews = view.childViews; - - if (!childViews.length) { childViews = view.childViews = childViews.slice(); } - childViews.push(attrNode); - - attrNode.parentView = view; - view.renderer.appendAttrTo(attrNode, view.element, attrNode.attrName); - - view.propertyDidChange('childViews'); - - return attrNode; } - }); export default inDOM; diff --git a/packages/ember-views/lib/views/view.js b/packages/ember-views/lib/views/view.js index 08958ffc932..92887861cba 100644 --- a/packages/ember-views/lib/views/view.js +++ b/packages/ember-views/lib/views/view.js @@ -924,7 +924,7 @@ var View = CoreView.extend( }, forEachChildView(callback) { - var childViews = this.childViews; + var childViews = get(this, 'childViews'); if (!childViews) { return this; } From 785f9c5dae7a2c401c715912cb64ba030ab719f2 Mon Sep 17 00:00:00 2001 From: Kris Selden Date: Fri, 12 Jun 2015 02:00:24 -0700 Subject: [PATCH 2/2] Cleanup `childViews` as computed. * Do not return an `Ember.A` * Reimplement `childViews` for `ContainerView` --- .../tests/helpers/collection_test.js | 4 ++-- .../ember-htmlbars/tests/helpers/view_test.js | 8 ++++---- .../lib/mixins/view_child_views_support.js | 4 +--- .../ember-views/lib/views/container_view.js | 19 ++++++++++++++++--- .../ember-views/tests/views/select_test.js | 2 +- .../tests/views/view/destroy_element_test.js | 2 +- .../tests/views/view/element_test.js | 2 +- .../tests/views/view/is_visible_test.js | 5 ++--- .../tests/views/view/remove_test.js | 6 +++--- 9 files changed, 31 insertions(+), 21 deletions(-) diff --git a/packages/ember-htmlbars/tests/helpers/collection_test.js b/packages/ember-htmlbars/tests/helpers/collection_test.js index 57f7a00b689..c05d74fa659 100644 --- a/packages/ember-htmlbars/tests/helpers/collection_test.js +++ b/packages/ember-htmlbars/tests/helpers/collection_test.js @@ -24,13 +24,13 @@ var TemplateTests, registry, container, lookup; function nthChild(view, nth) { - return get(view, 'childViews').objectAt(nth || 0); + return get(view, 'childViews')[nth || 0]; } var firstChild = nthChild; function firstGrandchild(view) { - return get(get(view, 'childViews').objectAt(0), 'childViews').objectAt(0); + return get(get(view, 'childViews')[0], 'childViews')[0]; } QUnit.module("collection helper", { diff --git a/packages/ember-htmlbars/tests/helpers/view_test.js b/packages/ember-htmlbars/tests/helpers/view_test.js index 77523bcc2b5..86bd1bce3ef 100644 --- a/packages/ember-htmlbars/tests/helpers/view_test.js +++ b/packages/ember-htmlbars/tests/helpers/view_test.js @@ -27,11 +27,11 @@ var view, originalLookup, registry, container, lookup; var trim = jQuery.trim; function firstGrandchild(view) { - return get(get(view, 'childViews').objectAt(0), 'childViews').objectAt(0); + return get(get(view, 'childViews')[0], 'childViews')[0]; } function nthChild(view, nth) { - return get(view, 'childViews').objectAt(nth || 0); + return get(view, 'childViews')[nth || 0]; } function viewClass(options) { @@ -715,7 +715,7 @@ QUnit.test('Template views add an elementId to child views created using the vie runAppend(view); - var childView = get(view, 'childViews.firstObject'); + var childView = get(view, 'childViews')[0]; equal(view.$().children().first().children().first().attr('id'), get(childView, 'elementId')); }); @@ -1157,7 +1157,7 @@ QUnit.test('should expose a controller keyword that persists through Ember.Conta runAppend(view); - var containerView = get(view, 'childViews.firstObject'); + var containerView = get(view, 'childViews')[0]; var viewInstanceToBeInserted = EmberView.create({ template: compile('{{controller.foo}}') }); diff --git a/packages/ember-views/lib/mixins/view_child_views_support.js b/packages/ember-views/lib/mixins/view_child_views_support.js index c31705d6a37..415efc715b8 100644 --- a/packages/ember-views/lib/mixins/view_child_views_support.js +++ b/packages/ember-views/lib/mixins/view_child_views_support.js @@ -4,8 +4,6 @@ */ import Ember from 'ember-metal/core'; import { Mixin } from "ember-metal/mixin"; -import { removeObject } from "ember-metal/enumerable_utils"; -import { get } from "ember-metal/property_get"; import { set } from "ember-metal/property_set"; import setProperties from "ember-metal/set_properties"; import { computed } from "ember-metal/computed"; @@ -48,7 +46,7 @@ var ViewChildViewsSupport = Mixin.create({ } } - return Ember.A(childViews); + return childViews; }), init() { diff --git a/packages/ember-views/lib/views/container_view.js b/packages/ember-views/lib/views/container_view.js index 7b8c7780c95..f533889bf90 100644 --- a/packages/ember-views/lib/views/container_view.js +++ b/packages/ember-views/lib/views/container_view.js @@ -10,6 +10,7 @@ import { beforeObserver } from "ember-metal/mixin"; import { on } from "ember-metal/events"; +import { indexOf } from "ember-metal/array"; import containerViewTemplate from "ember-htmlbars/templates/container-view"; containerViewTemplate.meta.revision = 'Ember@VERSION_STRING_PLACEHOLDER'; @@ -178,6 +179,8 @@ var ContainerView = View.extend(MutableArray, { ); }, + childViews: [], + init() { this._super(...arguments); @@ -187,7 +190,7 @@ var ContainerView = View.extend(MutableArray, { // redefine view's childViews property that was obliterated // 2.0TODO: Don't Ember.A() this so users disabling prototype extensions // don't pay a penalty. - var childViews = this.childViews = Ember.A([]); + var childViews = this.childViews = []; forEach(userChildViews, function(viewName, idx) { var view; @@ -205,7 +208,7 @@ var ContainerView = View.extend(MutableArray, { var currentView = get(this, 'currentView'); if (currentView) { - if (!childViews.length) { childViews = this.childViews = Ember.A(this.childViews.slice()); } + if (!childViews.length) { childViews = this.childViews = this.childViews.slice(); } childViews.push(this.createChildView(currentView)); } @@ -242,9 +245,19 @@ var ContainerView = View.extend(MutableArray, { layout: containerViewTemplate, + removeChild(childView) { + if (!this.isDestroyed) { + var idx = indexOf.call(this.childViews, childView); + if (idx !== -1) { + this.replace(idx, 1); + } + } + return this._super(childView); + }, + replace(idx, removedCount, addedViews=[]) { var addedCount = get(addedViews, 'length'); - var childViews = get(this, 'childViews'); + var childViews = this.childViews; Ember.assert("You can't add a child to a container - the child is already a child of another view", () => { for (var i=0, l=addedViews.length; i