Skip to content

Commit

Permalink
In IE<=8, delay deletion of options until after new items are added t…
Browse files Browse the repository at this point in the history
…o fix a selection bug.
  • Loading branch information
mbest committed Nov 22, 2013
1 parent 511d082 commit 5efcd94
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 1 deletion.
28 changes: 28 additions & 0 deletions spec/defaultBindings/optionsBehaviors.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,34 @@ describe('Binding: Options', function() {
expect(testNode.childNodes[0]).toHaveSelectedValues(["B"]);
});

it('Should select first option when removing the selected option and the original first option', function () {
// This test failed in IE<=8 without changes made in #1208
testNode.innerHTML = '<select data-bind="options: filterValues, optionsText: \'x\', optionsValue: \'x\'">';
var viewModel = {
filterValues: ko.observableArray([{x:1},{x:2},{x:3}])
};
ko.applyBindings(viewModel, testNode);
testNode.childNodes[0].options[1].selected = true;
expect(testNode.childNodes[0]).toHaveSelectedValues([2]);

viewModel.filterValues.splice(0, 2, {x:4});
expect(testNode.childNodes[0]).toHaveSelectedValues([4]);
});

it('Should select caption by default and retain selection when adding multiple items', function () {
// This test failed in IE<=8 without changes made in #1208
testNode.innerHTML = '<select data-bind="options: filterValues, optionsCaption: \'foo\'">';
var viewModel = {
filterValues: ko.observableArray()
};
ko.applyBindings(viewModel, testNode);
expect(testNode.childNodes[0]).toHaveSelectedValues([undefined]);

viewModel.filterValues.push("1");
viewModel.filterValues.push("2");
expect(testNode.childNodes[0]).toHaveSelectedValues([undefined]);
});

it('Should trigger a change event when the options selection is populated or changed by modifying the options data (single select)', function() {
var observable = new ko.observableArray(["A", "B", "C"]), changeHandlerFireCount = 0;
testNode.innerHTML = "<select data-bind='options:myValues'></select>";
Expand Down
13 changes: 12 additions & 1 deletion src/binding/defaultBindings/options.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@ ko.bindingHandlers['options'] = {

var unwrappedArray = ko.utils.unwrapObservable(valueAccessor());
var includeDestroyed = allBindings.get('optionsIncludeDestroyed');
var arrayToDomNodeChildrenOptions = {};
var captionPlaceholder = {};
var captionValue;

var previousSelectedValues;
if (element.multiple) {
previousSelectedValues = ko.utils.arrayMap(selectedOptions(), ko.selectExtensions.readValue);
Expand Down Expand Up @@ -88,6 +90,15 @@ ko.bindingHandlers['options'] = {
return [option];
}

// By using a beforeRemove callback, we delay the removal until after new items are added. This fixes a selection
// problem in IE<=8. See https://github.com/knockout/knockout/issues/1208
if (ko.utils.ieVersion <= 8) {
arrayToDomNodeChildrenOptions['beforeRemove'] =
function (option) {
element.removeChild(option);
};
}

function setSelectionCallback(arrayEntry, newOptions) {
// IE6 doesn't like us to assign selection to OPTION nodes before they're added to the document.
// That's why we first added them without selection. Now it's time to set the selection.
Expand All @@ -109,7 +120,7 @@ ko.bindingHandlers['options'] = {
}
}

ko.utils.setDomNodeChildrenFromArrayMapping(element, filteredArray, optionForArrayItem, null, callback);
ko.utils.setDomNodeChildrenFromArrayMapping(element, filteredArray, optionForArrayItem, arrayToDomNodeChildrenOptions, callback);

// Determine if the selection has changed as a result of updating the options list
var selectionChanged;
Expand Down

0 comments on commit 5efcd94

Please sign in to comment.