PDA

View Full Version : [FIXED-19][3.0.0/2.2] GridView does not fire rowsinserted event when store is empty



wiznia
23 Jul 2009, 7:07 AM
Any explanation for this behaviour? Is this a bug? The same thing happens in Ext 3.0
I think a fix could be applied with this override (not tested):


Ext.override(Ext.grid.GridView, {
insertRows : function(dm, firstRow, lastRow, isUpdate){
var last = dm.getCount() - 1;
if(!isUpdate && firstRow === 0 && lastRow >= last){
this.fireEvent("beforerowsinserted", this, firstRow, lastRow);
this.refresh();
this.fireEvent("rowsinserted", this, firstRow, lastRow);
}else{
if(!isUpdate){
this.fireEvent("beforerowsinserted", this, firstRow, lastRow);
}
var html = this.renderRows(firstRow, lastRow),
before = this.getRow(firstRow);
if(before){
if(firstRow === 0){
Ext.fly(this.getRow(0)).removeClass(this.firstRowCls);
}
Ext.DomHelper.insertHtml('beforeBegin', before, html);
}else{
var r = this.getRow(last - 1);
if(r){
Ext.fly(r).removeClass(this.lastRowCls);
}
Ext.DomHelper.insertHtml('beforeEnd', this.mainBody.dom, html);
}
if(!isUpdate){
this.fireEvent("rowsinserted", this, firstRow, lastRow);
this.processRows(firstRow);
}else if(firstRow === 0 || firstRow >= last){
//ensure first/last row is kept after an update.
Ext.fly(this.getRow(firstRow)).addClass(firstRow === 0 ? this.firstRowCls : this.lastRowCls);
}
}
this.syncFocusEl(firstRow);
});

mjlecomte
23 Jul 2009, 8:09 AM
I copied your original post from the 2.x bugs thread here:
http://extjs.com/forum/showthread.php?t=46972

mjlecomte
23 Jul 2009, 8:13 AM
This would potentially be a breaking change if the behavior was altered.

I don't know that the existing behavior is clearly a bug. It could be considered a documentation issue, depends on how the definition of "insert" is perceived (You can't really "insert" into something that doesn't exist, you're only adding really. If you insert into something that is empty is the insert position at 0-start or at the end?).

mjlecomte
23 Jul 2009, 8:15 AM
For ease of use here's a drop in test case for array grid example:


/*
* Ext JS Library 3.0+
* Copyright(c) 2006-2009, Ext JS, LLC.
* [email protected]
*
* http://extjs.com/license
*/

