search

theironcook / Backbone.ModelBinder

Simple, flexible and powerful Model-View binding for Backbone.
1.42k stars 159 forks source link

Why do we need to call _isElUserEditable in function copyViewValuesToModel() #167

Open kiinoo opened 11 years ago

kiinoo commented 11 years ago

Why need I call this function?

I have a web page opened in browser, I want to reformat the page. What I'm trying is to build the models from existing html elements and then use these models to drive the page.

What is my question?

I'm trying to call that function, but model is not populated with values in view.

I read the doc, we can see:

>>> If you do need to have values copied from the view to the model when bind() is called I would first question why.......
you can use the 4th optional parameter to the bind() function. {initialCopyDirection: Backbone.ModelBinder.Constants.ViewToModel} You can also specify this behavior as the default for all ModelBinder's by calling Backbone.ModelBinder.SetOptions({initialCopyDirection: Backbone.ModelBinder.Constants.ViewToModel});

but this is actually inaccurate, we need to set attribute 'contenteditable' to true for each html element that is bound to a property of model, unless the element is 'input', 'select' or 'textarea'. see below code:

    _isElUserEditable: function(el){
        var isContentEditable = el.attr('contenteditable');
        return isContentEditable || el.is('input') || el.is('select') || el.is('textarea');
    },

so the "copying values from view to model" is blocked by _isElUserEditable() function. I wonder why we need that? can we remove it?

What is my trial solution?

I created a function called copyViewValuesToModel() in ModelCollectionBinder (see below), so we can bind an html element to a collection in which the model is bind to child of the html element. But again the "copying values from view to model" is blocked by _isElUserEditable() function.

    copyViewValuesToModel: function(){
      var parentEl = $(this._elManagerFactory._getParentEl());
      var children = parentEl.children();
      for (var ii = 0; ii < children.length; ii++) {
        var child = children[ii];
        var model = new Backbone.Model();
        var binder = new Backbone.ModelBinder();
        binder.bind(model, child, this._options);
        binder.copyViewValuesToModel();
        this._collection.models.push(model);
      };
    },

does the new function (copyViewValuesToModel) above make sense? if yes, I will send a pull request.

thanks.

theironcook commented 11 years ago

@kiinoo - honestly, I forget why I included _isElUserEditable but I know I didn't add it there just for fun.

Let me think about it and see if I can recall why it exists... Please don't bother with a pull request. I need to refresh my memory...

kiinoo commented 11 years ago

@theironcook thanks for your quick reply. there is a tip may help you remember why, that the 'contenteditable' is also used in _getElValue function line 450 of file Backbone.ModelBinder.js

I'm trying to get value from a <TD>, but failed because it has no 'contenteditable‘ attribute.

theironcook commented 11 years ago

@kiinoo

OK, I remember I added these guards because the code was crashing for elements that were not editable in some situations.

Can you please show me your bindings? How are you setting 'contenteditable' for elements in your bindings?

kiinoo commented 11 years ago

@theironcook

No, I did not set 'contenteditable', I just want to bind values in TD back to property of a model. below is my code:

