[2.2] GridView bug

[sniffer]

Hi,
I think I've found a bug in ExtJS 2.2 version with GridPanel/EditorGridPanel and selection model.

Issue is tested with ExtJS 2.1 and ExtJS 2.2, with 2.1 everything working fine with grid panel object but with 2.2 it doesn't. New created grid panel has two issues with selection model. One is that created (shown) grid panel doesn't allowing selection of first row by user click on it, to select it user needs to click first some other row and then after to select first row. Second is nonfunctional call of selectFirstRow() method over getSelectionModel() method. Call of a method doesn't throws an exception because grid panel object does existing but it doesn't do selection of row.

Existing thread at forum about this is at http://extjs.com/forum/showthread.php?t=44038&page=1

Best regards,
Aleksandar Cajic

[mystix]

One is that created (shown) grid panel doesn't allowing selection of first row by user click on it, to select it user needs to click first some other row and then after to select first row.

confirmed -- behaviour is evident in the Array Grid Example.
Editor Grid Example works fine though.




Second is nonfunctional call of selectFirstRow() method over getSelectionModel() method. Call of a method doesn't throws an exception because grid panel object does existing but it doesn't do selection of row.

confirmed -- also evident in the Array Grid Example.
the Editor Grid uses a cellselectionmodel (not the rowselectionmodel), so there's no selectFirstRow() method to worry about.

[Condor]

I think this basically is the same problem as this post (http://extjs.com/forum/showthread.php?t=43543):

Ext 2.2 changed the execution order of grid rendering.

To be backward compatible I think it should be supported that a selection is made before the grid is rendered.

This means that GridView.renderRows should also take the current selections into account.

[mystix]

hmm.. regarding selectFirstRow() -- i tried making that call via the FB console (on the array grid example) a full 20s after the page loaded, but to no avail. any idea why?

[jay@moduscreate.com]

http://extjs.com/forum/showthread.php?p=208857#post208857

I found the problem - not a solution (no time)

[jay@moduscreate.com]

hmm.. regarding selectFirstRow() -- i tried making that call via the FB console (on the array grid example) a full 20s after the page loaded, but to no avail. any idea why?

SelModel finds the record selected (it's in the selmodel mixed collection), thus it doesn't make the change.

[jay@moduscreate.com]

Sniffer, please update your title, it's not a selmodel bug, but a gridview bug.

[jay@moduscreate.com]

i would look at GridView.js



doRender : function(cs, rs, ds, startRow, colCount, stripe){
var ts = this.templates, ct = ts.cell, rt = ts.row, last = colCount-1;
var tstyle = 'width:'+this.getTotalWidth()+';';
// buffers
var buf = [], cb, c, p = {}, rp = {tstyle: tstyle}, r;
for(var j = 0, len = rs.length; j < len; j++){
r = rs[j]; cb = [];
var rowIndex = (j+startRow);
for(var i = 0; i < colCount; i++){
c = cs[i];
p.id = c.id;
p.css = i == 0 ? 'x-grid3-cell-first ' : (i == last ? 'x-grid3-cell-last ' : '');
p.attr = p.cellAttr = "";
p.value = c.renderer(r.data[c.name], p, r, rowIndex, i, ds);
p.style = c.style;
if(p.value == undefined || p.value === "") p.value = "&#160;";
if(r.dirty && typeof r.modified[c.name] !== 'undefined'){
p.css += ' x-grid3-dirty-cell';
}
cb[cb.length] = ct.apply(p);
}
var alt = [];
if(stripe && ((rowIndex+1) % 2 == 0)){
alt[0] = "x-grid3-row-alt";
}
if(r.dirty){
alt[1] = " x-grid3-dirty-row";
}
rp.cols = colCount;
if(this.getRowClass){
alt[2] = this.getRowClass(r, rowIndex, rp, ds);
}
rp.alt = alt.join(" ");
rp.cells = cb.join("");
buf[buf.length] = rt.apply(rp);
}
return buf.join("");
},

[jay@moduscreate.com]

scratch that last post ;)

[Condor]

scratch that last post ;)

Why? That would work:


