search

coderenaissance / knockout.viewmodel

The knockout viewmodel plugin is the fastest, smallest, cleanest, most flexible way to create a knockout viewmodel.
http://coderenaissance.github.com/knockout.viewmodel
106 stars 28 forks source link

Array sort order is not kept when arrayChildId is set #47

Open paolocosta opened 10 years ago

paolocosta commented 10 years ago

See the example:

http://jsfiddle.net/wVGZc/1/

If you click on change page a new list is loaded but the sort is inverted. If the arrayChildId option is removed it works fine.

spring1975 commented 10 years ago

To my understanding, the "array id" doesn't double as what array.sort() should use as the sorting key. The results of your example are misleading. It's not sorting the array when you update the viewmodel at all. See example:

http://jsfiddle.net/spring1975/SrVWJ/1/

If you want the array of custom objects like this, you need to add a custom sorting function. See example on knockoutjs.com http://knockoutjs.com/documentation/observableArrays.html#pop_push_shift_unshift_reverse_sort_splice

On Sun, Oct 6, 2013 at 12:46 PM, paolocosta notifications@github.comwrote:

See the example:

http://jsfiddle.net/wVGZc/1/

If you click on change page a new list is loaded but the sort is inverted. If the arrayChildId option is removed it works fine.

— Reply to this email directly or view it on GitHubhttps://github.com/coderenaissance/knockout.viewmodel/issues/47 .

paolocosta commented 10 years ago

I don't know if you understood the problem.

I don't need to sort the data, that is already sorted as it come from the server (simulated by the variables). Only I need the items to be displayed in the correct order, and this doesn't happen when you set the arraychildId option.

You can see here http://jsfiddle.net/Kake7/ the order of the items is kept, as the arrayChildId option is not set anymore.

spring1975 commented 10 years ago

So the problem isn't that you can't sort them, but by setting the arrayChildId option, knockout.viewmodel is changing the order from what it was given whether that be reversed or whatever. The expected behavior is that it won't change the order from what it was given in any way. Correct?

On Tue, Oct 8, 2013 at 8:26 AM, paolocosta notifications@github.com wrote:

I don't know if you understood the problem.

I don't need to sort the data, that is already sorted as it come from the server (simulated by the variables). Only I need the items to be displayed in the correct order, and this doesn't happen when you set the arraychildId option.

You can see here http://jsfiddle.net/Kake7/ the order of the items is kept, as the arrayChildId option is not set anymore.

— Reply to this email directly or view it on GitHubhttps://github.com/coderenaissance/knockout.viewmodel/issues/47#issuecomment-25900024 .

paolocosta commented 10 years ago

Yes, that's correct.

To implement a paging functionality this is the required behaviour. And the possibility to define a key is really important

Paolo

paolocosta commented 10 years ago

Any news about this bug?

istr commented 10 years ago

Is there anybody out there with a patch for this issue? Keeping order is a relevant property; e.g. the data from a server may be pre-sorted and that op might be really expensive.

FlushablPet commented 9 years ago

Bumping this in case if anyone has been able to work out a solution, this is the only bug that affects usability / UX.

nmehlei commented 9 years ago

As this is beginning to seriously affect my project, I had to begin working on it. I tried to do it "right", but, at least for the moment, I saw no easy way to maintain or fix the ordering w/o spamming notifications of the observable array (because of intermittent pushFromModel invocations) and I was running out of time, so I committed an experimental version to an own fork: https://github.com/nmehlei/knockout.viewmodel .

I think it works, a mini test worked at least, but it's only build for correctness not efficiency so I don't think it will have good performance characteristics when the element count goes up. Maybe this can be helpful for others or be a starting point for someone else who wants to fix this, since it doesn't look like the original maintainer is coming back :/

cezarypiatek commented 8 years ago

@nmehlei I've created another fork and simplified your solution. Unfortunately I've not time to create appropriate test to proof that fix. You can try to run your own test suit against my code to check it's correctness. I've also fix refreshing bindings by sorting viewModel (sorting unwrapped list didn't affect DOM) https://github.com/cezarypiatek/knockout.viewmodel/commit/09a6071c82344d2182ce38076aa84577b8746619