Skip to content

Commit

Permalink
add .render() to make the plugin system useful again. reduce .bind() …
Browse files Browse the repository at this point in the history
…functionality. Add plugin tests. See #1
  • Loading branch information
Jon Eisen committed Sep 26, 2014
1 parent df98d16 commit ab36aa3
Show file tree
Hide file tree
Showing 9 changed files with 259 additions and 170 deletions.
10 changes: 1 addition & 9 deletions lib/each.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,6 @@
module.exports = function(el, val) {
var self = this;

// get the reactive constructor from the current reactive instance
// TODO(shtylman) port over adapter and bindings from instance?
var Reactive = self.reactive.constructor;

var val = val.split(/ +/);
el.removeAttribute('each');

Expand All @@ -31,11 +27,7 @@ module.exports = function(el, val) {
var views = [];

function childView(el, model) {
return Reactive(el, model, {
delegate: self.view,
adapter: self.reactive.opt.adapter,
bindings: self.reactive.bindings
});
return self.reactive.subview(model).render(el)
}

var array;
Expand Down
81 changes: 49 additions & 32 deletions lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,43 +30,49 @@ exports = module.exports = Reactive;
* @api public
*/

function Reactive(el, model, opt) {
if (!(this instanceof Reactive)) return new Reactive(el, model, opt);
function Reactive(model, opt) {
if (!(this instanceof Reactive)) return new Reactive(model, opt);
opt = opt || {};

if (typeof el === 'string') {
el = domify(el);
}

var self = this;
self.opt = opt || {};
self.model = model || {};
self.adapter = (opt.adapter || Adapter)(self.model);
self.el = el;
self.view = opt.delegate || Object.create(null);

self.bindings = opt.bindings || Object.create(null);

// TODO undo this crap and just export bindings regularly
// not that binding order matters!!
bindings({
bind: function(name, fn) {
self.bindings[name] = fn;
}
});

self._bind(this.el, []);
this.use(bindings)
}

Emitter(Reactive.prototype);

/**
* Render the view.
*
* @param {String|DOM} el
* @return {Reactive}
* @api public
*/

Reactive.prototype.render = function (el) {

This comment has been minimized.

Copy link
@defunctzombie

defunctzombie Oct 26, 2014

Contributor

Does this render onto an element? or is the el meant to be a string template? Can you render more than once? I feel that the template should be specified when creating the view and render should be just the thing that binds maybe?

var self = this;

if (typeof el === 'string') {
el = domify(el);
}

self.el = el;
self._bind(this.el, []);
return self;
}

/**
* Subscribe to changes on `prop`.
*
* @param {String} prop
* @param {Function} fn
* @return {Reactive}
* @api private
* @api public
*/

Reactive.prototype.sub = function(prop, fn){
Expand Down Expand Up @@ -288,24 +294,18 @@ Reactive.prototype._bind = function() {
Reactive.prototype.bind = function(name, fn) {
var self = this;
if ('object' == typeof name) {
for (var key in name) {
this.bind(key, name[key]);
}
return;
Object.keys(name).forEach(function (key) {
self.bind(key, name[key]);
})
return self;
}

var els = query.all('[' + name + ']', this.el);
if (this.el.hasAttribute && this.el.hasAttribute(name)) {
els = [].slice.call(els);
els.unshift(this.el);
if (!self.el) {
self.bindings[name] = fn;
return self;
}
if (!els.length) return;

debug('bind [%s] (%d elements)', name, els.length);
for (var i = 0; i < els.length; i++) {
var binding = new Binding(name, this, els[i], fn);
binding.bind();
}
throw new Error('.bind() cannot be called after .render()')
};

/**
Expand All @@ -326,6 +326,7 @@ Reactive.prototype.destroy = function() {
self.adapter.unsubscribeAll();
self.emit('destroyed');
self.removeAllListeners();
delete self.el;
};

/**
Expand All @@ -339,6 +340,22 @@ Reactive.prototype.use = function(fn) {
return this;
};

/**
* Create a Dommit view with the given model but the same bindings, etc.
*
* @param {Object} model
* @return {Reactive} subview
* @api public
*/

Reactive.prototype.subview = function (model) {
return new Reactive(model, {
bindings: this.bindings,
adapter: this.opt.adapter,
bindings: this.bindings,
delegate: this.view || {}
})
}

This comment has been minimized.

Copy link
@defunctzombie

defunctzombie Oct 26, 2014

Contributor

This should be pulled out into a separate commit. Otherwise, I think I like the direction this PR changed things.

This comment has been minimized.

Copy link
@defunctzombie

defunctzombie Oct 26, 2014

Contributor

I think a better name might be "clone" ? One thought I have is that the render method should maybe accept the model and return a "view instance" ?

This comment has been minimized.

Copy link
@yanatan16

yanatan16 via email Oct 26, 2014

Member

This comment has been minimized.

Copy link
@defunctzombie

defunctzombie Oct 27, 2014

Contributor

I like dommit(template, model, options).render() for now. It is more consistent with what we have currently and allows for your other code cleanup to remain. Lets go with that. This method we can remove and add back later.


function inDomTree(el) {
try {
Expand Down
16 changes: 8 additions & 8 deletions test/adapters.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,17 +73,17 @@ describe('custom adapter', function() {
});

it('setting obj[prop] should update view', function() {
reactive(el, person, {
reactive(person, {
adapter: BackboneAdapter
});
}).render(el);
person.set('name', 'TJ');
assert('TJ' == el.children[0].textContent);
});

it('should not double set when updating reactive instance', function(done) {
var react = reactive(el, person, {
var react = reactive(person, {
adapter: BackboneAdapter
});
}).render(el);
react.sub('name', function(val) {
assert.equal(val, 'TJ');
done();
Expand All @@ -92,18 +92,18 @@ describe('custom adapter', function() {
});

it('shouldnt update view after being unsubscribed', function() {
var react = reactive(el, person, {
var react = reactive(person, {
adapter: BackboneAdapter
});
}).render(el);
react.unsub('name');
person.set('name', 'TJ');
assert('Matt' == el.children[0].textContent);
});

it('setting view should update object', function() {
var react = reactive(el, person, {
var react = reactive(person, {
adapter: BackboneAdapter
});
}).render(el);
react.set('name', 'TJ');
assert('TJ' == el.children[0].textContent);
assert('TJ' == person.get('name'));
Expand Down
10 changes: 5 additions & 5 deletions test/attr-interpolation.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,21 @@ describe('attr interpolation', function(){
it('should support initialization', function(){
var el = domify('<a href="/download/{{id}}"></a>');
var user = { id: '1234' };
var view = reactive(el, user);
var view = reactive(user).render(el);
assert('/download/1234' == el.getAttribute('href'));
})

it('should ignore whitespace', function(){
var el = domify('<a href="/download/{{ id }}"></a>');
var user = { id: '1234' };
var view = reactive(el, user);
var view = reactive(user).render(el);
assert('/download/1234' == el.getAttribute('href'));
})

it('should react to changes', function(){
var el = domify('<a href="/download/{{id}}"></a>');
var user = { id: '1234' };
var view = reactive(el, user);
var view = reactive(user).render(el);

assert('/download/1234' == el.getAttribute('href'));

Expand All @@ -34,7 +34,7 @@ describe('attr interpolation', function(){
it('should support multiple attributes', function(){
var el = domify('<a href="/download/{{id}}" id="file-{{id}}">Download {{file}}</a>');
var user = { id: '1234' };
var view = reactive(el, user);
var view = reactive(user).render(el);

assert('/download/1234' == el.getAttribute('href'));
assert('file-1234' == el.getAttribute('id'));
Expand All @@ -47,7 +47,7 @@ describe('attr interpolation', function(){
it('should support multiple properties', function(){
var el = domify('<a href="/download/{{id}}-{{file}}"></a>');
var user = { id: '1234', file: 'something' };
var view = reactive(el, user);
var view = reactive(user).render(el);

assert('/download/1234-something' == el.getAttribute('href'));

Expand Down
82 changes: 36 additions & 46 deletions test/bindings.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,12 @@ var reactive = require('../');
describe('reactive.bind(name, fn)', function(){
it('should define a new binding', function(done){
var el = domify('<div><h1 data-editable="/item/12">Title</h1></div>');
var react = reactive(el, {}, {
bindings: {
'data-editable': function(el, url){
el.setAttribute('contenteditable', 'true');
assert('/item/12' == url);
}
}
});
var react = reactive()
.bind('data-editable', function(el, url){
el.setAttribute('contenteditable', 'true');
assert('/item/12' == url);
})
.render(el)

assert(el.children[0].getAttribute('contenteditable'));
done();
Expand All @@ -26,64 +24,56 @@ describe('reactive.bind(name, fn)', function(){
var model = {
todos: [ { title: 'test title' } ]
};
var react = reactive(el, model, {
bindings: {
'lowercase': function(el, prop){
var binding = this;
assert(prop === 'title');
var val = binding.value(prop);
assert(val === 'test title');
done();
}
}
});
var react = reactive(model)
.bind('lowercase', function(el, prop){
var binding = this;
assert(prop === 'title');
var val = binding.value(prop);
assert(val === 'test title');
done();
})
.render(el)
});
})

describe('reactive.bind(obj)', function(){
it('should define several bindings', function(done){
var el = domify('<div><h1 hello="world">Title</h1></div>');
var react = reactive(el, {}, {
bindings: {
'hello': function(el, val){
assert('world' == val);
done();
}
}
});
var react = reactive({})
.bind('hello', function(el, val){
assert('world' == val);
done();
})
.render(el)
})
})

describe('Reactive#bind(name, fn)', function(){
it('should initialize a view-specific binding', function(done){
var el = domify('<ul><li removable></li></ul>');
var view = reactive(el, {}, {
bindings: {
'removable': function(el){
assert('LI' == el.nodeName);
done();
}
}
});
var view = reactive()
.bind('removable', function(el){
assert('LI' == el.nodeName);
done();
})
.render(el)
})

it('should support root-level bindings', function(done){
var el = domify('<ul removable><li></li></ul>');
var view = reactive(el, {}, {
bindings: {
'removable': function(el){
assert('UL' == el.nodeName);
done();
}
}
});
var view = reactive()
.bind('removable', function(el){
assert('UL' == el.nodeName);
done();
})
.render(el)
})
})

describe('Reactive#bind(name, fn)', function(){
it('should not use setAttribute to update input\'s value', function(){
var el = domify('<input data-value="value" />');
var view = reactive(el, { value: 'old value' });
var view = reactive({ value: 'old value' }).render(el);
view.el.value = 'old value';

assert(el.value == 'old value');
Expand All @@ -93,14 +83,14 @@ describe('Reactive#bind(name, fn)', function(){

it('should not use setAttribute to update textarea\'s value', function(){
var el = domify('<textarea data-value="value"></textarea>');
var view = reactive(el, { value: 'old value' });
var view = reactive({ value: 'old value' }).render(el);
view.set('value', 'value');
assert(el.value == 'value');
})

it('should change value of `.value` to update textarea\'s text content', function(){
var el = domify('<textarea data-text="value"></textarea>');
var view = reactive(el, { value: 'old value' });
var view = reactive({ value: 'old value' }).render(el);
view.set('value', 'value');
assert(el.value == 'value');
})
Expand Down
Loading

0 comments on commit ab36aa3

Please sign in to comment.