Opened 11 years ago

Closed 11 years ago

Last modified 10 years ago

#7854 closed enhancement (wontfix)

[patch]DataGrid 1.2 : newItem not necessarily reflected

Reported by: rdunklau Owned by: Nathan Toone
Priority: low Milestone: future
Component: DojoX Grid Version: 1.2.0
Keywords: Cc: nonken, jared_j, Kris Zyp
Blocked By: Blocking:

Description

Setup :

dojox/grid/tests/test_edit_dijit.html with some modification to the UI addRow action :

addRow = function(){
   test_store.newItem({
     id: grid.rowCount,
     col1: 'normal',
     col2: true,
     col3: 'new',
     col4: 'Now is the time for all good men to come to the aid of  their party.',
     col5: 99.99,
     col6: 9.99,
     col7: false,
     col8: (new Date()).getTime()
   });
}

Then, click on the addRow button, sort by the "Mark column" descending, add a new row, then scroll the grid all way down : the second line has not been added, because the grid was fetching items in the sort order so a line with col2 field set to true has nothing to do in the last lines.

What i can't explain, is why is the line rendered if you scoll down the whole grid before rendering it.

I think we need a way to take account the sorting order in the onNew event, or (better) to allow the datastore to specify the index where the item was added, if it is sorted. That will allow two things : consistent add operation with the current sort order, and possibility to implement index inserting by a datastore user.

I see two possibilities for this :

  • define a "Sorted" interface for Datastores to conform, if needed, containing methods such as newItem(item,index), getItem(index),moveItem(item, index) etc...
  • generalize the "parent info" optional argument in the onNew method from notification API to allow the datastore to pass the index to insert to the grid. I don't know what this argument is used for, but i think it would be consistent to add an optional index parameter to this parentinfo object.

What do you think about it ?

(Sorry, it is as much an enhancement request as a defect, so i did not know how to file it. Maybe i should create another ticket ? )

Attachments (2)

DataGrid_Insert.patch (1.0 KB) - added by rdunklau 11 years ago.
test_edit_dijit.html (4.2 KB) - added by rdunklau 11 years ago.

Download all attachments as: .zip

Change History (13)

Changed 11 years ago by rdunklau

Attachment: DataGrid_Insert.patch added

Changed 11 years ago by rdunklau

Attachment: test_edit_dijit.html added

comment:1 Changed 11 years ago by rdunklau

comment:2 Changed 11 years ago by rdunklau

The change doesnot affect test_edit_dijit.html, since it uses an ItemFileWriteStore?, which doenot mixin all parentinfo properties in newItem method.

I did not include a patch, since my ticket only concerns the grid. DataStores? implementation should choose to take this index property into account , or not.

(I've sent a CLA this morning, but tell me if i must submit it with my contribution).

comment:3 Changed 11 years ago by nonken

Cc: nonken added

comment:4 in reply to:  3 Changed 11 years ago by rdunklau

Replying to nonken: Anything new about it ?

comment:5 Changed 11 years ago by Nathan Toone

Owner: changed from Bryan Forbes to Nathan Toone

Reassigning to me

comment:6 Changed 11 years ago by Nathan Toone

Milestone: 1.3future

comment:7 Changed 11 years ago by Nathan Toone

Summary: DataGrid 1.2 : newItem not necessarily reflected[patch]DataGrid 1.2 : newItem not necessarily reflected

Have you submitted a CLA?

comment:8 Changed 11 years ago by Nathan Toone

Cc: jared_j added
Priority: highlow

The data api doesn't have a mechanism for specifying an "insert index" - that issue should probably be addressed before trying to come up with a solution in the grid. I know it's been discussed in the past, but I don't know what was decided upon.

comment:9 Changed 11 years ago by Nathan Toone

Resolution: wontfix
Status: newclosed

Closing as wontfix for now - have not heard back on the cla, and we need to figure out limitations around the datastore...

comment:10 Changed 10 years ago by Kris Zyp

Jared and I have discussed this at some length, and the API design of the dojox.data.ClientFilter? was specifically intended to provide a very simple generic interface for widgets to update in these type of situations. This is more than a just a sorting issue, many times the grid may be showing a view of data that will exclude some of the new items that are created, or updated items may become move in and out of the set of items that should be displayed. The interface is such that widgets can simple check to see if store.updateResultSet and if so, it can update an array of items using a request arguments:

if(store.updateResultSet){

arrayOfItems = store.updateResultSet(arrayOfItems, requestArgs);

}

The ClientFilter? was implemented with the intention that the grid could be updated to utilize updateResultSet as an opportunity to try to move forward with addressing this issue to see if it works before creating a standard dojo.data.api.Updateable interface.

comment:11 Changed 10 years ago by Kris Zyp

Cc: Kris Zyp added
Note: See TracTickets for help on using tickets.