Merge pull request #2261 from knockout/2261-checked-binding-fix

checked binding (3.5.0-pre)
knockout · Jul 19, 2017 · b58e2b6 · b58e2b6
2 parents 04a8f84 + fd4ed82
commit b58e2b6
Show file tree

Hide file tree

Showing 2 changed files with 74 additions and 67 deletions.
diff --git a/spec/defaultBindings/checkedBehaviors.js b/spec/defaultBindings/checkedBehaviors.js
@@ -392,69 +392,74 @@ describe('Binding: Checked', function() {
                 expect(testNode.childNodes[0]).toHaveCheckedStates([false, true]);
             });
 
-            it('Should use that value as the checkbox\'s value when not bound to an array', function () {
-                var myobservable = ko.observable('random value');
-                testNode.innerHTML = "<input type='checkbox' data-bind='checked:someProp, " + binding + ":true' />" +
-                    "<input type='checkbox' data-bind='checked:someProp, " + binding + ":false' />";
-                ko.applyBindings({ someProp: myobservable }, testNode);
-
-                expect(myobservable()).toEqual('random value');
-
-                // Check initial state: both are unchecked because neither has a matching value
-                expect(testNode).toHaveCheckedStates([false, false]);
-
-                // Update observable; verify element states
-                myobservable(false);
-                expect(testNode).toHaveCheckedStates([false, true]);
-                myobservable(true);
-                expect(testNode).toHaveCheckedStates([true, false]);
-
-                // "check" a box; verify observable and elements
-                testNode.childNodes[1].click();
-                expect(myobservable()).toEqual(false);
-                expect(testNode).toHaveCheckedStates([false, true]);
-
-                // "uncheck" a box; verify observable and elements
-                testNode.childNodes[1].click();
-                expect(myobservable()).toEqual(undefined);
-                expect(testNode).toHaveCheckedStates([false, false]);
-            });
-
-            it('Should be able to use observables as value of checkboxes when not bound to an array', function() {
-                var object1 = {id:ko.observable(1)},
-                    object2 = {id:ko.observable(2)},
-                    model = { value: ko.observable(1), choices: [object1, object2] };
-                testNode.innerHTML = "<div data-bind='foreach: choices'><input type='checkbox' data-bind='" + binding + ":id, checked:$parent.value' /></div>";
-                ko.applyBindings(model, testNode);
-
-                expect(model.value()).toEqual(1);
-                expect(testNode.childNodes[0]).toHaveCheckedStates([true, false]);
-
-                // Update the value observable of the checked item; should update the selected values and leave checked values unchanged
-                object1.id(3);
-                expect(model.value()).toEqual(3);
-                expect(testNode.childNodes[0]).toHaveCheckedStates([true, false]);
-
-                // Update the value observable of the unchecked item; should do nothing
-                object2.id(4);
-                expect(model.value()).toEqual(3);
-                expect(testNode.childNodes[0]).toHaveCheckedStates([true, false]);
-
-                // Update the value observable of the unchecked item to the current model value; should set to checked
-                object2.id(3);
-                expect(model.value()).toEqual(3);
-                expect(testNode.childNodes[0]).toHaveCheckedStates([true, true]);
-
-                // Update the value again; should leave checked and replace selected value (other button should be unchecked)
-                object2.id(4);
-                expect(model.value()).toEqual(4);
-                expect(testNode.childNodes[0]).toHaveCheckedStates([false, true]);
-
-                // Revert to original value; should update selected value
-                object2.id(2);
-                expect(model.value()).toEqual(2);
-                expect(testNode.childNodes[0]).toHaveCheckedStates([false, true]);
-            });
+            if (binding == "checkedValue") {
+                // When bound to a checkbox, the "checkedValue" binding will affect a non-array
+                // "checked" binding, but "value" won't.
+
+                it('Should use that value as the checkbox\'s value when not bound to an array', function () {
+                    var myobservable = ko.observable('random value');
+                    testNode.innerHTML = "<input type='checkbox' data-bind='checked:someProp, " + binding + ":true' />" +
+                        "<input type='checkbox' data-bind='checked:someProp, " + binding + ":false' />";
+                    ko.applyBindings({ someProp: myobservable }, testNode);
+
+                    expect(myobservable()).toEqual('random value');
+
+                    // Check initial state: both are unchecked because neither has a matching value
+                    expect(testNode).toHaveCheckedStates([false, false]);
+
+                    // Update observable; verify element states
+                    myobservable(false);
+                    expect(testNode).toHaveCheckedStates([false, true]);
+                    myobservable(true);
+                    expect(testNode).toHaveCheckedStates([true, false]);
+
+                    // "check" a box; verify observable and elements
+                    testNode.childNodes[1].click();
+                    expect(myobservable()).toEqual(false);
+                    expect(testNode).toHaveCheckedStates([false, true]);
+
+                    // "uncheck" a box; verify observable and elements
+                    testNode.childNodes[1].click();
+                    expect(myobservable()).toEqual(undefined);
+                    expect(testNode).toHaveCheckedStates([false, false]);
+                });
+
+                it('Should be able to use observables as value of checkboxes when not bound to an array', function() {
+                    var object1 = {id:ko.observable(1)},
+                        object2 = {id:ko.observable(2)},
+                        model = { value: ko.observable(1), choices: [object1, object2] };
+                    testNode.innerHTML = "<div data-bind='foreach: choices'><input type='checkbox' data-bind='" + binding + ":id, checked:$parent.value' /></div>";
+                    ko.applyBindings(model, testNode);
+
+                    expect(model.value()).toEqual(1);
+                    expect(testNode.childNodes[0]).toHaveCheckedStates([true, false]);
+
+                    // Update the value observable of the checked item; should update the selected values and leave checked values unchanged
+                    object1.id(3);
+                    expect(model.value()).toEqual(3);
+                    expect(testNode.childNodes[0]).toHaveCheckedStates([true, false]);
+
+                    // Update the value observable of the unchecked item; should do nothing
+                    object2.id(4);
+                    expect(model.value()).toEqual(3);
+                    expect(testNode.childNodes[0]).toHaveCheckedStates([true, false]);
+
+                    // Update the value observable of the unchecked item to the current model value; should set to checked
+                    object2.id(3);
+                    expect(model.value()).toEqual(3);
+                    expect(testNode.childNodes[0]).toHaveCheckedStates([true, true]);
+
+                    // Update the value again; should leave checked and replace selected value (other button should be unchecked)
+                    object2.id(4);
+                    expect(model.value()).toEqual(4);
+                    expect(testNode.childNodes[0]).toHaveCheckedStates([false, true]);
+
+                    // Revert to original value; should update selected value
+                    object2.id(2);
+                    expect(model.value()).toEqual(2);
+                    expect(testNode.childNodes[0]).toHaveCheckedStates([false, true]);
+                });
+            }
 
             it('Should ignore \'undefined\' value for checkbox', function () {
                 var myobservable = new ko.observable(true);

diff --git a/src/binding/defaultBindings/checked.js b/src/binding/defaultBindings/checked.js
@@ -7,11 +7,13 @@ ko.bindingHandlers['checked'] = {
             // Treat "value" like "checkedValue" when it is included with "checked" binding
             if (allBindings['has']('checkedValue')) {
                 return ko.utils.unwrapObservable(allBindings.get('checkedValue'));
-            } else if (allBindings['has']('value')) {
-                return ko.utils.unwrapObservable(allBindings.get('value'));
+            } else if (useElementValue) {
+                if (allBindings['has']('value')) {
+                    return ko.utils.unwrapObservable(allBindings.get('value'));
+                } else {
+                    return element.value;
+                }
             }
-
-            return useElementValue ? element.value : undefined;
         });
 
         function updateModel() {