Sorting an observableArray messes up the "checked" binding

Question

I have a viewmodel with an observable array of child viewmodels.

These should be user-sortable, so I'm using the sort() method of the observableArray:

function ViewModel() {
    this.items = ko.observableArray([/* ... some items ... */]);
    this.sort = function () {
        this.items.sort(function (a, b) {
            // comparison
        }
    }
}

Each item has a type observable, bound to a list of radio boxes via the checked binding.

<li>
    <input type="radio" value="A" data-bind="checked: type" /> A
    <input type="radio" value="B" data-bind="checked: type" /> B
    <input type="radio" value="C" data-bind="checked: type" /> C
    <input type="radio" value="D" data-bind="checked: type" /> D
</li>

The phenomenon is that when the items array is sorted, the radio boxes randomly lose their state, i.e. no dot is visible on the screen for a seemingly random sub-set of them - even though their value still is maintained in the viewmodel.

Try it yourself: http://jsfiddle.net/yxg53bph/ - click "Sort" a few times and observe the effect.

Simple question: What's wrong here and how do I fix it?

Calling notifySubscribers() on the individual items after the sort fixes the display problem. However that's a hack, not a solution.

An interesting observation - if I use not random sort, but `return b.i - a.i;`, where `a` and `b` are parameters of sorting function, selected values disappear for 0-9 radios [always](http://jsfiddle.net/iluzyanin/yxg53bph/1/). Maybe this can lead somewhere? P.S. I've tried with 30 values, and it hided 15 radios. Exactly the half both times. — Ilya Luzyanin, Sep 18 '14 at 13:44
Well at least it's not seemingly random anymore. ;-) If you ask me, that smells like a genuine bug in knockout. — Tomalak, Sep 18 '14 at 13:53
Giving each input a unique name: `name: 'type_0'..name: 'type_1'...` results in a single random item being lost each time, [see here](http://jsfiddle.net/yxg53bph/6/) — Tanner, Sep 18 '14 at 14:26
Just did as @manji did and set `id` instead of `name` and it worked: http://jsfiddle.net/yxg53bph/7/ — Tanner, Sep 18 '14 at 14:29
Ironic that the one detail that I left out in the above code sample for the sake of brevity triggers the misbehavior. — Tomalak, Sep 18 '14 at 14:37
This reminds me of what I observed on [this question](http://stackoverflow.com/questions/25838821). You're messing with an element's identity (name in this case) and knockout will try to be efficient and reuse dom elements to minimize rendering time. And (this is pure conjecture) browsers are not expecting identity changes will not update the state of the element when it does. When you `notifySubscribers`, knockout is just reapplying the data binding to the element. The actual dom element stays in place. — Jeff Mercado, Sep 18 '14 at 15:49
@Jeff The checkbox `name` is *not* its ID. Names can legally be duplicated, using them for element identification is plain wrong. If knockout did this, this would absolutely qualify as a fat bug. — Tomalak, Sep 18 '14 at 15:53
My point was that the name of the element (for the purposes of identifying the group the input was in) was changing and the browser loses track of the state of the elements as the items are rendered. But that doesn't matter now, I think Ilya's comment makes perfect sense. — Jeff Mercado, Sep 18 '14 at 16:08
@Jeff Definitely. It also perfectly explains the strange "half of the items are affected" quirk. Good thing that having a name on the checkbox group is not necessary in knockout, I just did it out of habit (or OCD, if you will). — Tomalak, Sep 18 '14 at 16:18

score 3 · Accepted Answer · answered Sep 18 '14 at 14:27

3

In your jsfiddle, you set the name attribute and that's what causes this buggy behavior.

I don't think it's a bug in knockout. It's just that when sorting, radio items will be concurrently set by knockout handler and the browser itself (because radio group can have only 1 radio button checked).

If you remove the name attribute (and that is ok, because all 4 radio buttons are bound to the same field, the only 1 ckecked constrraint will be respected), the sort will run as expected.

Demo: jsfiddle

answered Sep 18 '14 at 14:27

manji

47,442
5
96
103

I still think it's a bug, because knockout's purpose is to govern the browser's behavior. It should be completely irrelevant how the browser initializes the checkbox group. And it is, when you initialize the elements the first time (i.e., create an `item`) it works as expected. It breaks when you sort the parent array, and it doesn't even break consistently. Having a named group is valid HTML and should - even though it's not strictly necessary - not affect how knockout updates its bindings. But I can confirm that things start to work in my app when I remove the `name` attribute binding. – Tomalak Sep 18 '14 at 14:34
1

The problem is (I guess) in mixing `name` with `$index()` - once you sort your array, the order changes thus changing names in each group, but during this process some radio already have this name and different groups mix, so I guess this is not a bug but complex behavior (I just don't see how knockout would foresee such things...). – Ilya Luzyanin Sep 18 '14 at 15:18
@IlyaLuzyanin, you're right, using `i` (a field of the bound viewmodel) instead of `$index()` fixes the problem. – manji Sep 18 '14 at 15:34
@IlyaLuzyanin *That's* the explanation. Thank you. – Tomalak Sep 18 '14 at 15:56

Sorting an observableArray messes up the "checked" binding

1 Answers1