Ext.override(Ext.grid.GridView, {
doRender : function(cs, rs, ds, startRow, colCount, stripe){
var ts = this.templates, ct = ts.cell, rt = ts.row, last = colCount-1;
var tstyle = 'width:'+this.getTotalWidth()+';';
var sm = this.grid.getSelectionModel(), sc = sm.getSelectedCell ? sm.getSelectedCell() : null;
var buf = [], cb, c, p = {}, rp = {tstyle: tstyle}, r;
for(var j = 0, len = rs.length; j < len; j++){
r = rs[j]; cb = [];
var rowIndex = (j+startRow);
for(var i = 0; i < colCount; i++){
c = cs[i];
p.id = c.id;
p.css = i == 0 ? 'x-grid3-cell-first ' : (i == last ? 'x-grid3-cell-last ' : '');
if (sc && (sc[0] == rowIndex) && (sc[1] == i)) {
p.css += 'x-grid3-cell-selected ';
}
p.attr = p.cellAttr = "";
p.value = c.renderer(r.data[c.name], p, r, rowIndex, i, ds);
p.style = c.style;
if(p.value == undefined || p.value === "") p.value = " ";
if(r.dirty && typeof r.modified[c.name] !== 'undefined'){
p.css += ' x-grid3-dirty-cell';
}
cb[cb.length] = ct.apply(p);
}
var alt = [];
if(stripe && ((rowIndex+1) % 2 == 0)){
alt[0] = "x-grid3-row-alt";
}
if(r.dirty){
alt[1] = "x-grid3-dirty-row";
}
rp.cols = colCount;
if(this.getRowClass){
alt[2] = this.getRowClass(r, rowIndex, rp, ds);
}
if(sm.isSelected && sm.isSelected(r)) {
alt[3] = "x-grid3-row-selected";
}
rp.alt = alt.join(" ");
rp.cells = cb.join("");
buf[buf.length] = rt.apply(rp);
}
return buf.join("");
}
});

(but it does add an extra dependency between GridView and RowSelectionModel/CellSelectionModel)

[jay@moduscreate.com]

Why? That would work:

(but it does add an extra dependency between GridView and RowSelectionModel/CellSelectionModel)

I was looking at an *old* version (1.1) :))

