From 91de0eff62dd371d148be9ae40712c28e60739fd Mon Sep 17 00:00:00 2001 From: David Greenspan Date: Fri, 20 Jun 2014 15:24:54 -0700 Subject: [PATCH] Fix more tests (and code) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Biggest fix is that event data was wrong; when binding “this” for an event handler, we didn’t have a pointer to the current template, just a pointer to the prototype (i.e. the object Template.foo itself), and we were using that. --- packages/blaze/component.js | 5 +++ packages/blaze/domrange.js | 7 +++-- packages/spacebars-tests/template_tests.js | 7 ++--- packages/spacebars-tests/templating_tests.js | 16 ---------- packages/ui/template.js | 32 +++++++++++--------- 5 files changed, 30 insertions(+), 37 deletions(-) diff --git a/packages/blaze/component.js b/packages/blaze/component.js index c1981e2d39..366ff14d24 100644 --- a/packages/blaze/component.js +++ b/packages/blaze/component.js @@ -205,3 +205,8 @@ Blaze.getElementDataVar = function (elem) { var theWith = Blaze.getElementControllerOfType(elem, Blaze.With); return theWith ? theWith.dataVar : null; }; + +Blaze.getComponentDataVar = function (comp) { + var theWith = Blaze.getParentControllerOfType(comp, Blaze.With); + return theWith ? theWith.dataVar : null; +}; diff --git a/packages/blaze/domrange.js b/packages/blaze/domrange.js index 3f0fada167..9a3018a53a 100644 --- a/packages/blaze/domrange.js +++ b/packages/blaze/domrange.js @@ -350,9 +350,10 @@ Blaze.DOMAugmenter = JSClass.create({ }); Blaze.EventAugmenter = Blaze.DOMAugmenter.extend({ - constructor: function (eventMap) { + constructor: function (eventMap, thisInHandler) { this.eventMap = eventMap; this.handles = []; + this.thisInHandler = thisInHandler; // optional }, attach: function (range, element) { var self = this; @@ -373,8 +374,8 @@ Blaze.EventAugmenter = Blaze.DOMAugmenter.extend({ element, newEvents, selector, function (evt) { if (! range.containsElement(evt.currentTarget)) - return; - return handler.apply(this, arguments); + return null; + return handler.apply(self.thisInHandler || this, arguments); }, range, function (r) { return r.parentRange; diff --git a/packages/spacebars-tests/template_tests.js b/packages/spacebars-tests/template_tests.js index 3e483ef396..b4651d4cb7 100644 --- a/packages/spacebars-tests/template_tests.js +++ b/packages/spacebars-tests/template_tests.js @@ -1240,7 +1240,7 @@ Tinytest.add("spacebars-tests - template_tests - inclusion lookup order", functi // test that {{> foo}} looks for a helper named 'foo', then a // template named 'foo', then a 'foo' field in the data context. var tmpl = Template.spacebars_template_test_inclusion_lookup; - tmpl.data = function () { + var tmplData = function () { return { // shouldn't have an effect since we define a helper with the // same name. @@ -1253,7 +1253,7 @@ Tinytest.add("spacebars-tests - template_tests - inclusion lookup order", functi tmpl.spacebars_template_test_inclusion_lookup_subtmpl = Template.spacebars_template_test_inclusion_lookup_subtmpl2; - test.equal(canonicalizeHtml(renderToDiv(tmpl).innerHTML), + test.equal(canonicalizeHtml(renderToDiv(tmpl, tmplData).innerHTML), ["This is generated by a helper with the same name.", "This is a template passed in the data context."].join(' ')); }); @@ -1499,7 +1499,7 @@ Tinytest.add("spacebars-tests - template_tests - checkbox", function(test) { Tinytest.add('spacebars-tests - template_tests - unfound template', function (test) { test.throws(function () { renderToDiv(Template.spacebars_test_nonexistent_template); - }, /Can't find template/); + }, /No such template/); }); Tinytest.add('spacebars-tests - template_tests - helper passed to #if called exactly once when invalidated', function (test) { @@ -1935,7 +1935,6 @@ Tinytest.add( var data = null; tmpl.events({ 'click span': function () { - console.log('clicked!'); data = this; } }); diff --git a/packages/spacebars-tests/templating_tests.js b/packages/spacebars-tests/templating_tests.js index fd7a973768..ae01a03a47 100644 --- a/packages/spacebars-tests/templating_tests.js +++ b/packages/spacebars-tests/templating_tests.js @@ -1,19 +1,3 @@ -// render and put in the document -var renderToDiv = function (comp, optData) { - var div = document.createElement("DIV"); - if (optData == null) { - Blaze.renderComponent(comp, div); - } else { - var constructor = - (typeof comp === 'function' ? comp : comp.constructor); - Blaze.render(function () { - return Blaze.With(optData, function () { - return new constructor; - }); - }).attach(div); - } - return div; -}; // for events to bubble an element needs to be in the DOM. // @return {Function} call this for cleanup diff --git a/packages/ui/template.js b/packages/ui/template.js index e3ab199b98..efca8616c6 100644 --- a/packages/ui/template.js +++ b/packages/ui/template.js @@ -36,10 +36,12 @@ updateTemplateInstance = function (comp) { }; } // assume `comp` is a UI.TemplateComponent, for now - if (comp.__dataFunc) + if (comp.__dataFunc) { tmpl.data = comp.__dataFunc(); - else - tmpl.data = null; + } else { + var dataVar = Blaze.getComponentDataVar(comp); + tmpl.data = dataVar ? dataVar.get() : null; + } if (comp.domrange && !comp.isFinalized) { tmpl.firstNode = comp.domrange.firstNode(); @@ -122,24 +124,25 @@ UI.TemplateComponent = Blaze.Component.extend({ }, renderTemplate: function () { return null; }, // override this renderToDOM: function() { - var range = UI.TemplateComponent.__super__.renderToDOM.call(this); - if (! this._eventMaps && - typeof this.events === "object") { + var self = this; + var range = UI.TemplateComponent.__super__.renderToDOM.call(self); + if (! self._eventMaps && + typeof self.events === "object") { // Provide limited back-compat support for `.events = {...}` - // syntax. Pass `this.events` to the original `.events(...)` + // syntax. Pass `self.events` to the original `.events(...)` // function. This code must run only once per component, in // order to not bind the handlers more than once, which is - // ensured by the fact that we only do this when `this._eventMaps` + // ensured by the fact that we only do this when `self._eventMaps` // is falsy, and we cause it to be set now. - UI.TemplateComponent.prototype.events.call(this, this.events); + UI.TemplateComponent.prototype.events.call(self, self.events); } - if (this._eventMaps) { - _.each(this._eventMaps, function (m) { - range.addDOMAugmenter(new Blaze.EventAugmenter(m)); + if (self._eventMaps) { + _.each(self._eventMaps, function (m) { + range.addDOMAugmenter(new Blaze.EventAugmenter(m, self)); }); } - if (this.rendered) { + if (self.rendered) { range.addDOMAugmenter(new UI.TemplateRenderedAugmenter); } return range; @@ -152,12 +155,13 @@ UI.TemplateComponent = Blaze.Component.extend({ for (var k in eventMap) { eventMap2[k] = (function (k, v) { return function (event/*, ...*/) { + var component = this; // passed by EventAugmenter var dataVar = Blaze.getElementDataVar(event.currentTarget); var data = dataVar && dataVar.get(); if (data == null) data = {}; var args = Array.prototype.slice.call(arguments); - var tmplInstance = updateTemplateInstance(self); + var tmplInstance = updateTemplateInstance(component); args.splice(1, 0, tmplInstance); return v.apply(data, args); };