4
\$\begingroup\$

I've written a custom binding for KnockoutJS which renders <select> elements with <optgroup> children.

When run, this binding adds <optgroup> and <option> DOM elements.

In order to make the binding two-way I obviously need to 'record' the observables for each of those newly-added DOM elements. I've done this using

ko.utils.domData.set(option, "data", thisOption);

and then in order to retrieve the observable (or plain object) I call

ko.utils.domData.get(node, "data");

This works fine. However, in Knockout's native bindings I can call ko.dataFor(element) and retrieve the observable, yet this doesn't work in my custom binding.

Can anyone tell me if I've gone about this the right/wrong way? And if wrong, how do I 'record' data that ko.dataFor can retrieve?

Furthermore, my custom binding accepts a parameter which is the property that the selected element's observable is assigned to (in exactly the same way that Knockout's native options binding uses the 'value' parameter). So in that same change-handler I retrieve this property using the same methodology as above - i.e. using ko.utils.domData...

Is this the correct approach for assigning a selected value to another observable in a change handler?

ko.bindingHandlers.groupedOptions = {
"init": function (element, valueAccessor, allBindings) {
 ko.utils.registerEventHandler(element, "change", function () {
 var value = valueAccessor(),
 property = ko.utils.domData.get(element, "property");
 ko.utils.arrayForEach(element.getElementsByTagName("option"), function(node) {
 if (node.selected) {
 var data = ko.utils.domData.get(node, "data");
 if (typeof(property) === "function") {
 property(data);
 } else if (typeof(property) === "string") {
 var vm = ko.dataFor(element);
 if (vm !== null) {
 vm[property] = data;
 }
 }
 }
 });
 });
},
"update": function(element, valueAccessor) {
 // Get the parameters
 var h = ko.utils.unwrapObservable(valueAccessor());
 var groups = h["groups"],
 groupsCollection,
 groupsLabel = "Label", // the convention for this property
 optionsCollProp = "Options", // the convention for this property
 optionsTextProp = "Text", // the convention for this property
 optionsValProp = "Value", // the convention for this property
 optionsValue = null;
 if (typeof (groups) === "undefined" || !groups) {
 throw "The \"groupedOption\" binding requires a \"groups\" object be specified.";
 } else {
 groupsCollection = groups["coll"];
 }
 if (!groupsCollection) {
 throw "The \"groupedOption\" binding's \"groups\" object requires that a collection (array or observableArray) be specified.";
 }
 if (typeof (groups["label"]) === "string" && groups["label"].length) {
 groupsLabel = groups["label"];
 }
 if (typeof (groups["options"]) === "object") {
 var options = groups["options"];
 if (typeof (options["coll"]) === "string" && options["coll"].length) {
 optionsCollProp = options["coll"];
 }
 if (typeof (options["text"]) === "string" && options["text"].length) {
 optionsTextProp = options["text"];
 }
 if (typeof (options["value"]) === "string" && options["value"].length) {
 optionsValProp = options["value"];
 }
 }
 var selectedItem = h["value"],
 selectedValue = ko.unwrap(selectedItem);
 if (typeof(selectedItem) === "function") {
 ko.utils.domData.set(element, "property", selectedItem); // this records the subscribing property, i.e., the property which stores the selected item
 } else if (typeof(selectedItem) === "string") {
 // this caters for the situation whereby the subscribing property is not an observable
 ko.utils.domData.set(element, "property", selectedItem); // this records the name of the subscribing property, i.e., the property which stores the selected item
 }
 // find how many elements have already been added to 'element'
 var childCount = 0,
 children = element.childNodes,
 childMax = children.length;
 for (var c = 0; c < childMax; c++) {
 if (children[c].nodeType != 3) {
 childCount++;
 }
 }
 // Default <option> element
 // if 'element' is currently empty then add the default <option> element
 if (!childCount) {
 var defaultText = h["optionsCaption"];
 if (defaultText && typeof(defaultText) === "string" && defaultText.length) {
 var defaultOption = document.createElement("option");
 defaultOption.innerHTML = defaultText;
 element.appendChild(defaultOption);
 }
 } else {
 // if 'element' is not empty then decrement realChildren by 1, which represents the default <option> element
 childCount--;
 }
 // now it's time to loop through each <optgroup>
 // in this loop, i is set to the the index in the collection which marks the start of the newly-added items, skipping items already added (which were counted above)
 var coll = ko.utils.unwrapObservable(groupsCollection);
 childMax = coll.length;
 for (; childCount < childMax; childCount++) {
 var groupLabel = ko.utils.unwrapObservable(coll[childCount][groupsLabel]);
 // if there is no label for this <optgroup> then don't add the <optgroup>
 if (!groupLabel || !groupLabel.length) {
 continue;
 }
 var optGroup = document.createElement("optgroup");
 optGroup.setAttribute("label", groupLabel);
 // loop through each <option>
 // determine whether the <option>s collection is an array or an observableArray
 var options = ko.utils.unwrapObservable(coll[childCount][optionsCollProp]);
 for (var j = 0, jMax = options.length; j < jMax; j++) {
 var thisOption = options[j],
 optionText = ko.utils.unwrapObservable(thisOption[optionsTextProp]);
 // if there is no text for this <option> then don't add the <option>
 if (!optionText || !optionText.length) {
 continue;
 }
 var option = document.createElement("option");
 option.innerHTML = optionText;
 // add the 'value' attribute if it exists
 var val = ko.utils.unwrapObservable(thisOption[optionsValProp]);
 if (val && val.length) {
 option.setAttribute("value", val);
 }
 // if this is the same object as the 'value' parameter then indicate so 
 if (thisOption === selectedValue) {
 option.setAttribute("selected", "selected");
 }
 // add the observable to this node so that we may retrieve this data in future
 ko.utils.domData.set(option, "data", thisOption);
 // now add this <option> to the parent <optgroup>
 optGroup.appendChild(option);
 }
 element.appendChild(optGroup);
 }
 return true;
}
};
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Feb 9, 2014 at 10:55
\$\endgroup\$
6
  • \$\begingroup\$ Furthermore, it sounds like your code is not working the way you want it to. Please only submit working code. \$\endgroup\$ Commented Feb 9, 2014 at 14:22
  • 1
    \$\begingroup\$ Ok, I've now added the JS in question. @konijn - yes it is working as expected. My question asks "if I've gone about this the right/wrong way?" and later "Is this the correct approach". What made you think it wasn't working? \$\endgroup\$ Commented Feb 9, 2014 at 16:12
  • \$\begingroup\$ @avi: 'yet this doesn't work in my custom binding.' \$\endgroup\$ Commented Feb 9, 2014 at 17:58
  • \$\begingroup\$ @konijn - my question is clearly asking for advice, not a solution. If you have nothing to suggest or no guidance to offer then you don't need to return to this post. \$\endgroup\$ Commented Feb 10, 2014 at 7:38
  • \$\begingroup\$ Just a note cause it was bothering me a bit reading your code typeof isn't a function and is usually written typeof x instead of typeof(x) \$\endgroup\$ Commented Feb 10, 2014 at 22:15