Ext.onReady(function(){

// NOTE: This is an example showing simple state management. During development,
// it is generally best to disable state management as dynamically-generated ids
// can change across page loads, leading to unpredictable results. The developer
// should ensure that stable state ids are set for stateful components in real apps.
Ext.state.Manager.setProvider(new Ext.state.CookieProvider());

// sample static data for the store
data = [
['3m Co',71.72,0.02,0.03,'9/1 12:00am'],
['Alcoa Inc',29.01,0.42,1.47,'9/1 12:00am'],
['Altria Group Inc',83.81,0.28,0.34,'9/1 12:00am'],
['American Express Company',52.55,0.01,0.02,'9/1 12:00am'],
['American International Group, Inc.',64.13,0.31,0.49,'9/1 12:00am'],
['AT&T Inc.',31.61,-0.48,-1.54,'9/1 12:00am'],
['Boeing Co.',75.43,0.53,0.71,'9/1 12:00am'],
['Caterpillar Inc.',67.27,0.92,1.39,'9/1 12:00am'],
['Citigroup, Inc.',49.37,0.02,0.04,'9/1 12:00am'],
['E.I. du Pont de Nemours and Company',40.48,0.51,1.28,'9/1 12:00am'],
['Exxon Mobil Corp',68.1,-0.43,-0.64,'9/1 12:00am'],
['General Electric Company',34.14,-0.08,-0.23,'9/1 12:00am'],
['General Motors Corporation',30.27,1.09,3.74,'9/1 12:00am'],
['Hewlett-Packard Co.',36.53,-0.03,-0.08,'9/1 12:00am'],
['Honeywell Intl Inc',38.77,0.05,0.13,'9/1 12:00am'],
['Intel Corporation',19.88,0.31,1.58,'9/1 12:00am'],
['International Business Machines',81.41,0.44,0.54,'9/1 12:00am'],
['Johnson & Johnson',64.72,0.06,0.09,'9/1 12:00am'],
['JP Morgan & Chase & Co',45.73,0.07,0.15,'9/1 12:00am'],
['McDonald\'s Corporation',36.76,0.86,2.40,'9/1 12:00am'],
['Merck & Co., Inc.',40.96,0.41,1.01,'9/1 12:00am'],
['Microsoft Corporation',25.84,0.14,0.54,'9/1 12:00am'],
['Pfizer Inc',27.96,0.4,1.45,'9/1 12:00am'],
['The Coca-Cola Company',45.07,0.26,0.58,'9/1 12:00am'],
['The Home Depot, Inc.',34.64,0.35,1.02,'9/1 12:00am'],
['The Procter & Gamble Company',61.91,0.01,0.02,'9/1 12:00am'],
['United Technologies Corporation',63.26,0.55,0.88,'9/1 12:00am'],
['Verizon Communications',35.57,0.39,1.11,'9/1 12:00am'],
['Wal-Mart Stores, Inc.',45.45,0.73,1.63,'9/1 12:00am']
];

/**
* Custom function used for column renderer
* @param {Object} val
*/
function change(val){
if(val > 0){
return '<span style="color:green;">' + val + '</span>';
}else if(val < 0){
return '<span style="color:red;">' + val + '</span>';
}
return val;
}

/**
* Custom function used for column renderer
* @param {Object} val
*/
function pctChange(val){
if(val > 0){
return '<span style="color:green;">' + val + '%</span>';
}else if(val < 0){
return '<span style="color:red;">' + val + '%</span>';
}
return val;
}

// create the data store
var store = new Ext.data.ArrayStore({
fields: [
{name: 'company'},
{name: 'price', type: 'float'},
{name: 'change', type: 'float'},
{name: 'pctChange', type: 'float'},
{name: 'lastChange', type: 'date', dateFormat: 'n/j h:ia'}
]
});

// create the Grid
grid = new Ext.grid.GridPanel({
store: store,
enableColumnResize: false,
columns: [
{id:'company',header: 'Company', width: 160, sortable: true, dataIndex: 'company'},
{header: 'Price', width: 75, sortable: true, renderer: 'usMoney', dataIndex: 'price'},
{header: 'Change', width: 75, sortable: true, renderer: change, dataIndex: 'change'},
{header: '% Change', width: 75, sortable: true, renderer: pctChange, dataIndex: 'pctChange'},
{header: 'Last Updated', width: 85, sortable: true, renderer: Ext.util.Format.dateRenderer('m/d/Y'), dataIndex: 'lastChange'}
],
stripeRows: true,
autoExpandColumn: 'company',
height: 350,
width: 600,
title: 'Array Grid',
// config options for stateful behavior
stateful: true,
stateId: 'grid'
});

grid.getView().on({
beforerefresh: function(){
console.info('beforerefresh');
},
beforerowsinserted: function(){
console.info('beforerowsinserted');
},
rowsinserted: function(){
console.info('rowsinserted');
}
});

// render the grid to the specified div in the page
grid.render('grid-example');

// manually load local data
// store.loadData(data);
// grid.getStore().loadData(data, true)

});

