When building selectedValues, append to existing array instance rathe…

…r than replacing with a new instance (just for consistency with other code in that method) Also make spec slightly more demanding to prove it works with multiple selections
olachan · Jan 30, 2012 · bc739b6 · bc739b6
1 parent 6f29dc3
commit bc739b6
Show file tree

Hide file tree

Showing 2 changed files with 7 additions and 4 deletions.
diff --git a/spec/defaultBindingsBehaviors.js b/spec/defaultBindingsBehaviors.js
@@ -507,16 +507,17 @@ describe('Binding: Selected Options', {
         }
 
         var selection = new ko.observableArray([]);
-        testNode.innerHTML = "<select multiple='multiple' data-bind='selectedOptions:mySelection'><optgroup label='group'><option value='a'>a</option><option value='b'>b</option></optgroup></select>";
+        testNode.innerHTML = "<select multiple='multiple' data-bind='selectedOptions:mySelection'><optgroup label='group'><option value='a'>a-text</option><option value='b'>b-text</option><option value='c'>c-text</option></optgroup></select>";
         ko.applyBindings({ mySelection: selection }, testNode);
 
         value_of(selection()).should_be([]);
 
         setMultiSelectOptionSelectionState(testNode.childNodes[0].childNodes[0].childNodes[0], true);
         setMultiSelectOptionSelectionState(testNode.childNodes[0].childNodes[0].childNodes[1], false);
+        setMultiSelectOptionSelectionState(testNode.childNodes[0].childNodes[0].childNodes[2], true);
         ko.utils.triggerEvent(testNode.childNodes[0], "change");
 
-        value_of(selection()).should_be(['a']);
+        value_of(selection()).should_be(['a', 'c']);
     }
 });
 

diff --git a/src/binding/defaultBindings.js b/src/binding/defaultBindings.js
@@ -276,8 +276,10 @@ ko.bindingHandlers['selectedOptions'] = {
             var node = nodes[i];
             if ((node.tagName == "OPTION") && node.selected)
                 result.push(ko.selectExtensions.readValue(node));
-            else if (node.tagName == "OPTGROUP")
-                result = result.concat.apply(result, ko.bindingHandlers['selectedOptions'].getSelectedValuesFromSelectNode(node));
+            else if (node.tagName == "OPTGROUP") {
+                var selectedValuesFromOptGroup = ko.bindingHandlers['selectedOptions'].getSelectedValuesFromSelectNode(node);
+                Array.prototype.splice.apply(result, [result.length, 0].concat(selectedValuesFromOptGroup)); // Add new entries to existing 'result' instance
+            }
         }
         return result;
     },