From fec60a5ea9642ec4a35a43efed6ab7acab55f58d Mon Sep 17 00:00:00 2001 From: David Greenspan Date: Fri, 10 Aug 2012 16:37:47 -0700 Subject: [PATCH] Unify `template` obj and template callback `this`. The `template` arg in `eventHandler(event, template)` and the `this` in all three of {create,render,destroy} are now an object with methods `find`,`findAll` assigned at creation time (closures) and properties `firstNode`, `lastNode`, and `data` (the handlebars data) set upon each callback. No other props are set and the app is free to scribble on this object. Only subtlety is that we can't support find/findAll/firstNode/lastNode in create/destroy callback. In this case, find/findAll throw an error and firstNode/lastNode are reliably null. Added landmark.hasDom(). --- .../landmark-demo/client/landmark-demo.js | 13 ++--- packages/spark/spark.js | 9 +++- packages/spark/spark_tests.js | 1 - packages/templating/deftemplate.js | 48 +++++++++++-------- packages/templating/templating_tests.js | 33 ++++++++++--- 5 files changed, 65 insertions(+), 39 deletions(-) diff --git a/examples/landmark-demo/client/landmark-demo.js b/examples/landmark-demo/client/landmark-demo.js index 7c8a4c7edd..f04d33cfb5 100644 --- a/examples/landmark-demo/client/landmark-demo.js +++ b/examples/landmark-demo/client/landmark-demo.js @@ -214,18 +214,11 @@ Template.circles.disabled = function () { '' : 'disabled="disabled"'; }; -Template.circles.render = function (template) { +Template.circles.render = function () { var self = this; - self.node = template.find("svg"); + self.node = self.find("svg"); - // XXX you really want to have template.data here. anything else is - // obnoxious. it's plenty well defined and it's useful. - var data = Spark.getDataContext(template._range.firstNode()); - - // XXX why doesn't this work? - // var data = Spark.getDataContext(template.find("svg")); - // this does: - // Spark.getDataContext(template.find("svg").parentNode) + var data = self.data; if (! self.handle) { // XXX template.firstRender would be handy here diff --git a/packages/spark/spark.js b/packages/spark/spark.js index 77e23859b1..a2499e54f6 100644 --- a/packages/spark/spark.js +++ b/packages/spark/spark.js @@ -29,6 +29,8 @@ // timer' button again. the problem is almost certainly in atFlushTime // (not hard to see what it is.) +// XXX test hasDom(). Test `template` object more. + (function() { Spark = {}; @@ -958,6 +960,9 @@ _.extend(Spark.Landmark.prototype, { var r = this._range; return DomUtils.findAllClipped(r.containerNode(), selector, r.firstNode(), r.lastNode()); + }, + hasDom: function () { + return !! this._range; } }); @@ -1036,8 +1041,10 @@ Spark.createLandmark = function (options, htmlFunc) { destroyCallback: options.destroy || function () {}, landmark: landmark, finalize: function () { - if (! this.superceded) + if (! this.superceded) { + this.landmark._range = null; this.destroyCallback.call(this.landmark); + } } }); diff --git a/packages/spark/spark_tests.js b/packages/spark/spark_tests.js index 6291b92726..f43a8cedb0 100644 --- a/packages/spark/spark_tests.js +++ b/packages/spark/spark_tests.js @@ -1223,7 +1223,6 @@ Tinytest.add("spark - labeled landmarks", function (test) { 5, function () { return "hi" + dep(9);} );});});});});});});})); }); - console.log(html); return html; })); diff --git a/packages/templating/deftemplate.js b/packages/templating/deftemplate.js index c22062b35b..aeaa638869 100644 --- a/packages/templating/deftemplate.js +++ b/packages/templating/deftemplate.js @@ -29,13 +29,15 @@ _.extend(Handlebars._default_helpers, { isolate: function (options) { + var data = this; return Spark.isolate(function () { - return options.fn(this); + return options.fn(data); }); }, constant: function (options) { + var data = this; return Spark.createLandmark({ constant: true }, function () { - return options.fn(this); + return options.fn(data); }); } }); @@ -46,17 +48,23 @@ var templateInstanceData = {}; var templateObjFromLandmark = function (landmark) { - var template = { - find: function (selector) { - return landmark.find(selector); - }, - findAll: function (selector) { - return landmark.findAll(selector); - }, - firstNode: landmark.firstNode(), - lastNode: landmark.lastNode(), - data: templateInstanceData[landmark.id] - }; + var template = templateInstanceData[landmark.id] || ( + templateInstanceData[landmark.id] = { + // set these once + find: function (selector) { + if (! landmark.hasDom()) + throw new Error("Template not in DOM"); + return landmark.find(selector); + }, + findAll: function (selector) { + if (! landmark.hasDom()) + throw new Error("Template not in DOM"); + return landmark.findAll(selector); + } + }); + // set these each time + template.firstNode = landmark.hasDom() ? landmark.firstNode() : null; + template.lastNode = landmark.hasDom() ? landmark.lastNode() : null; return template; }; @@ -73,18 +81,18 @@ var html = Spark.createLandmark({ preserve: tmpl.preserve || {}, create: function () { - templateInstanceData[this.id] = {}; - tmpl.create && - tmpl.create.call(templateInstanceData[this.id]); + var template = templateObjFromLandmark(this); + template.data = data; + tmpl.create && tmpl.create.call(template); }, render: function () { - tmpl.render && - tmpl.render.call(templateInstanceData[this.id], - templateObjFromLandmark(this)); + var template = templateObjFromLandmark(this); + template.data = data; + tmpl.render && tmpl.render.call(template); }, destroy: function () { tmpl.destroy && - tmpl.destroy.call(templateInstanceData[this.id]); + tmpl.destroy.call(templateObjFromLandmark(this)); delete templateInstanceData[this.id]; } }, function (landmark) { diff --git a/packages/templating/templating_tests.js b/packages/templating/templating_tests.js index ceac3d94e6..47fa456ed5 100644 --- a/packages/templating/templating_tests.js +++ b/packages/templating/templating_tests.js @@ -502,9 +502,9 @@ Tinytest.add("templating - matching in list", function (test) { var buf = []; _.extend(Template.test_listmatching_a1, { create: function () { buf.push('+'); }, - render: function (template) { - var letter = DomUtils.rangeToHtml(template.firstNode, - template.lastNode).match(/\S+/)[0]; + render: function () { + var letter = DomUtils.rangeToHtml(this.firstNode, + this.lastNode).match(/\S+/)[0]; buf.push('*'+letter); }, destroy: function () { buf.push('-'); } @@ -577,19 +577,38 @@ Tinytest.add("templating - template arg", function (test) { template.find('i').innerHTML = (template.findAll('*').length)+"-element"; template.lastNode.innerHTML += ' (the secret is '+ - template.data.secret+')'; + template.secret+')'; } }; - Template.test_template_arg_a.render = function (template) { + Template.test_template_arg_a.create = function() { + var self = this; + test.isFalse(self.firstNode); + test.isFalse(self.lastNode); + test.throws(function () { return self.find("*"); }); + test.throws(function () { return self.findAll("*"); }); + }; + + Template.test_template_arg_a.render = function () { + var template = this; template.firstNode.innerHTML = 'Greetings'; template.lastNode.innerHTML = 'Line'; template.find('i').innerHTML = (template.findAll('b').length)+"-bold"; - template.data.secret = "strawberry pie"; + template.secret = "strawberry "+template.data.food; }; - var div = OnscreenDiv(Spark.render(Template.test_template_arg_a)); + Template.test_template_arg_a.destroy = function() { + var self = this; + test.isFalse(self.firstNode); + test.isFalse(self.lastNode); + test.throws(function () { return self.find("*"); }); + test.throws(function () { return self.findAll("*"); }); + }; + + var div = OnscreenDiv(Spark.render(function () { + return Template.test_template_arg_a({food: "pie"}); + })); test.equal(div.text(), "Foo Bar Baz"); Meteor.flush();