wiznia
24 Jul 2009, 2:03 AM
I think it's a bug. Maybe I wasn't clear in my explanation, I modified you test case to better reproduce the problem.
Click on the "new" button. The first time you click it, it only fires the beforerefresh and refresh events. The following times it fires the beforerowsinserted and rowsinserted despite the action is the same (calling the store.add method).
With the override I proposed, the first time you click it fires beforerowsinserted, beforerefresh, refresh, rowsinserted which I think it's what's expected.



/*
* Ext JS Library 3.0+
* Copyright(c) 2006-2009, Ext JS, LLC.
* [email protected]
*
* http://extjs.com/license
*/

Ext.onReady(function(){

// NOTE: This is an example showing simple state management. During development,
// it is generally best to disable state management as dynamically-generated ids
// can change across page loads, leading to unpredictable results. The developer
// should ensure that stable state ids are set for stateful components in real apps.
Ext.state.Manager.setProvider(new Ext.state.CookieProvider());

// sample static data for the store
data = [
['3m Co',71.72,0.02,0.03,'9/1 12:00am'],
['Alcoa Inc',29.01,0.42,1.47,'9/1 12:00am'],
['Altria Group Inc',83.81,0.28,0.34,'9/1 12:00am'],
['American Express Company',52.55,0.01,0.02,'9/1 12:00am'],
['American International Group, Inc.',64.13,0.31,0.49,'9/1 12:00am'],
['AT&T Inc.',31.61,-0.48,-1.54,'9/1 12:00am'],
['Boeing Co.',75.43,0.53,0.71,'9/1 12:00am'],
['Caterpillar Inc.',67.27,0.92,1.39,'9/1 12:00am'],
['Citigroup, Inc.',49.37,0.02,0.04,'9/1 12:00am'],
['E.I. du Pont de Nemours and Company',40.48,0.51,1.28,'9/1 12:00am'],
['Exxon Mobil Corp',68.1,-0.43,-0.64,'9/1 12:00am'],
['General Electric Company',34.14,-0.08,-0.23,'9/1 12:00am'],
['General Motors Corporation',30.27,1.09,3.74,'9/1 12:00am'],
['Hewlett-Packard Co.',36.53,-0.03,-0.08,'9/1 12:00am'],
['Honeywell Intl Inc',38.77,0.05,0.13,'9/1 12:00am'],
['Intel Corporation',19.88,0.31,1.58,'9/1 12:00am'],
['International Business Machines',81.41,0.44,0.54,'9/1 12:00am'],
['Johnson & Johnson',64.72,0.06,0.09,'9/1 12:00am'],
['JP Morgan & Chase & Co',45.73,0.07,0.15,'9/1 12:00am'],
['McDonald\'s Corporation',36.76,0.86,2.40,'9/1 12:00am'],
['Merck & Co., Inc.',40.96,0.41,1.01,'9/1 12:00am'],
['Microsoft Corporation',25.84,0.14,0.54,'9/1 12:00am'],
['Pfizer Inc',27.96,0.4,1.45,'9/1 12:00am'],
['The Coca-Cola Company',45.07,0.26,0.58,'9/1 12:00am'],
['The Home Depot, Inc.',34.64,0.35,1.02,'9/1 12:00am'],
['The Procter & Gamble Company',61.91,0.01,0.02,'9/1 12:00am'],
['United Technologies Corporation',63.26,0.55,0.88,'9/1 12:00am'],
['Verizon Communications',35.57,0.39,1.11,'9/1 12:00am'],
['Wal-Mart Stores, Inc.',45.45,0.73,1.63,'9/1 12:00am']
];

/**
* Custom function used for column renderer
* @param {Object} val
*/
function change(val){
if(val > 0){
return '<span style="color:green;">' + val + '</span>';
}else if(val < 0){
return '<span style="color:red;">' + val + '</span>';
}
return val;
}

/**
* Custom function used for column renderer
* @param {Object} val
*/
function pctChange(val){
if(val > 0){
return '<span style="color:green;">' + val + '%</span>';
}else if(val < 0){
return '<span style="color:red;">' + val + '%</span>';
}
return val;
}

// create the data store
var store = new Ext.data.ArrayStore({
fields: [
{name: 'company'},
{name: 'price', type: 'float'},
{name: 'change', type: 'float'},
{name: 'pctChange', type: 'float'},
{name: 'lastChange', type: 'date', dateFormat: 'n/j h:ia'}
]
});

// create the Grid
grid = new Ext.grid.GridPanel({
store: store,
enableColumnResize: false,
columns: [
{id:'company',header: 'Company', width: 160, sortable: true, dataIndex: 'company'},
{header: 'Price', width: 75, sortable: true, renderer: 'usMoney', dataIndex: 'price'},
{header: 'Change', width: 75, sortable: true, renderer: change, dataIndex: 'change'},
{header: '% Change', width: 75, sortable: true, renderer: pctChange, dataIndex: 'pctChange'},
{header: 'Last Updated', width: 85, sortable: true, renderer: Ext.util.Format.dateRenderer('m/d/Y'), dataIndex: 'lastChange'}
],
stripeRows: true,
autoExpandColumn: 'company',
height: 350,
width: 600,
title: 'Array Grid',
// config options for stateful behavior
stateful: true,
stateId: 'grid',
tbar: [{
xtype: "button",
text: 'New',
handler: function(){
store.add(new store.recordType({price:0}));
}
}]
});

grid.getView().on({
beforerefresh: function(){
console.info('beforerefresh');
},
beforerowsinserted: function(){
console.info('beforerowsinserted');
},
rowsinserted: function(){
console.info('rowsinserted');
}
});

// render the grid to the specified div in the page
grid.render('grid-example');

// manually load local data
// store.loadData(data);
// grid.getStore().loadData(data, true)

});

