Page 1 of 2 12 LastLast
Results 1 to 10 of 14

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

    Thank you for reporting this bug. We will make it our priority to review this report.
  1. #1
    Sencha User
    Join Date
    Jun 2008
    Posts
    157

    Default [OPEN-19][3.0.0/2.2] GridView does not fire rowsinserted event when store is empty

    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):
    Code:
    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);
        });

  2. #2

  3. #3
    Ext User
    Join Date
    Jul 2007
    Location
    Florida
    Posts
    9,996

    Default

    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?).

  4. #4
    Ext User
    Join Date
    Jul 2007
    Location
    Florida
    Posts
    9,996

    Default

    For ease of use here's a drop in test case for array grid example:
    Code:
    /*
     * 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)
    
    });

  5. #5
    Sencha User
    Join Date
    Jun 2008
    Posts
    157

    Default

    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.

    Code:
    /*
     * 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)
    
    });

  6. #6
    Ext User
    Join Date
    Jul 2009
    Location
    Madrid
    Posts
    3

    Thumbs up

    Quote Originally Posted by mjlecomte View Post
    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.

  7. #7
    Ext User
    Join Date
    Jul 2007
    Location
    Florida
    Posts
    9,996

    Default

    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.

  8. #8
    Sencha User jay@moduscreate.com's Avatar
    Join Date
    Mar 2007
    Location
    DC Area =)
    Posts
    16,364

    Default

    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.

  9. #9
    Ext User
    Join Date
    Jul 2007
    Location
    Florida
    Posts
    9,996

    Default

    I'm going to mark this OPEN for time being.

  10. #10
    Sencha User
    Join Date
    Jun 2008
    Posts
    157

    Default

    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.

Page 1 of 2 12 LastLast

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •