PDA

View Full Version : RowExpander collapse event has undefined 'body'



eashwaranp
4 Aug 2011, 9:10 AM
I had a 'collapse' listener registered on the RowExpander plugin. But the body argument was undefined. On scrutinizing the collapseRow in the plugin code, I found that it was looking for the 1st child (tr:nth(1) div.x-grid3-row-body) instead of the 2nd child.

Changing it to use the same expression as expandRow (i.e. tr:nth(2) div.x-grid3-row-body), seem to have fixed it. I now get a proper defined 'body' in my collapse listener.

Just wanted to run this by here, to see if this is correct. If so, can this fix be applied to the next version of Ext.ux.grid.RowExpander?


collapseRow : function(row){
if(typeof row == 'number'){
row = this.grid.view.getRow(row);
}
var record = this.grid.store.getAt(row.rowIndex);
var body = Ext.fly(row).child('tr:nth(1) div.x-grid3-row-body', true);
if(this.fireEvent('beforecollapse', this, record, body, row.rowIndex) !== false){
this.state[record.id] = false;
Ext.fly(row).replaceClass('x-grid3-row-expanded', 'x-grid3-row-collapsed');
this.fireEvent('collapse', this, record, body, row.rowIndex);
}
}

jsakalos
4 Aug 2011, 11:24 PM
This is a valid bug report together with a suggested fix. Do you want this thread to be moved to Bugs so that developers can include your fix in a next Ext release?

eashwaranp
5 Aug 2011, 6:11 AM
That would be great! Thanks for reviewing the fix.

I'm not sure how to go about moving it to Bugs. Not done this before. Please let me know how to go about it and I can move this there.

Thanks again,
Eashwaran.

eashwaranp
8 Aug 2011, 10:37 AM
Hello Saki,

I was wondering if the 'tr:nth()' is also needed. We could just use 'tr div.x-grid3-row-body', right?

The tr:nth(2) also will fail if there are other table/tr/td elements embedded in some columns in the main row. Eg. if one of the columns uses a renderer and it uses a table/tr/td to render it, then tr:nth(2) will point to the tr in that rendered column!!

So, maybe the nth(2) should also go away.
Please let me know what you think.

Regards,
Eashwaran.

eashwaranp
13 Aug 2011, 11:46 AM
Also, I think we should not use Ext.fly to get the body, since it gets passed into the fireEvent as argument!

eashwaranp
13 Aug 2011, 11:47 AM
Nevermind the last comment! I see that we only use Ext.fly to get to Element and then immediately invoke its 'child' method. Sorry!

daiei27
2 Sep 2011, 9:50 AM
This is a valid bug report together with a suggested fix. Do you want this thread to be moved to Bugs so that developers can include your fix in a next Ext release?
Saki, you're implying the ExtJS team is helping to maintain the RowExpander plugin?

The original RowExpander had issues like memory leaks when Ext components were added to the expanded body. Mikhail and I updated the plugin in 3.x to fix these issues, but I can't figure out a good way to get it into the mainstream. It seems many users are unaware and starting from the original plugin. I'm wondering how to make this the starting point for 4.x or added to the ExtJS source if there is a place for such things...
http://www.sencha.com/forum/showthread.php?96245-3.x-3.2.0-RowExpander-plugin-UPDATED-April-5-2010