it does work, but it ultimatelyl slows things down. :(

The added dependency is required because it seems that the enchancement makes the rendering of the rows asynchrnous.

[jay@moduscreate.com]

One thing i can tell is that by the time processRows is fired, the selModel is aware of the selection.

[jay@moduscreate.com]

Got it:


// private
processRows : function(startRow, skipStripe){
if(this.ds.getCount() < 1){
return;
}
skipStripe = skipStripe || !this.grid.stripeRows;
startRow = startRow || 0;
var rows = this.getRows();
var cls = ' x-grid3-row-alt ';
for(var i = startRow, len = rows.length; i < len; i++){
var row = rows[i];
row.rowIndex = i;
if(!skipStripe){
var isAlt = ((i+1) % 2 == 0);
var hasAlt = (' '+row.className + ' ').indexOf(cls) != -1;
if(isAlt == hasAlt){
continue;
}

if(isAlt){
row.className += " x-grid3-row-alt";
}else{
row.className = row.className.replace("x-grid3-row-alt", "");
}
}
var isSelected = false;
}
/*
Faster to go through the selections (if they are less than the total number of records of course);
*/
var selections = this.grid.selModel.getSelections();
if (selections.length > 0) {
for (var i = 0; i < selections.length; i++) {
var rec = selections[i];
var index = this.grid.store.indexOf(selections[i]);
Ext.fly(this.getRow(index)).addClass('x-grid3-row-selected');
}
}
},

[jay@moduscreate.com]

this is faster:


// private
processRows : function(startRow, skipStripe){
if(this.ds.getCount() < 1){
return;
}
skipStripe = skipStripe || !this.grid.stripeRows;
startRow = startRow || 0;
var rows = this.getRows();
var cls = ' x-grid3-row-alt ';
for(var i = startRow, len = rows.length; i < len; i++){
var row = rows[i];
row.rowIndex = i;
if(!skipStripe){
var isAlt = ((i+1) % 2 == 0);
var hasAlt = (' '+row.className + ' ').indexOf(cls) != -1;
if(isAlt == hasAlt){
continue;
}

if(isAlt){
row.className += " x-grid3-row-alt";
}else{
row.className = row.className.replace("x-grid3-row-alt", "");
}
}
var isSelected = false;
}
/*
Faster to go through the selections,
*/
var selections = this.grid.selModel.getSelections();
if (selections.length > 0) {
for (var i = 0; i < selections.length; i++) {
var rec = selections[i];
var index = this.grid.store.indexOf(selections[i]);
this.getRow(index).className += ' x-grid3-row-selected';
}
}
},

[Condor]

1. You assume that the selection model is a RowSelectionModel. It fails if you use a CellSelectionModel.

2. This is only faster if the number of selections is small. Both store.indexOf() and getRow(index) are slow.

[jay@moduscreate.com]

1. You assume that the selection model is a RowSelectionModel. It fails if you use a CellSelectionModel.

2. This is only faster if the number of selections is small. Both store.indexOf() and getRow(index) are slow.

Agreed. :-\

If we added it to the loop, it would be equally as slow:


for each row,

for each selected
... slow here ...
...

[jay@moduscreate.com]

A *really* simple solution to this is not to select anything until N time after the grid is rendered.

[jay@moduscreate.com]

The following works for the row selection model:
i go back to doRender ...



// private
doRender : function(cs, rs, ds, startRow, colCount, stripe){
var ts = this.templates, ct = ts.cell, rt = ts.row, last = colCount-1;
var tstyle = 'width:'+this.getTotalWidth()+';';
// buffers

/*
Faster to go through the selections,
*/
var selections = this.grid.selModel.getSelections();
if (selections.length > 0) {
/*
for (var i = 0; i < selections.length; i++) {
var rec = selections[i];
var index = this.grid.store.indexOf(selections[i]);
this.getRow(index).className += ' x-grid3-row-selected';
}
*/
}



var buf = [], cb, c, p = {}, rp = {tstyle: tstyle}, r;
for(var j = 0, len = rs.length; j < len; j++){
r = rs[j]; cb = [];
var rowIndex = (j+startRow);
for(var i = 0; i < colCount; i++){
c = cs[i];
p.id = c.id;
p.css = i == 0 ? 'x-grid3-cell-first ' : (i == last ? 'x-grid3-cell-last ' : '');
p.attr = p.cellAttr = "";
p.value = c.renderer(r.data[c.name], p, r, rowIndex, i, ds);
p.style = c.style;
if(p.value == undefined || p.value === "") p.value = "&#160;";
if(r.dirty && typeof r.modified[c.name] !== 'undefined'){
p.css += ' x-grid3-dirty-cell';
}
cb[cb.length] = ct.apply(p);
}
var alt = [];
if(stripe && ((rowIndex+1) % 2 == 0)){
alt[0] = "x-grid3-row-alt";
}
if(r.dirty){
alt[1] = " x-grid3-dirty-row";
}
rp.cols = colCount;
if(this.getRowClass){
alt[2] = this.getRowClass(r, rowIndex, rp, ds);
}
if (this.grid.selModel.isIdSelected(r.id)) {
alt.push(' x-grid3-row-selected');
}

rp.alt = alt.join(" ");
rp.cells = cb.join("");
buf[buf.length] = rt.apply(rp);
}
return buf.join("");
},

[Condor]

Using a delay to bypass asynchronous rendering really goes against my better judgement!

ps. I added:


if (sc && (sc[0] == rowIndex) && (sc[1] == i)) {
p.css += 'x-grid3-cell-selected ';
}
a

if(sm.isSelected && sm.isSelected(r)) {
alt[3] = "x-grid3-row-selected";
}

to the rendering loop and these should execute rather quickly (sm.isSelected(r) actually is fast, because it uses id based matching).

[jay@moduscreate.com]

Using a delay to bypass asynchronous rendering really goes against my better judgement!

ps. I added:


if (sc && (sc[0] == rowIndex) && (sc[1] == i)) {
p.css += 'x-grid3-cell-selected ';
}
a

if(sm.isSelected && sm.isSelected(r)) {
alt[3] = "x-grid3-row-selected";
}

to the rendering loop and these should execute rather quickly (sm.isSelected(r) actually is fast, because it uses id based matching).

Very nice indeed. :D

Ext core team - it's in your court now :)

[Condor]

The previous solutions work, but they add an extra dependency between GridView and RowSelectionModel/CellSelectionModel.

Maybe it would be better/simpler to use:


Ext.override(Ext.grid.RowSelectionModel, {
initEvents : function(){
if(!this.grid.enableDragDrop && !this.grid.enableDrag){
this.grid.on("rowmousedown", this.handleMouseDown, this);
}else{ this.grid.on("rowclick", function(grid, rowIndex, e) {
if(e.button === 0 && !e.shiftKey && !e.ctrlKey) {
this.selectRow(rowIndex, false);
grid.view.focusRow(rowIndex);
}
}, this);
}
this.rowNav = new Ext.KeyNav(this.grid.getGridEl(), {
"up" : function(e){
if(!e.shiftKey){
this.selectPrevious(e.shiftKey);
}else if(this.last !== false && this.lastActive !== false){
var last = this.last;
this.selectRange(this.last, this.lastActive-1);
this.grid.getView().focusRow(this.lastActive);
if(last !== false){
this.last = last;
}
}else{
this.selectFirstRow();
}
},
"down" : function(e){
if(!e.shiftKey){
this.selectNext(e.shiftKey);
}else if(this.last !== false && this.lastActive !== false){
var last = this.last;
this.selectRange(this.last, this.lastActive+1);
this.grid.getView().focusRow(this.lastActive);
if(last !== false){
this.last = last;
}
}else{
this.selectFirstRow();
}
},
scope: this
});
var view = this.grid.view;
view.on("render", this.onRefresh, this);
view.on("refresh", this.onRefresh, this);
view.on("rowupdated", this.onRowUpdated, this);
view.on("rowremoved", this.onRemove, this);
}
});
Ext.override(Ext.grid.GridView, {
afterRender: function(){
this.mainBody.dom.innerHTML = this.renderBody();
this.processRows(0, true);
if(this.deferEmptyText !== true){
this.applyEmptyText();
}
this.fireEvent("render", this);
}
});

[jay@moduscreate.com]

I would remove maybe from your post and put is :)
It removes the dependency *and* possible performance issues.
great job dude.

