From 1038da20a9055c90e487ac7fa1ee9169d0572f96 Mon Sep 17 00:00:00 2001 From: Gadi Cohen Date: Sat, 6 Jun 2015 20:49:12 +0200 Subject: [PATCH] 0.5.2 fix, more flexible component instantiation; TODO reuse dom stuff --- HISTORY.md | 4 +++ lib/meteorFamousView.js | 14 ++++++++- lib/utilities.js | 7 +++++ lib/wrappers/Components.js | 26 +++++++++++++---- lib/wrappers/Components/DOMElement.js | 41 ++++++++++++++++++++------- lib/wrappers/Nodes/Scene.js | 16 +++-------- package.js | 2 +- tests/famousEach.js | 2 +- tests/lib/prepare.js | 14 ++++----- tests/wrappers/DOMElement.js | 10 +++---- tests/wrappers/Node.js | 6 ++-- 11 files changed, 96 insertions(+), 46 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index ded00e9..44b1958 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -1,5 +1,9 @@ ## vNEXT +* Fix for Famous 0.5.2 (and further work on DOMElement handling) +* Initial child cleanup handling +* Support components that don't add themselves to the node + set `_id` + * Fix wrapper with no args' renderFunc using `with` data as args * famousEach, in a totally new and extensible way * Some internal changes in how nodes are dismounted diff --git a/lib/meteorFamousView.js b/lib/meteorFamousView.js index e770e53..0fd2b40 100644 --- a/lib/meteorFamousView.js +++ b/lib/meteorFamousView.js @@ -33,11 +33,23 @@ MeteorFamousView = FView._MeteorFamousView = * @method */ MeteorFamousView.prototype.destroy = function() { + var fview = this; delete fviews[this.id]; log.debug("Destroying " + this.type + " (#" + this.id + ") from " + this._source); - // TODO children, etc. + // TODO, tests + _.each(this.children, function(child) { + //child.destroy(); + Blaze.remove(child.blazeView); + }); + + // components + // TODO, tests + // TODO, is this the right place for this? + _.each(this.components, function(compData) { + compData.fvClass.destroy.call(compData, fview); + }); // remove from parent if (this.parent) diff --git a/lib/utilities.js b/lib/utilities.js index 4100f50..c7e0776 100644 --- a/lib/utilities.js +++ b/lib/utilities.js @@ -162,3 +162,10 @@ getElementFromDOMElement = function(node, callback) { // except for this :( clock.setTimeout(query, 64); }; + +block = function(s) { + var ms = s * 1000; + var start = performance.now(); + console.log('blocking for ' + s + 's'); + while (performance.now() - start < ms); +} diff --git a/lib/wrappers/Components.js b/lib/wrappers/Components.js index 5c3f994..5a579c6 100644 --- a/lib/wrappers/Components.js +++ b/lib/wrappers/Components.js @@ -3,8 +3,11 @@ var ComponentClass = { famousClass: null, newComponentInstance: function(fview) { - var fvClass = this; - return new fvClass.famousClass(fview.node); + //var fvClass = this; + var comp = new this.famousClass(fview.node); + if (!(comp._id || comp.id)) + comp._id = fview.node.addComponent(comp); + return comp; }, addChild: function(child) { @@ -73,12 +76,25 @@ var ComponentClass = { instance: fvClass.newComponentInstance.call(fvClass, fview) }; - fview[fvClass.shortcutName] = fview.components[fvClass.name].instance; + if (fvClass.shortcutName) + fview[fvClass.shortcutName] = fview.components[fvClass.name].instance; + }, + + destroy: function(fview) { + // dont do this? some components rely on node unmount + //fview.node.removeComponent(this.instance); }, templateDestroyed: function() { - throw new Error("You tried to remove a Component via Blaze, but you " - + "but should remove it's parent node instead"); + // TODO, untested + var blazeView = this.view; + var fview = fviewParentFromBlazeView(blazeView); + var fvClass = blazeView.template._fviewClass; + var componentData = fview.components[fvClass.name]; + fvClass.destroy.call(componentData, fview); + +// throw new Error("You tried to remove a Component via Blaze, but you " +// + "but should remove it's parent node instead"); }, makeTemplate: function(fvClass) { diff --git a/lib/wrappers/Components/DOMElement.js b/lib/wrappers/Components/DOMElement.js index d052080..8f34987 100644 --- a/lib/wrappers/Components/DOMElement.js +++ b/lib/wrappers/Components/DOMElement.js @@ -4,7 +4,7 @@ FView.ready(function() { var renderedPaths = {}; FView.wrapComponent('DOMElement', famous.domRenderables.DOMElement, { - attrUpdate: function(key, value, oldValue, data, firstTime) { + attrUpdate: function(key, value, oldValue, data, firstTime, compData) { var fview = this; if (!fview._domElementData) { @@ -72,13 +72,27 @@ FView.ready(function() { } }, + destroy: function(fview) { + var domRenderer = fview._loadedDomRenderer(); + delete domRenderer._target.content._fview; + Blaze.remove(this.renderedView); + this.fvClass._super.destroy.apply(this, arguments); + }, + templateCreated: function() { var blazeView = this.view; var fview = fviewParentFromBlazeView(blazeView); var fvClass = blazeView.template._fviewClass; - //fview.domElement = new DOMElement(fview.node); - fview.domElement = fvClass.newComponentInstance.call(fvClass, fview); + fview.components.DOMElement = { + fvClass: fvClass, + blazeView: this, + instance: fvClass.newComponentInstance.call(fvClass, fview), + }; + + // shortcut + fview.domElement = fview.components.DOMElement.instance; + var path = fview.node.getId(); var context = path.split('/', 1)[0]; var domRenderer = FamousEngine.compositor.getOrSetContext(context).DOMRenderer; @@ -99,7 +113,7 @@ FView.ready(function() { }; fview.updateSizeDeferred = function() { - _.defer(function() { + FView.defer(function() { fview.updateSize(); }); }; @@ -122,15 +136,20 @@ FView.ready(function() { // Remove when merged: https://github.com/Famous/engine/pull/58 domRenderer.findChildren(); - domRenderer._target.content = document.createElement('div'); - domRenderer._target.content.classList.add('famous-dom-element-content'); - domRenderer._target.element.insertBefore( - domRenderer._target.content, - domRenderer._target.element.firstChild - ); + if (!domRenderer._target.content) { + domRenderer._target.content = document.createElement('div'); + domRenderer._target.content.classList.add('famous-dom-element-content'); + domRenderer._target.element.insertBefore( + domRenderer._target.content, + domRenderer._target.element.firstChild + ); + } + // make sure we don't rerender to an existing element domRenderer._target.content._fview = true; - Blaze.render(viewToRender, domRenderer._target.content, blazeView); + // Store renderedView ref for cleanup on distroy() + fview.components[fvClass.name].renderedView = + Blaze.render(viewToRender, domRenderer._target.content, blazeView); domRenderer.setSize( domRenderer._target.explicitWidth ? false : domRenderer._target.size[0], diff --git a/lib/wrappers/Nodes/Scene.js b/lib/wrappers/Nodes/Scene.js index 96c1b25..33f68be 100644 --- a/lib/wrappers/Nodes/Scene.js +++ b/lib/wrappers/Nodes/Scene.js @@ -19,7 +19,9 @@ FView.wrap('Scene', null, { }, dismount: function() { - this.node.dismount(); + // FamousEngine does not destroy scenes #182 + // https://github.com/Famous/engine/issues/182 + this._scene.dismount(); }, renderFunc: function() { @@ -91,15 +93,5 @@ FView.wrap('Scene', null, { log.error("No such helper for _onRender: " + data._onRender); delete data._onRender; } - }, - - templateDestroy: function() { - var fview = FView.from(this); - fview.destroy(); - - // FamousEngine does not destroy scenes #182 - // https://github.com/Famous/engine/issues/182 - fview._scene.dismount(); - }, - + } }); diff --git a/package.js b/package.js index 52d8811..e78fdf1 100644 --- a/package.js +++ b/package.js @@ -21,7 +21,7 @@ function common(api) { ], client) // Famous - api.use('gadicohen:famous@0.5.0_4', client); + api.use('gadicohen:famous@0.5.2', client); api.addFiles([ 'lib/famous-views.js', diff --git a/tests/famousEach.js b/tests/famousEach.js index 7554d4b..d0542a1 100644 --- a/tests/famousEach.js +++ b/tests/famousEach.js @@ -20,7 +20,7 @@ Template.famousEachTest.helpers({ Tinytest.add('famous-views - famousEach - setup', function(test) { // maintain order - Blaze.render(Template.famousEachTest, commonDiv); + Blaze.render(Template.famousEachTest, testDiv()); }); Tinytest.addAsync('famous-views - famousEach - addedAt append', function(test, complete) { diff --git a/tests/lib/prepare.js b/tests/lib/prepare.js index c3534a0..ae1aa80 100644 --- a/tests/lib/prepare.js +++ b/tests/lib/prepare.js @@ -1,8 +1,8 @@ -commonDiv = null; -Meteor.startup(function() { - commonDiv = document.createElement('div'); - commonDiv.style.display = 'none'; - document.body.appendChild(commonDiv); -}); +noop = function() {}; -noop = function() {}; \ No newline at end of file +testDiv = function() { + var div = document.createElement('div'); + div.style.display = 'none'; + document.body.appendChild(div); + return div; +} diff --git a/tests/wrappers/DOMElement.js b/tests/wrappers/DOMElement.js index 168477b..b7ff7d6 100644 --- a/tests/wrappers/DOMElement.js +++ b/tests/wrappers/DOMElement.js @@ -6,7 +6,7 @@ Tinytest.addAsync('famous-views - Wrappers - DOMElement - content', function(tes complete(); }); } - Blaze.render(Template.domEl1, commonDiv); + Blaze.render(Template.domEl1, testDiv()); }); Tinytest.addAsync('famous-views - Wrappers - DOMElement - content on repurposed el', function(test, complete) { @@ -30,7 +30,7 @@ Tinytest.addAsync('famous-views - Wrappers - DOMElement - content on repurposed Template.domEl2.helpers({ x: function() { return x.get(); } }); - Blaze.render(Template.domEl2, commonDiv); + Blaze.render(Template.domEl2, testDiv()); }); @@ -41,7 +41,7 @@ Tinytest.add('famous-views - Wrappers - DOMElement - attributes - classes', func getClasses: function() { return classes.get(); } }); - Blaze.render(Template.domEl_classes, commonDiv); + Blaze.render(Template.domEl_classes, testDiv()); Tracker.flush(); var fview = FView.byId("domEl_classes"); @@ -64,7 +64,7 @@ Tinytest.add('famous-views - Wrappers - DOMElement - attributes - style', functi reactiveStyle: function() { return style.get(); } }); - Blaze.render(Template.domEl_style, commonDiv); + Blaze.render(Template.domEl_style, testDiv()); Tracker.flush(); var DE = FView.byId("domEl_style").domElement; @@ -88,7 +88,7 @@ Tinytest.add('famous-views - Wrappers - DOMElement - attributes - other', functi getLang: function() { return lang.get(); } }); - Blaze.render(Template.domEl_attributes, commonDiv); + Blaze.render(Template.domEl_attributes, testDiv()); Tracker.flush(); var DE = FView.byId("domEl_attributes").domElement; diff --git a/tests/wrappers/Node.js b/tests/wrappers/Node.js index 33e8df5..ad8c753 100644 --- a/tests/wrappers/Node.js +++ b/tests/wrappers/Node.js @@ -66,7 +66,7 @@ Tinytest.addAsync('famous-views - Wrappers - Node - _onRender', function(test, c } }); - Blaze.render(Template.node2, commonDiv); + Blaze.render(Template.node2, testDiv()); }); // also tests: addToParent, addChild @@ -79,7 +79,7 @@ Tinytest.addAsync('famous-views - Wrappers - Node - template create', function(t test.equal(scene.node._children.indexOf(node.node), 0); complete(); }; - Blaze.render(Template.node3, commonDiv); + Blaze.render(Template.node3, testDiv()); }); // also tests: dismount @@ -100,5 +100,5 @@ Tinytest.addAsync('famous-views - Wrappers - Node - template destroy', function( }); x.set(false); }; - Blaze.render(Template.node4, commonDiv); + Blaze.render(Template.node4, testDiv()); });