kiinoo commented 11 years ago 
<html>
  <head>
    <script type="text/javascript" src="lib/jquery.js"></script>
    <script type="text/javascript" src="lib/underscore.js"></script>
    <script type="text/javascript" src="lib/backbone.js"></script>
    <script type="text/javascript" src="Backbone.ModelBinder.js"></script>
    <script type="text/javascript" src="Backbone.CollectionBinder.js"></script>
    <script>
      (function extendBackboneBinder () {
        Backbone.ModelBinder.prototype._isElUserEditable = function (el) {
          return true;
        }

        Backbone.ModelBinder.prototype._getElValue = function(elementBinding, el){
          var elType = el.attr('type');
          if(elType == null)
            return el.html();
          switch (elType) {
            case 'checkbox':
              return el.prop('checked') ? true : false;
            default:
              if(el.attr('contenteditable') !== undefined){
                return el.html();
              }
              else {
                return el.val();
              }
          }
        };

        Backbone.CollectionBinder.prototype.copyViewValuesToModel = function(){
          var parentEl = $(this._elManagerFactory._getParentEl());
          var models = [];
          var children = parentEl.children();
          for (var ii = 0; ii < children.length; ii++) {
            var child = children[ii];
            var model = new Backbone.Model();
            var binder = new Backbone.ModelBinder();
            binder.bind(model, child, this._options);
            binder.copyViewValuesToModel();
            models.push(model);
          }
          this._collection.add(models, {silent: true});
        };
      })();

      function startCopying() {
        if(!window.collectionResult) {window.collectionResult = new Backbone.Collection([]);}
        var elManagerFactory = new Backbone.CollectionBinder.ElManagerFactory('something wont be used');
        collectionBinder = new Backbone.CollectionBinder(elManagerFactory, {id:'>:nth-child(1)', name:'>:nth-child(2)'});
        Backbone.ModelBinder.SetOptions({initialCopyDirection: Backbone.ModelBinder.Constants.ViewToModel});
        collectionBinder.bind(window.collectionResult, jQuery('#example-1-result tbody'));
        collectionBinder.copyViewValuesToModel();
        console.log(JSON.stringify(window.collectionResult));
      }
    </script>

  </head>
  <body>
    <a href="javascript:startCopying()">Start Test!</a>
    <table id="example-1-result">
      <thead>
        <tr>
          <th>Id</th>
          <th>Name</th>
        </tr>
      </thead>
      <tbody>
        <tr>
          <td>12</td>
          <td>Josh</td>
        </tr>
        <tr>
          <td>13</td>
          <td>John</td>
        </tr>
        <tr>
          <td>14</td>
          <td>Jack</td>
        </tr>
      </tbody>
    </table>
  </body>
</html>
theironcook commented 11 years ago

@kiinoo

OK, thank you for the detailed explanation and examples.

This function will not work in a lot of situations...

Backbone.CollectionBinder.prototype.copyViewValuesToModel = function(){
          var parentEl = $(this._elManagerFactory._getParentEl());
          var models = [];
          var children = parentEl.children();
          for (var ii = 0; ii < children.length; ii++) {
            var child = children[ii];
            var model = new Backbone.Model();
            var binder = new Backbone.ModelBinder();
            binder.bind(model, child, this._options);
            binder.copyViewValuesToModel();
            models.push(model);
          }
          this._collection.add(models, {silent: true});
        };

The problem is that you are only looking directly under the parentEl and you are automatically binding to each and every one of the children and you are ignoring the bindings. That completely bypasses most of the ModelBinder's architecture. The ModelBinder is designed to use it's bindings to define scope - what should be bound etc. This might included nested DOM elements or some DOM elements should be ignored etc.

You said you are trying to bind the values of a 'td' back into your model. I'm guessing it's because you are populating the 'td' values on the server and you want to bind those values into your model. Normally I would strongly disagree with this approach - I'd want to bootstrap or fetch your backend data and populate a backbone model. Then bind that model to your view. But I don't know your project or your constraints so maybe it's not possible to do that.

So, if you have to continue with this approach here is what I would suggest. Your example with 'td' elements suggests you have a table and you are populating several models into a table. If that is the case, it makes things messy because you need to iterate through each 'tr' and populate a model. Normally the CollectionBinder is used for this type of situation but it can't for this situation.

If your table only has a single row and you are populating a single model, you should still be using a ModelBinder with a bindings hash defined. Something like this...

// Create your own ModelBinder shim...
// not sure if this will create crashes sometimes?
Backbone.ModelBinder._isBindingUserEditable = function(){return true; };
Backbone.ModelBinder._isElUserEditable= function(){return true; };

// Now create your bindings, this assumes you need to loop over a table's rows because there are multiple models
this.collection = new Backbone.Collection();

this.$('tbody').forEach(function(tr){
  var model = new Backbone.Model();
  var modelBinder = new Backbone.ModelBinder();
  modelBinder.bind(tr, model, {'name': '#name'}, {initialCopyDirection: 'ViewToModel'});
  this.collection.add(model);
}

This is basically pseudo code - I'm not sure if I messed up any syntax. But it should get you close to what you need. Or you can just keep your existing shims.