Opened 8 years ago

Closed 8 years ago

Last modified 6 years ago

#13432 closed defect (fixed)

DataGrid : deleting a row does not notify grid.selection

Reported by: PEM Owned by: Evan
Priority: high Milestone: 1.7
Component: DojoX Grid Version: 1.7.0b1
Keywords: Cc:
Blocked By: Blocking:

Description

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

Attachments (1)

DataGrid.diff (310 bytes) - added by PEM 8 years ago.
Patch to notify the grid.selection when a selected row is deleted

Download all attachments as: .zip

Change History (11)

Changed 8 years ago by PEM

Attachment: DataGrid.diff added

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

comment:1 Changed 8 years ago by PEM

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

comment:2 Changed 8 years ago by PEM

Issue is also present in 1.7.0b4

comment:3 Changed 8 years ago by Evan

Owner: changed from evan to Evan

comment:4 Changed 8 years ago by PEM

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

comment:5 Changed 8 years ago by Colin Snover

Resolution: fixed
Status: newclosed

In [26641]:

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

comment:6 Changed 8 years ago by 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)

comment:7 Changed 8 years ago by 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();
		}
	},

comment:8 Changed 8 years ago by Evan

In [26735]:

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

comment:9 in reply to:  7 Changed 8 years ago by Evan

Milestone: tbd1.7

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.

comment:10 in reply to:  8 Changed 6 years ago by 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.

Note: See TracTickets for help on using tickets.