[sniffer]

Guys, I hope that core team reading this and already working on patch for version 2.2, this kind of issues from version to version are not good for ExtJS framework, I have even payed license for this product and running application on 2.1 version. With all respect, I don't need some extra code to override existing classes and to think what will be if new version of ExtJS comes out with existing applications based on this framework.

[jay@moduscreate.com]

I got confirmation that they are aware. :)

[sniffer]

Thank you! :)

[sniffer]

@Condor
Hi, solution that you provide is functional with SimpleStore but when the same code is in use with JsonStore provided override solution it isn't functional. Selection of first row isn't functional and mouse over row effect isn't functional.

So, code is completely the same just SimpleStore is replaced with JsonStore, here is a code example:



this.getGridElement = function() {
try {
// create the data store
var store = new Ext.data.JsonStore({
url: gridCrisisDataUrl,
baseParams: {
method: 'GET'
},
id: 'crisisId',
fields: [
{name: 'crisisId'},
{name: 'orderNumber'},
{name: 'name'},
{name: 'creator'},
{name: 'crDate', type : 'date', dateFormat:'d.m.Y'},
{name: 'crTime'},
{name: 'documents', type: 'bool'}
]
});
store.load();

/**
* Render document icons for each row
* @param val
*/
function documentRender(val) {
if (val) {
return '<div class="documentIcon"><img src="../lib/ext/resources/icons/book_open.png" class="control_document" title="'+translationData.label_document+'"><img src="../lib/ext/resources/icons/book_previous.png" class="control_upload" title="'+translationData.label_upload_document+'"></div>';
} else {
return '<div class="documentIcon"><img src="'+Ext.BLANK_IMAGE_URL+'"><img src="../lib/ext/resources/icons/book_previous.png" class="control_upload" title="'+translationData.label_upload_document+'"></div>';
}
}

/**
* Render view for user field
* @param val
*/
function renderUser(val) {
return val.name;
}

/**
* Create grid for crisis
*/
var grid;
return grid = new Ext.grid.EditorGridPanel({
id:'editableCrisisGrid',
store: store,
clicksToEdit:2,
autoScroll:true,
loadMask: true,
columns: [
{header: "Order nr.", width:20, sortable: true, dataIndex: 'orderNumber'},
{header: "Crisis", width:15, sortable: true, dataIndex: 'name', editor:new Ext.form.TextField({allowBlank:false})},
{header: "User", width:20, sortable: true, dataIndex: '{values.name}'},// renderer: renderUser},
{header: "Date", width:20, sortable: true, renderer: Ext.util.Format.dateRenderer('d.m.Y'), dataIndex: 'crDate'},
{header: "Time", width:20, sortable: true, dataIndex: 'crTime'},
{header: "Documents", width:20, sortable: true, dataIndex: 'documents', renderer: documentRender}
],
sm: new Ext.grid.RowSelectionModel({singleSelect:false}),
viewConfig:{
forceFit:true
},
stripeRows: true,
listeners: {
render: {
fn: function() {
grid.getSelectionModel().selectFirstRow()
}
}
}
});
} catch(exception) {
alert('getGridEl '+exception)
return null;
}
}


