#5724 closed defect (fixed)
Grid DojoData add/remove row broken
Reported by: | haysmark | Owned by: | benschell |
---|---|---|---|
Priority: | high | Milestone: | 1.1 |
Component: | DojoX Grid | Version: | 1.0 |
Keywords: | grid | Cc: | |
Blocked By: | Blocking: |
Attachments (3)
Change History (16)
comment:1 Changed 13 years ago by
Owner: | changed from sorvell to benschell |
---|
comment:2 Changed 13 years ago by
Status: | new → assigned |
---|
comment:3 Changed 13 years ago by
Component: | Dijit → DojoX Grid |
---|
Changed 13 years ago by
Attachment: | 5724.patch added |
---|
Fixes add/remove rows. Note, this patch includes fixes/enhancements from patch 5030.
comment:4 Changed 13 years ago by
Reviewed. There are some minor stylistic things to fix (I'll go ahead and fix them). The logic seems reasonable for the actions.
Tested on:
FireFox? 2.0.0.11: PASSED
IE 6: PASSED
Opera 9.2: PASSED
Safari B3: PASSED
Seamonkey 1.1.2: PASSED
Changed 13 years ago by
Attachment: | dojox.grid._data.model_20080208.patch added |
---|
Updated patch for style issues.
comment:5 Changed 13 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
comment:6 Changed 13 years ago by
Resolution: | fixed |
---|---|
Status: | closed → reopened |
Remove function passes arguments, not inRowIndexes to _removeItems on line 683. Should be:
this._removeItems(inRowIndexes);
CLA Manninen
comment:7 follow-up: 8 Changed 13 years ago by
To reproduce the abovementioned issue see: test_dojo_data_edit. "Remove (Store)" is broken. The first item gets removed instead of the selected ones.
comment:8 Changed 13 years ago by
Replying to guest:
To reproduce the abovementioned issue see: test_dojo_data_edit. "Remove (Store)" is broken. The first item gets removed instead of the selected ones.
Actually, this is as intended. That button removes the first item in the model directly from the store. This is in contrast to the other Remove button, which removes the selected item using the Grid's removeSelectedRows function. More specifically, the Remove (Store) button calls this:
removeItem = function() { // Removes the first item in the model from the store // Grid should reflect removal of the first item and items should be re-indexed jsonStore.deleteItem(dataModel.data[0].__dojo_data_item); }
Line 683 in model.js does indeed look odd as indicated above, however this is working properly. Items removed from the store (by using the "Remove (Store)" button) are removed from both Grids on the indicated test page, which is the expected behavior. I'm not seeing where the bug is.
comment:9 Changed 13 years ago by
I still argue that there's a bug here. "Remove (Store)" was maybe my mistake, but see "Remove". On the the page I mentioned the grid rows are in the beginning:
0 Afrika 1 Egypt 2 Kenya 3 Nairobi 4 Mombasa
Select 1 & 3 and click Remove - what you have now is:
0 Egypt 1 Kenya 2 Nairobi 3 Mombasa
Only Africa is gone.
Try also 1) refreshing the page, 2) selecting 1 & 3 and 3) clicking "Remove (Store)". You get "Removal [ [ 0]]" twice on FireBug? console and once again only the first item is gone.
comment:10 Changed 13 years ago by
The second item you listed is still as expected. The "Remove (Store)" button has nothing to do with the selected rows in the table, as it is interacting directly with the data store (which has no knowledge of the rows selected in the Grid). "Removal 0?" is printed twice, as each Grid receives the notification from the data store and removes the appropriate row.
Hoewever, I'll agree that the first behavior (selecting two rows and selecting 'Remove' only results in the first row being removed) is a bug, and I'll look into it. I think my folly here was not testing deleting multiple rows at once, as you pointed out.
Changed 13 years ago by
Attachment: | 5724-2.patch added |
---|
comment:12 Changed 13 years ago by
As suggested, this patch fixes the problem. Now, selecting multiple rows to be deleted will result in the correct rows being deleted from the current Grid.
comment:13 Changed 13 years ago by
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
I'm taking a stab at a whole bunch of Grid bugs. Re-assigning.