1 Answer 1

3
\$\begingroup\$

From a once over:

  • You use the following type of coding a ton:

    var groupsLabel = "Label";
    ...
    if (typeof (groups["label"]) === "string" && groups["label"].length) {
     groupsLabel = groups["label"];
    }
    

    You could consider using a helper function for that

    function getStringValue( s , defaultValue )
    {
     return (typeof s === "string" && s.length) ? s : defaultValue;
    }
    

    Then you can

     var groupsLabel,
     optionsCollProp, 
     optionsTextProp, 
     optionsValProp, 
     optionsValue, 
     groupsLabel = getStringValue( groups.label, "Label" );
     if (typeof (groups["options"]) === "object") {
     var config = groups["options"];
     optionsCollProp = getStringValue( config.coll , "Options" );
     optionsTextProp = getStringValue( config.text , "Text" );
     optionsValProp = getStringValue( config.value, "Value" ); 
     }
    

    Note that I also changed options["something"] to options.something which is the preferred style in JavaScript.

  • You use var options twice, the meaning of options is different in the 2 usages, I would use config instead the first time as in my counter proposal above this point.
  • You do not use var value = valueAccessor() as far as I can tell
  • You do not use optionsValue = null; as far as I can tell
  • Some variables are perhaps too Spartan vm -> viewModel ?
  • .nodeType != 3 <- This could use a line of comment, given the otherwise excellent level of commenting

As for your actual question, I see nothing wrong with your approach.

answered Feb 10, 2014 at 20:56
\$\endgroup\$
2
  • \$\begingroup\$ default is a reserved word by the way and you have some syntax errors like getStringValue( groups["label"]), "Label" ); \$\endgroup\$ Commented Feb 10, 2014 at 21:33
  • \$\begingroup\$ Those are great comments, @konijn. I've implemented them all except for the first one, regarding the getStringValue function: the problem here is, if the groups["options"] !== "object" then the default values are never set. groups["options"] is an optional parameter, so it could feasably be !== "object" and in that case the defaults wouldn't be set. But other than that, great suggestions - thanks. \$\endgroup\$ Commented Feb 11, 2014 at 9:42

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.