PDA

View Full Version : [FIXED][3.x] Tabs don't hover



stever
8 May 2009, 4:05 PM
Try hovering over tabs in this example:

http://extjs.com/deploy/ext-3.0-rc1.1/examples/tabs/tabs.html

I made a quick fix in the tab panel class but later came upon other similar bugs signaling a deeper problem. In these cases the hover class was being added on random elements!

addClassOnOver() and addClassOnFocus() really mean to have the pattern of addClassOnClick(), saving the dom element via closure, not a reference to this, which in the case of the Fly element points to different things at different times.

Also, in the same file, repaint() has the dom via closure, yet forces a creation of Element even if the caller is using a flyweight. It doesn't mark it as _internal either. That part should read Ext.fly(dom,INTERNAL).removeClass(...).

Hmm... I should get a discount...

aconran
12 May 2009, 8:08 AM
Steve -

Thanks for the bug report. As you noted above addClassOnOver and addClassOnFocus methods are bugged due to a bad closure. What's currently in SVN:


addClassOnOver : function(className){
var me = this;
me.hover(
function(){
Ext.fly(me, INTERNAL).addClass(className);
},
function(){
Ext.fly(me, INTERNAL).removeClass(className);
}
);
return me;
}


The me variable reference can change when you the Ext.Element flyweight instance.

One solution would be to store the reference to the dom as you suggested above like so:


addClassOnOver : function(className){
var dom = this.dom;
this.hover(
function(){
Ext.fly(dom, INTERNAL).addClass(className);
},
function(){
Ext.fly(dom, INTERNAL).removeClass(className);
}
);
return this;
}


Another 3rd and final solution would be to use the original 2.x code.


addClassOnOver : function(className){
this.hover(
function(){
Ext.fly(this, '_internal').addClass(className);
},
function(){
Ext.fly(this, '_internal').removeClass(className);
}
);
return this;
}


Many of the changes done to Ext.Element from 2.x to 3.x were done to conserve filesize when running through a compressor. In this case, I don't think the byte size warrants the changes and I will be reverting back to the 2.x version.

stever
12 May 2009, 8:42 AM
Personally, I like the clarity of using the dom variable as was done in addClassOnClick. The current bugs have lots of visually facinating artifacts on a complex system when using simple class names...