Safe solution for this would be great, thank you!

Best regards,
Aleksandar Cajic

[Condor]

This solution has absolutely nothing to do with which kind of store and reader you are using!
There must be something else wrong with your code.

ps.
1. It hasn't been mentioned in this thread yet, but by far the easiest solution is to add:

deferRowRender:false
to the grid config.
2. The solution mentioned in this feature request (http://extjs.com/forum/showthread.php?t=47119) would also work.

[sniffer]

Yes, there shouldn't be a problem which kind of store is in use for grid but it does, complete the same code with SimpleStore working fine with your row selection solution. I just change store source and there is a problem again with first row selection, so rest of code is identical.

I've check other solution but nothing.

[mjlecomte]

Yes, there shouldn't be a problem which kind of store is in use for grid but it does, complete the same code with SimpleStore working fine with your row selection solution. I just change store source and there is a problem again with first row selection, so rest of code is identical.

I've check other solution but nothing.

Did you use the provided code in the other thread as is to verify it didn't work?

The store you choose could affect the results in that simplestore the data is already present, whereas the other store you're going off to get the data from a url and then come back, etc. As mentioned in other thread some people are using a delay to allow some time for the code to catch up, etc. Anyway, I dislike using defer hence the introduction of an event that actually signifies when you can actually select the row.

Condor, if you think the feature request I posted qualifies for a bug maybe you can move that thread to bugs so others can bump it, otherwise only premium level can bump it at moment. In my previous posts no one seemed to think it qualified as a bug so I made the 'feature request' (I guess the thought was that adding defer/delay was acceptable way of doing things).

[Condor]

Condor, if you think the feature request I posted qualifies for a bug maybe you can move that thread to bugs so others can bump it, otherwise only premium level can bump it at moment. In my previous posts no one seemed to think it qualified as a bug so I made the 'feature request' (I guess the thought was that adding defer/delay was acceptable way of doing things).

But that would create a duplicate post. I think it would be better if you edited your feature request and added a link to this bugreport.

[mjlecomte]

Ok, you are right. I didn't actually read this thread to see what the concern was. I'm not sure if this is a complete dup, but yes, might get more attention if left where it is.

To the OP and your original post. To clarify, the row DOES get selected. The problem is that it gets unselected later (discussed in my thread(s) if you want to know why).

[sniffer]

Solution for JsonStore is to use callback on method load() and there to call selectFirstRow() method.

Like:


store.load({
callback: function() {
grid.getSelectionModel().selectFirstRow()
}
});

General issue is that loadData() working with Condor's solution without a problem but load() method has issue with that existing solution. So if you are using load() instead loadData() then use with load() method callback function.

[mjlecomte]

Sounds like you're just getting lucky to me. As said before it's local vs. remote data and timing just "happen" to work out.