gmac / backbone.epoxy

Declarative data binding and computed models for Backbone
http://epoxyjs.org
MIT License
615 stars 89 forks source link

broken checked binding handler behavior when used against array attributes? #145

Open disruptek opened 7 years ago

disruptek commented 7 years ago

The checked binding handler is not working for me against array attributes. Obviously, this code has been in production for ages, so perhaps I'm simply missing something, but I really cannot see how this could possibly be working correctly for others.

Let's walk through the set() for the checked binding handler. This function is called with a jQuery selection of the bound input elements and the incoming attribute value from the model.

      set: function($element, value) {
        if ($element.length > 1) {
          $element = $element.filter('[value="'+ value +'"]');
        }

This is an immediate problem, because value is an array of values here, so we've effectively emptied $element with this statement.

        // Default as loosely-typed boolean:
        var checked = !!value;

Now this check to see if the $element is a radio input will throw an exception because we're attempting to toLowerCase() an undefined value. isRadio() is reproduced below.

        if (this.isRadio($element)) {
          // Radio button: match checked state to radio value.
          checked = (value == $element.val());

Next we check to see if value is an array and then look for $element's value therein. This doesn't work, of course, because $element is empty. Even if it remained a list of jQuery elements, this operation wouldn’t work correctly because jQuery will only return the first selected element’s val() and underscore's contains() is expecting to operate upon a single value.

        } else if (isArray(value)) {
          // Checkbox array: match checked state to checkbox value in array contents.
          checked = _.contains(value, $element.val());
        }

        // Set checked property to element:
        $element.prop('checked', checked);
      },

That $element is still empty, so the checked property cannot be set correctly in any event!

Here is isRadio() for completeness:

      // Is radio button: avoids '.is(":radio");' check for basic Zepto compatibility.
      isRadio: function($element) {
        return $element.attr('type').toLowerCase() === 'radio';
      }
    }),
disruptek commented 7 years ago

Also, it seems that inputs (even radio inputs) aren't properly deselected when the model changes -- this is some kinda 1.5-way binding. And when I deselect all checkbox elements, they are completely removed from the view. Go figure.