Skip to content

Fixing the plugin system #3

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
yanatan16 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fixing the plugin system #3

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 1 addition & 9 deletions lib/each.js
View file
Open in desktop
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
View file
Open in desktop
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) {
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) {

@stephenmathieson stephenmathieson Mar 17, 2015

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IE will explode here

@yanatan16 yanatan16 Mar 18, 2015

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IE9+ should be fine. There are shims for IE8: see the reactive-ie8-shims.
We haven't changed the name yet
On Mar 16, 2015 9:26 PM, "Stephen Mathieson" notifications@github.com
wrote:

In lib/index.js
#3 (comment):

@@ -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) {

IE will explode here

Reply to this email directly or view it on GitHub
https://github.com/notacatjs/dommit/pull/3/files#r26547051.

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 || {}
})
}

function inDomTree(el) {
try {
Expand Down
16 changes: 8 additions & 8 deletions test/adapters.js
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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