Thanks. Merged.

Tony

tony said:

Again me,

This is not correct fix. The correct fix should be in multiselect plugin – i.e when we add a new visible column (whitout reordering) the select list should be updated.

Tony

Tony,

I've just pushed a fix. I've overridden the ui.multiselect's _setSelected method to keep the options array sorted correctly.

Mark

Looking deeper in your fix I will say, that it is better than my.

I have merged the fix. I do not have do all the tests, but I think during the release we will polish it.

About cache!?! Cache IMHO should be applied with care and I do not want implement it currently in the grid.

I think that all is ok with the fix – suppose that the most grids contain 5-30 columns, this will not impact too much the performance, looping only once per request throght the array.

As your said – the most heavy things are when we do with  DOM manipulation.

Next week (if all goes ok) I will do more on the grid as promised. It is pleasure for me to work with you.

Tony

Regarding speed - I completely agree, and I think your efforts have certainly paid off; jqGrid is very fast.

But the cost of a lookup in an array of length "n", is completely dwarfed by the cost of fetching a number of lines of length "n" from a remote server, creating table rows from them, and inserting them into the DOM. I think we can afford to do the lookup once per fetch...

Also, for things like this (if performance /was/ an issue), you could cache the last value and then check to see if the cached result is valid - and get the performance but keep things dynamic...

Mark

Thanks for iii) and ii) .

Also you are right, in my solution is not possible to make these dynamic (as before).

My primary goal (as of the whohle grid) was speed, but it seems that in this case we can not implement my fix.

Will test your fix tomorrow and make a merge.

Thanks again.

Tony

No problem. But I see a couple of issues with your solution:

i) you can no longer change xmlReader.id, jsonReader.id, xmlmap or jsonmap after the grid is created

ii) it doesnt handle the case of ts.p.xmlReader.id is an integer, and repeatitems is true.

iii) there's a bug in the xmlreader:  if(f && ts.p.keyIndex[1]) idn = ts.p.keyIndex[1];
That should be if (f.length...) since f is always true. This is in part related to ii), but fixing this doesnt fix ii).

Mark

Sorry that I do not see your last message. With my last post I mean, that I will fix this.

I pushed my solution in the GitHub. I have made allmost all tests, but not the full ones.

Will see and test your solution. We should at end put one.

Sorry again.

Tony

I've made a small change to the xml reader behavior - if the id is a number, it uses it as the column index (just like the json reader).

Otherwise, I think the behavior is as before (except that id's are now correct after reordering with repeatitems:false, and a numeric id, or a "key" field).

There are a lot of cases - Im not sure Ive tested them all...

Mark

"reader" returns the items in the current order. So for repeatitems false, the index is looked up incorrectly.

Should be easy to fix - I'll take a look tonight (if you havent fixed it first).

Mark

I'm just looking into the code how to fix this.

Just you can see in Fire bug what are the id after reordering and clicking on next page.

Still not seeing it… the rowid is fetched from the row /as returned by the server/, so doesnt (and shouldnt) use orderedCols.

My example is almost identical to yours (language aside!) and Im not having any issues…

Mark

Here is the example:

http://www.trirand.com/test/jsonmap.html

1. Select row

2. click Get id link ( the value from first column is alerted)

3. reorder columns so that the Inv No is not first.

4. Click next page.

5. Select row and click Get id link - the value of the first column is alerted

Here is the config:

After reordering I get wrong id

Sorry for edit.

I mean after pagging the coulmns are ok, but the id is wrong

Im having trouble reproducing the problem - Im not even sure what the problem is.

One thing to note is that the id in the json reader (or xml reader) is not used when you set "key":true on a column (setting key takes priority), so your step 2 does nothing - unless that's the bug you're referring to? But in that case, its not related to the column reordering...

Mark

To reproduce:

1. Set in colModel the property for some column key:true. If the column is hidden and is first all is ok, but the problem appear when it is visible.

2. Json reader repeatitems : true or reapeatitems:false  and the id is set as number – something like

jsonReader : {repeatitems: true(false), id:”0″}, where id :”0″, point to the first column.

Tony

Re: ui.multiselect. Yes, there's a bug. It looks like there is an "experimental" version in github which has fixed this, but also introduces dependencies on a couple of new plugins.

Re: repeatitems false. I think I do see /a/ problem - if the xml reader has its index specified by ts.p.xmlReader.id, then it tries to look it up by name, even if the index is numeric; but Im pretty sure I didnt change that behavior (ie its nothing to do with column reordering).

I did find a bug in addXMLData that I introduced for the repeatitems true case: if the server doesnt return all the columns (ie, it just returns the first few), then it errors out. The fix is in gitHub.

Mark