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;
}
};
\$\endgroup\$
  • \$\begingroup\$ Furthermore, it sounds like your code is not working the way you want it to. Please only submit working code. \$\endgroup\$ – konijn Feb 9 '14 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\$ – awj Feb 9 '14 at 16:12
  • \$\begingroup\$ @avi: 'yet this doesn't work in my custom binding.' \$\endgroup\$ – konijn Feb 9 '14 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\$ – awj Feb 10 '14 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\$ – megawac Feb 10 '14 at 22:15
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.

\$\endgroup\$
  • \$\begingroup\$ default is a reserved word by the way and you have some syntax errors like getStringValue( groups["label"]), "Label" ); \$\endgroup\$ – megawac Feb 10 '14 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\$ – awj Feb 11 '14 at 9:42

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.