PDA

View Full Version : [FIXED][3.0] Observable problems



stever
26 Mar 2009, 8:40 PM
First, let me say I hate the whole core/more and lite editions of the same class. Ick.

Anyway, doing something as simple as mycomponent.on('render',myfunction, myscope, {delay:10000}) does not work. It fires immediately.

In Ext2:

Ext.util.Event.prototype = {
createListener : function(fn, scope, o){
o = o || {};
scope = scope || this.obj;
var l = {fn: fn, scope: scope, options: o};
var h = fn;
if(o.delay){
h = createDelayed(h, o, scope);
}
if(o.single){
h = createSingle(h, this, fn, scope);
}
if(o.buffer){
h = createBuffered(h, o, scope);
}
l.fireFn = h;
return l;
},



In Ext 3:

EXTUTIL.Event.prototype = {
createListener : function(fn, scope, o){
o = o || {};
scope = scope || this.obj;
var l = {fn: fn, scope: scope, options: o},
h = fn;
if(o.target){
h = createTargeted(h, o, scope);
}
l.fireFn = h;
return l;
},


No single, buffered, or delayed.

Just to make things confusing, in Ext 3, Observable-more:

Ext.applyIf(Ext.util.Observable.prototype, function(){
...
return {
createListener: function(fn, scope, o){
o = o ||
{};
scope = scope || this.obj;
var l = {
fn: fn,
scope: scope,
options: o
}, h = fn;
if (o.target) {
h = createTargeted(h, o, scope);
}
if (o.delay) {
h = createDelayed(h, o, scope);
}
if (o.single) {
h = createSingle(h, this, fn, scope);
}
if (o.buffer) {
h = createBuffered(h, o, scope);
}
l.fireFn = h;
return l;
},

Not that anyone ever calls an observable::createListener (that only happens on events). It also shows some wasteful duplicate code.

In fact, by searching for things like "function createDelayed" I can see that there is lots of duplicated code that we have to live with. Has anyone figured out what this new strategy is going to cost us in size and speed?

evant
26 Mar 2009, 8:56 PM
Actually that duplicate code has always existed, the same in the 2.x source, to keep the different types of events seperate.

Looks like that should be apply() instead of applyIf().

evant
26 Mar 2009, 9:00 PM
Also, the just to explain, the -more stuff is there for a reason. As you may know, we're releasing an Ext base library very soon. As such, some features from a class are included while others aren't, so we've needed to split the files to make it easier to build them instead of having to maintain 2 source branches.

stever
26 Mar 2009, 9:19 PM
Looks like that should be apply() instead of applyIf().

That's what I thought, but that didn't work. The apply for that function needs to go the Event, not to Observable. Obviously the docs say it is for Observable, but that is because that is the API -- the options get passed to the event to actually handle.

stever
26 Mar 2009, 9:27 PM
Also, the just to explain, the -more stuff is there for a reason. As you may know, we're releasing an Ext base library very soon. As such, some features from a class are included while others aren't, so we've needed to split the files to make it easier to build them instead of having to maintain 2 source branches.

Yes, I know, I'm just of the opinion to not bother with the base library. I get the business idea behind it, just think that ship has sailed, and it makes things harder for the rest of us. Anyhow, just an opinion -- that doesn't count. ;)

I have a backlog of other bugs here, so I doubt the 'very soon' part, unless an alpha release or similar. Unfortunately, I used up all the time I set aside to see if I could get Ext3 up enough to test. Thanks for your quick responses the last day or two! Very nice and helpful.

evant
26 Mar 2009, 9:40 PM
You're right, it should actually be applied to the Event, not the Observable. I'm talking with the other guys now about why it wasn't included in the core to begin with. It could be a size thing.

evant
26 Mar 2009, 9:43 PM
Just FYI, here's a temporary fix.

If this doesn't go in the core, this will be the fix. Otherwise it will be in the core and we won't worry about it ;)



Ext.apply(Ext.util.Observable.prototype, function(){
// this is considered experimental (along with beforeMethod, afterMethod, removeMethodListener?)
// allows for easier interceptor and sequences, including cancelling and overwriting the return value of the call
// private
function getMethodEvent(method){
var e = (this.methodEvents = this.methodEvents ||
{})[method], returnValue, v, cancel, obj = this;

if (!e) {
this.methodEvents[method] = e = {};
e.originalFn = this[method];
e.methodName = method;
e.before = [];
e.after = [];

function makeCall(fn, scope, args){
if (!Ext.isEmpty(v = fn.apply(scope || obj, args))) {
if (Ext.isObject(v)) {
returnValue = !Ext.isEmpty(v.returnValue) ? v.returnValue : v;
cancel = !!v.cancel;
}
else
if (v === false) {
cancel = true;
}
else {
returnValue = v;
}
}
}

this[method] = function(){
var args = Ext.toArray(arguments);
returnValue = v = undefined;
cancel = false;

Ext.each(e.before, function(b){
makeCall(b.fn, b.scope, args);
if (cancel) {
return returnValue;
}
});

if (!Ext.isEmpty(v = e.originalFn.apply(obj, args))) {
returnValue = v;
}
Ext.each(e.after, function(a){
makeCall(a.fn, a.scope, args);
if (cancel) {
return returnValue;
}
});
return returnValue;
};
}
return e;
}

return {
// these are considered experimental
// allows for easier interceptor and sequences, including cancelling and overwriting the return value of the call
// adds an "interceptor" called before the original method
beforeMethod: function(method, fn, scope){
getMethodEvent.call(this, method).before.push({
fn: fn,
scope: scope
});
},

// adds a "sequence" called after the original method
afterMethod: function(method, fn, scope){
getMethodEvent.call(this, method).after.push({
fn: fn,
scope: scope
});
},

removeMethodListener: function(method, fn, scope){
var e = getMethodEvent.call(this, method), found = false;
Ext.each(e.before, function(b){
if (b.fn == fn && b.scope == scope) {
b.splice(i, 1);
found = true;
return false;
}
});
if (!found) {
Ext.each(e.after, function(a){
if (a.fn == fn && a.scope == scope) {
a.splice(i, 1);
return false;
}
});
}
},

/**
* Relays selected events from the specified Observable as if the events were fired by <tt><b>this</b></tt>.
* @param {Object} o The Observable whose events this object is to relay.
* @param {Array} events Array of event names to relay.
*/
relayEvents: function(o, events){
var me = this;
function createHandler(ename){
return function(){
return me.fireEvent.apply(me, [ename].concat(Ext.toArray(arguments)));
};
};
Ext.each(events, function(ename){
me.events[ename] = !me.events[ename];
o.on(ename, createHandler(ename), me);
});
},

/**
* Used to enable bubbling of events
* @param {Object} events
*/
enableBubble: function(events){
var me = this;
events = Ext.isArray(events) ? events : Ext.toArray(arguments);
Ext.each(events, function(ename){
ename = ename.toLowerCase();
var ce = me.events[ename] || true;
if (typeof ce == "boolean") {
ce = new Ext.util.Event(me, ename);
me.events[ename] = ce;
}
ce.bubble = true;
});
}
}
}());

Ext.apply(Ext.util.Event.prototype, function(){
function createBuffered(h, o, scope){
var toArray = Ext.toArray, task = new foo.DelayedTask();
return function(){
task.delay(o.buffer, h, scope, toArray(arguments));
};
}

function createSingle(h, e, fn, scope){
return function(){
e.removeListener(fn, scope);
return h.apply(scope, arguments);
};
}

function createDelayed(h, o, scope){
return function(){
var args = toArray(arguments);
(function(){
h.apply(scope, args);
}).defer(o.delay || 10);
};
}

return {
createListener: function(fn, scope, o){
o = o || {}, scope = scope || this.obj;
var l = {
fn: fn,
scope: scope,
options: o
}, h = fn;
if (o.target) {
h = createTargeted(h, o, scope);
}
if (o.delay) {
h = createDelayed(h, o, scope);
}
if (o.single) {
h = createSingle(h, this, fn, scope);
}
if (o.buffer) {
h = createBuffered(h, o, scope);
}
l.fireFn = h;
return l;
}
};
}());

stever
26 Mar 2009, 9:46 PM
OK, just a note: there is a call to createTargeted() which ought to be defined with the others like createBuffered(). I think it is global at the moment?

evant
26 Mar 2009, 9:55 PM
Yeah, it looks like that method will need to be copied from the core because it will private because it's in a closure.

aconran
4 May 2009, 11:10 AM
Is this issue committed and resolved?

evant
4 May 2009, 5:50 PM
This has been fixed in core as part of the refactoring Tommy has done.

stever
4 May 2009, 5:51 PM
Yes, it was committed to SVN a while ago..