pwnfactory
24 Jul 2009, 2:17 AM
I don't know that the existing behavior is clearly a bug. It could be considered a documentation issue, depends on how the definition of "insert" is perceived (You can't really "insert" into something that doesn't exist, you're only adding really.)

I think you are inserting into the Grid, which does always exist.

I agree with wiznia, beforerowsinserted and rowsinserted should be fired regardless of the number of existing rows.

mjlecomte
24 Jul 2009, 5:31 AM
To clarify, I'm not really taking a position one way or the other. I haven't subscribed to that event so it wouldn't break any of my apps.

In the code I posted above I do recognize the difference depending on if there's data in the store or not.

Do you guys at least agree that this could be a breaking change for someone? That is my stated concern for changing the behavior.

24 Jul 2009, 5:41 AM
It definitely alters the behavior and is needed.

IMHO, The only way this could be considered a breaking change if somoene has developed grids to match the current behavior.

mjlecomte
24 Jul 2009, 7:36 AM
I'm going to mark this OPEN for time being.

wiznia
24 Jul 2009, 11:38 AM
Yes, this could be a breaking change, of course. But, I think it's still a bug, if you have an event that fires when a row is inserted, then it doesn't matter if it's the first row that is inserted or not, it should fire always. And if this is the correct behaviour, then it should be well documented.
It's up to the ext team to decide if the functionality it's changed or not, for now I'll use the override I posted.

pwnfactory
27 Jul 2009, 1:51 AM
Do you guys at least agree that this could be a breaking change for someone? That is my stated concern for changing the behavior.
Yes, this could be a breaking change. Also, it seems the current behavior was explicitly coded and perhaps there's a subtle reason we're not seeing?

elkidos
2 Aug 2009, 2:50 PM
Like the others, I would like to get a fix to make the rowinserted event fires anytime, which it doesn't happen when the store is empty. Thx!

wiznia
28 Oct 2009, 4:39 AM
So, what is the status of this? Is it being fixed in the next version??

aconran
30 Oct 2009, 11:38 AM
Fix applied to SVN revision 5568 for the Ext 3.1 release.