PDA

View Full Version : [FIXED] Selectionmodel bug/logic error, selectionchange triggered when not needed



klodoma1
28 Nov 2012, 2:31 PM
Ext version tested:

Ext 4.1.1
Browser versions tested against:

Firefox
Description:

In case of "MULTI" selectiontype the selectionchange event is triggered even when it's not needed. In case of a grid for example, if you click on the same row, then a "deselect" and a "select"is triggered. Imo this is a bug, which is causing issues, as the "selection" has not changed at all!
Steps to reproduce the problem:
Execute the code bellow and click on the same row multiple times. You'll see that thes selectionchange event is triggered with every click 2 times, it's a deselect and select. Imo, of you click the same row, nothing should happen as a "new" selection is not happening.

The bug is in Ext.selection.Model definition in the selectWithEvent: function(record, e, keepExisting) function:
In case of "MULTI" if the current record is selected, it is selected again.

Test Case:


Ext.define('Company', {
extend: 'Ext.data.Model',
fields: ['name'],
});


// sample static data for the store
var myData = [
['3m Co'], ['Alcoa Inc'],['3m Co'], ['Alcoa Inc'],['3m Co'], ['Alcoa Inc']
];


// create the data store
var store = Ext.create('Ext.data.ArrayStore', {
model: 'Company',
data: myData
});


// create the Grid
var grid = Ext.create('Ext.grid.Panel', {
store: store,
stateful: true,
collapsible: true,
multiSelect: true,
stateId: 'stateGrid',
columns: [
{
text : 'Name1',
dataIndex: 'name'
},
{
text : 'Name2',
dataIndex: 'name'
}
],
height: 350,
width: 600,
title: 'Array Grid',
renderTo: document.body,
selModel : Ext.create('Ext.selection.RowModel', {
mode : 'MULTI',
listeners : {
selectionchange: function( selModel, selected, eOpts ){
console.log('selectionchange', selModel, selected, eOpts );
},
}
})
});



Possible fix:

Original code:

selectWithEvent: function(record, e, keepExisting) { var me = this;


switch (me.selectionMode) {
case 'MULTI':
if (e.ctrlKey && me.isSelected(record)) {
me.doDeselect(record, false);
} else if (e.shiftKey && me.lastFocused) {
me.selectRange(me.lastFocused, record, e.ctrlKey);
} else if (e.ctrlKey) {
me.doSelect(record, true, false);
} else if (me.isSelected(record) && !e.shiftKey && !e.ctrlKey && me.selected.getCount() > 1) {
me.doSelect(record, keepExisting, false);
} else {
me.doSelect(record, false);
}
break;
case 'SIMPLE':
if (me.isSelected(record)) {
me.doDeselect(record);
} else {
me.doSelect(record, true);
}
break;
case 'SINGLE':
// if allowDeselect is on and this record isSelected, deselect it
if (me.allowDeselect && me.isSelected(record)) {
me.doDeselect(record);
// select the record and do NOT maintain existing selections
} else {
me.doSelect(record, false);
}
break;
}

},

My suggestion:
The change is in the last else if / else in the "MULTI" case.
selectWithEvent: function(record, e, keepExisting) { var me = this;


switch (me.selectionMode) {
case 'MULTI':
if (e.ctrlKey && me.isSelected(record)) {
me.doDeselect(record, false);
} else if (e.shiftKey && me.lastFocused) {
me.selectRange(me.lastFocused, record, e.ctrlKey);
} else if (e.ctrlKey) {
me.doSelect(record, true, false);
} else if (me.isSelected(record) && !e.shiftKey && !e.ctrlKey && me.selected.getCount() > 1) {
me.doSelect(record, keepExisting, false);
} else if (!me.isSelected(record)){
me.doSelect(record, false);
} else {
//do nothing
}
break;
case 'SIMPLE':
if (me.isSelected(record)) {
me.doDeselect(record);
} else {
me.doSelect(record, true);
}
break;
case 'SINGLE':
// if allowDeselect is on and this record isSelected, deselect it
if (me.allowDeselect && me.isSelected(record)) {
me.doDeselect(record);
// select the record and do NOT maintain existing selections
} else {
me.doSelect(record, false);
}
break;
}

}

mitchellsimoens
28 Nov 2012, 7:55 PM
Thanks for the report. This should be fixed for the 4.2.0 release.

klodoma1
2 Dec 2012, 1:45 PM
Hi Mitchell,
From where could I download the 4.2.0?

Andrei.

mitchellsimoens
3 Dec 2012, 4:25 AM
4.2.0 has not been released yet.