Skip to content

Commit

Permalink
Fix for Issue #4
Browse files Browse the repository at this point in the history
  • Loading branch information
AndersMalmgren committed Sep 22, 2013
1 parent 2b7c86f commit 7898311
Showing 1 changed file with 58 additions and 15 deletions.
73 changes: 58 additions & 15 deletions src/Knockout.BindingConventions.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,17 @@
var getDataFromComplexObjectQuery = function (name, context) {
var parts = name.split(".");
for (var i = 0; i < parts.length; i++) {
var value = context[parts[i]];
var part = parts[i];
var value = context[part];
if (i != parts.length - 1)
value = ko.utils.unwrapObservable(value);
else {
return {
data: value,
context: context,
name: part
};
}

context = value;
}
Expand All @@ -85,17 +93,23 @@
};

var setBindingsByConvention = function (name, element, bindingContext, bindings) {
var dataFn = bindingContext[name] ?
function() { return bindingContext[name]; } :
function() { return bindingContext.$data[name]; };
var data = dataFn();
var data = bindingContext[name] ? bindingContext[name] : bindingContext.$data[name];
var context = bindingContext.$data;

if (data === undefined) {
dataFn = function() { return getDataFromComplexObjectQuery(name, bindingContext.$data); };
data = dataFn();
var result = getDataFromComplexObjectQuery(name, context);
data = result.data;
bindingContext = { $data: result.context };
name = result.name;
}

if (data === undefined) {
throw "Can't resolve member: " + name;
}

var dataFn = function() {
return data;
};
var unwrapped = ko.utils.peekObservable(data);

This comment has been minimized.

Copy link
@mbest

mbest Sep 24, 2013

Contributor

With your changes here, you've broken true 3.0 support. The dataFn function must encapsulate the actual work of retrieving the data, including unwrapping any observables. Your code is now backwards.

This comment has been minimized.

Copy link
@AndersMalmgren

AndersMalmgren Sep 24, 2013

Author Owner

I see your point, but the old code didn't unwrap any observables in the dataFn? It's first on line 113

var unwrapped = ko.utils.peekObservable(data);

But since it uses peekObservable that wont hook up any dependencies. Its first when a convention binding handler uses the observable that it might hook up any dependencies. But I can revert it to your version so that I do not oversee any scenarios that might break (Tests are green though). Why I changed it was that your version created alot of closures which are a bit expensive on the performance.

Thanks for the feedback. I also have to fix a bug with the options binding that I introduced with this commit

This comment has been minimized.

Copy link
@AndersMalmgren

AndersMalmgren Sep 24, 2013

Author Owner

Hmm, getDataFromComplexObjectQuery does unwrap the observables, but I should change it to peekObservable because that code wont support that the path changes. I and do not think it should because it will be too complex. The devoloper should use the with convention instead in scenarios where the path can change during runtime

var type = typeof unwrapped;
var convention = element.__bindingConvention;
Expand Down Expand Up @@ -155,36 +169,45 @@
apply: function (name, element, bindings, unwrapped, type, dataFn, bindingContext) {
bindings.options = dataFn;
var selectedMemberFound = false;
var bindingName = null;
var selected = null;

singularize(name, function (singularized) {
var pascalCasedItemName = getPascalCased(singularized);
if (setBinding(bindings, 'value', "selected" + pascalCasedItemName, bindingContext)) {
bindingName = "value";
selected = setBinding(bindings, bindingName, "selected" + pascalCasedItemName, bindingContext);
if (selected) {
setBinding(bindings, 'enable', "canChangeSelected" + pascalCasedItemName, bindingContext);
selectedMemberFound = true;
return true;
}
});

if (selectedMemberFound) return;
if (!selectedMemberFound) {
var pascalCased = getPascalCased(name);
bindingName = "selectedOptions";
selected = setBinding(bindings, bindingName, "selected" + pascalCased, bindingContext);
setBinding(bindings, 'enable', "canChangeSelected" + pascalCased, bindingContext);
}

var pascalCased = getPascalCased(name);
setBinding(bindings, 'selectedOptions', "selected" + pascalCased, bindingContext);
setBinding(bindings, 'enable', "canChangeSelected" + pascalCased, bindingContext);
applyMemberWriter(bindings, bindingName, selected, name, bindingContext);
}
};

ko.bindingConventions.conventionBinders.input = {
rules: [function (name, element) { return element.tagName === "INPUT" || element.tagName === "TEXTAREA"; } ],
apply: function (name, element, bindings, unwrapped, type, dataFn, bindingContext) {
var bindingName = null;
if (type === "boolean") {
if (ko.utils.ieVersion === undefined) {
element.setAttribute("type", "checkbox");
}
bindings.checked = dataFn;
bindingName = "checked";
} else {
bindings.value = dataFn;
bindingName = "value";
}

bindings[bindingName] = dataFn;
applyMemberWriter(bindings, bindingName, dataFn, name, bindingContext);
setBinding(bindings, 'enable', "canChange" + getPascalCased(name), bindingContext);
}
};
Expand Down Expand Up @@ -324,6 +347,26 @@

return hasContentBeforeEndTag(node.nextSibling);
};

var applyMemberWriter = function (bindings, bindingName, accessor, memberName, bindingContext) {
if (!ko.isObservable(accessor())) {
getMemberWriters(bindings)[bindingName] = function (value) {
bindingContext.$data[memberName] = value;
};
}
};

var getMemberWriters = function(bindings) {
var propertyWriters = null;
if (bindings._ko_property_writers === undefined) {
propertyWriters = {};
bindings._ko_property_writers = function () {
return propertyWriters;
};
}

return propertyWriters;
};

var preCheckConstructorNames = function () {
var flagged = [];
Expand Down

0 comments on commit 7898311

Please sign in to comment.