DataGrid : deleting a row does not notify grid.selection

[PEM]

In the _onDelete function of the DataGrid?, we should call this.selection.isSelected to test if the deleted row was selected or not, and if so, we should call this.selection.deselect to remove it so that when this.selection.getSelectedCount() is called, the deletedRow is no longer in the count. Problem exists in 1.6 to 1.7

[PEM]

Patch to notify the grid.selection when a selected row is deleted

[PEM]

Just wanted to say that this bug is also in 1.7.0b2 :)

[PEM]

Issue is also present in 1.7.0b4

[PEM]

any news ? will it be included in 1.7 release ? Is there any problem with the bug or patch provided ?

[Colin Snover]

In [26641]:

Ensure that deleted rows are deselected in DataGrid?. Fixes #13432. Thanks to Pierre-Emmanuel Manteau (DoYouSoft?, CCLA) for the patch.

[Simon Speich]

This fix does not work when deleting multiple selected items in a loop (because of shifting rows up but not updating grid.selection) and should be reopened:

Lets say I have rows 2 and 3 selected and I want to delete both. I call grid.selection.getSelected(), loop through the returned array and call store.deleteItem(item). The first item is correctly deselected, but the second item has now rowIndex of first item, but selection still has idx 3 as selected, since 2 got already deleted. The effect: Both items are deleted but row 3 is reselected (which was row 5)

[PEM]

Not sure the problem comes from the fix or even From the DataGrid?. I think the file Selection.js in the deselect function should also splice like the DataGrid?

	deselect: function(inIndex){
		if(this.mode == 'none'){ return; }
		if(dojo.isArray(inIndex)){
			dojo.forEach(inIndex, this.deselect, this);
			return;
		}
		inIndex = Number(inIndex);
		if(this.selectedIndex == inIndex){
			this.selectedIndex = -1;
		}
		if(this.selected[inIndex]){
			if(this.onCanDeselect(inIndex) === false){
				return;
			}
			var rowNode = this.grid.getRowNode(inIndex);
			if(rowNode){
				dojo.attr(rowNode,"aria-selected","false");
			}
			this._beginUpdate();


// here we should probably delete + splice, 
// because deleting only will leave us with an undefined at this index

			delete this.selected[inIndex];

// so maybe splice it, but haven't tested, 
// and i think it should be another trac report about Selection.js being bugged

this.selected.splice(inIndex, 1);


			//this.grid.onDeselected(inIndex);
			this.onDeselected(inIndex);
			//this.onSetSelected(inIndex, false);
			this._endUpdate();
		}
	},

[Evan]

In [26735]:

Fixes #13432, remove corresponding the item in grid.selection.selected[] when the row is deleted

[Evan]

Replying to PEM_FR:

Not sure the problem comes from the fix or even From the DataGrid?. I think the file Selection.js in the deselect function should also splice like the DataGrid?

	deselect: function(inIndex){
		if(this.mode == 'none'){ return; }
		if(dojo.isArray(inIndex)){
			dojo.forEach(inIndex, this.deselect, this);
			return;
		}
		inIndex = Number(inIndex);
		if(this.selectedIndex == inIndex){
			this.selectedIndex = -1;
		}
		if(this.selected[inIndex]){
			if(this.onCanDeselect(inIndex) === false){
				return;
			}
			var rowNode = this.grid.getRowNode(inIndex);
			if(rowNode){
				dojo.attr(rowNode,"aria-selected","false");
			}
			this._beginUpdate();


// here we should probably delete + splice, 
// because deleting only will leave us with an undefined at this index

			delete this.selected[inIndex];

// so maybe splice it, but haven't tested, 
// and i think it should be another trac report about Selection.js being bugged

this.selected.splice(inIndex, 1);


			//this.grid.onDeselected(inIndex);
			this.onDeselected(inIndex);
			//this.onSetSelected(inIndex, false);
			this._endUpdate();
		}
	},

We can't do that in selection.js since this logic only makes sense when a row is deleted. [26735] should fixed the issue.

[Jan Schatz]

Replying to Evan:

In [26735]:

Fixes #13432, remove corresponding the item in grid.selection.selected[] when the row is deleted

This fix by Evan does not seem to work for TreeGrids? since this.selection.selected is an object there and does not have the function splice. Anyway I don't understand the need of this change. The existing call to deselect() should do the job.