Binding checkboxes to writeable computedobservable array

[mbest]

I found an issue when binding a list of checkboxes to a computedobservable. It thinks that push and splice are available in addOrRemoveItem which they are not. It is fixed with this code. Could you please implement it:

addOrRemoveItem: function(array, value, included) {
var existingEntryIndex = ko.utils.arrayIndexOf(ko.utils.peekObservable(array), value);
if (existingEntryIndex < 0) {
if (included) {
// Original: array.push(value)
var arr = array();
arr.push(value);
array(arr);
}
} else {
if (!included) {
// Original: array.splice(existingEntryIndex, 1);
var arr = array();
arr.splice(existingEntryIndex, 1);
array(arr);
}
}
},

[brianmhunt]

@oscaruribe Sorry, but it is not apparent to me what use case you are demonstrating – a jsFiddle would be of some assistance.

Speculating, are you perhaps passing to addOrRemoveItem a computed observable whose value is not an array?

Some unit tests would be necessary to accompany a pull request with a proposal like the above, though I think a custom addOrRemoveItem to suit your needs may be preferable -- but please do share use cases that you think ought to be accounted for.

Cheers

[oscaruribe]

Sorry. Given this viewModel (redacted):

function UserViewModel() {

var self = this;
self.User = ko.observable(); // This property has an observableArray called Committees which represent the Users selected committees
self.Committees = ko.observableArray(); // This is the observable with all possible options
... 

self.UserCommitteeIds = ko.computed({
        read: function () {
            if (!self.User())
                return [];

            var ids = ko.utils.arrayMap(ko.mapping.toJS(self.User().Committees()), function (item) {
                return item.CommitteeId;
            });
            console.log('returning ids', ids);
            return ids;
        },
        write: function (ids) {
            var items = ko.utils.arrayMap(ids, function (id) {
                return ko.utils.arrayFirst(self.Committees(), function (item) {
                    return item.CommitteeId == id;
                });
            });
            self.User().Committees(items);
        }
    });
}

Ok so the viewmodel has a User observable which has an observableArray called Committees.

I've found that for select and checkboxes it's best to bind on simple ids so the markup looks like this:

                        </div>

This works perfect for select:

but with the checkbox case adding/removing items caused an error on line 186 (addOrRemoveItems) since it assumes the array parameter is an observableArray (which extends push/splice) something which a computedObservable does not.

So therefor my quickfix was to read the observable, push/splice and then write it in it's entirety which should work for observableArrays as well as computedObservables but might be less optimal in performance perhaps (I haven't looked at the observableArray.push/splice implementation).

[oscaruribe]

Ok did a quick fiddle for it: http://jsfiddle.net/tgh6k99r/2/

As you see in the fiddle it works great for select but crashes for checkboxes. My code fixes that.

[brianmhunt]

Thanks @oscaruribe - will have a look. By the way, if you wrap the code in your comments in ```, your code will look better (syntax highlighting, monospace font)

[brianmhunt]

Okay, @oscaruribe -- I see your problem. I am not confident enough with the innards and use of addOrRemoveItem to suggest what change to the core might be appropriate here, but here is a workaround that might suit you, here on jsFiddle.

The gist is to have a computed that reciprocates writes with an observableArray, and bind the observableArray to the DOM (instead of the computed). i.e.

    self.UserCommitteeIds = ko.computed({ read: ..., write: ... });
    self.UserCommitteeIdsArr = ko.observableArray()
    self.UserCommitteeIds.subscribe(self.UserCommitteeIdsArr);
    self.UserCommitteeIdsArr.subscribe(self.UserCommitteeIds);

Whenever the UserCommitteeIds changes it will propagate those to the UserCommitteIdsArr, and vise versa.

Mind your memory management, of course.

It is a couple lines of extra code, a couple subscriptions, but avoids having to change the KO core. (It's also a good technique to have in your back pocket).

Does that help?

[oscaruribe]

:) It works but of course it would be pretty nice if it was in the core don't you think?

Especially since options/selectedOptions works perfectly even with computed observables.

[brianmhunt]

I definitely agree that it would be good if the computed observables would work with checkboxes. I do have a concern about the performance for the suggested solution (copying an entire array anytime one item changes), and I have a suspicion that there might be side effects here with the array mutation events.

That said, someone smarter than me may be able to sort out an ideal solution. :)

[oscaruribe]

I look forward to seeing it work in an upcoming version:) As I mentioned just copy the same logic from selectedOptions to checked and it should work as expected.

[Roarster]

I believe I'm seeing the same problem as this. I've got a 2 dimensional matrix of data (two observable arrays) with every option represented by a checkbox. This works fine but I also want a select all checkbox for each row and column that will both show whether all corresponding items are selected and allow the selection/deselection of all items in the row/column. A writeable computed observable seemed the obvious solution to me but when the checked binding tries to write it attempts to call push or splice on the computed assuming that it's an array which it isn't.

I have an example of this here: http://jsfiddle.net/3485g8w6/2/ - the computed will accurately show whether all items in the column are selected but fails when attempting to check one of the header checkboxes. There are other solutions to this sort of problem but they are all far less graceful than the writeable computed solution.

[mbest]

@Roarster, the solution, at least for now, is to use a boolean computed observable for each column.

[Roarster]

Thanks for the reply @mbest though I'm not sure I understand what you are saying. In reality my lists of columns and rows are dynamic (hence the observable arrays) so I guess I'd need to dynamically generate the boolean computed observables to match for your solution. Also, I'm not clear how I would update the value of the other inner checkboxes with this approach which is part of what I need.

Anyway, I've got a solution I'm using involving detecting changes manually, it's just not as neat as it would be if the checked binding handled writing to computed observables backed by arrays. I've only added to this issue just to give another example of this problem and one I'd imagine being fairly common.

[mbest]

Thanks for the feedback. It's definitely possible to make the checked binding update writable computed observables. I think we can take a look at this for the next version.

[mbest]

I've attached a possible fix for this problem to this issue. Let me know what you think.